Re: [Crash-utility] [PATCH v2 03/10] back_trace: don't check /proc if !LOCAL_ACTIVE()
----- Original Message -----
On 04/28, Dave Anderson wrote:
>
>
> > --- a/kernel.c
> > +++ b/kernel.c
> > @@ -2902,7 +2902,7 @@ back_trace(struct bt_info *bt)
> >
> > if (ACTIVE() && !INSTACK(esp, bt)) {
> > sprintf(buf, "/proc/%ld", bt->tc->pid);
> > - if (!file_exists(buf, NULL))
> > + if (!(LOCAL_ACTIVE() && file_exists(buf, NULL)))
> > error(INFO, "task no longer exists\n");
> > else
> > error(INFO,
> > --
> > 2.5.0
>
> This doesn't make sense to me. If it's !LOCAL_ACTIVE() (i.e.
hybrid-live-dump), then
> why would you want to call file_exists()?
It won't be called in this case, please see below.
> Shouldn't it be: LOCAL_ACTIVE() and !file_exists()
This is what I did initially... then decided that error("task no longer
exists\n")
makes more sense if !LOCAL_ACTIVE() && !INSTACK(esp, bt).
IOW. with the patch above the code actually does
if (ACTIVE() && !INSTACK(...)) {
if (LOCAL_ACTIVE() && file_exists(...))
error("invalid/stale stack pointer");
else
error("task no longer exists\n");
}
is it wrong?
I thought that !INSTACK() here likely means the task has gone, but back_trace()
does the additional file_exists() to verify this, and "invalid/stale stack
pointer"
error means that something was wrong.
No?
I can't even remember -- that code's been in place for so long I'd prefer to
just
leave it as-is, and for you to just add something like this:
if (ACTIVE() && !INSTACK(esp, bt)) {
+ if (!(LOCAL_ACTIVE()) }
+ error(INFO, "whatever error message you'd like\n");
+ return;
+ }
sprintf(buf, "/proc/%ld", bt->tc->pid);
if (!file_exists(buf, NULL))
error(INFO, "task no longer exists\n");
else
error(INFO,
"invalid/stale stack pointer for this task: %lx\n",
esp);
return;
}
Dave