[Crash-utility] [PATCH v2 03/10] back_trace: don't check /proc if !LOCAL_ACTIVE()

[Oleg Nesterov]

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?

Oleg.