----- 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
--
Crash-utility mailing list
Crash-utility@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/crash-utility
