[Crash-utility] PATCH 00/10] teach crash to work with "live" ramdump

Oleg Nesterov oleg at redhat.com
Tue Apr 26 18:18:43 UTC 2016


n 04/26, Dave Anderson wrote:
>
>
> ----- Original Message -----
> > On 04/26, Dave Anderson wrote:
> > >
> > > > But all the LOCAL_ACTIVE changes in 1-7 do not care about the details of "live"
> > > > mechanism at all. So I still think we need a generic helper which should be true
> > > > if local-and-active. Or, vice versa, remote-and-active, this doesn't matter.
> > ...
> > > > So you suggest to change this patch to do
> > > >
> > > > 		if (ACTIVE() && !ACTIVE_QEMU() && !INSTACK(...))
> > > >
> > > > To me this simply looks worse, but I won't insist. But note that if we ever have
> > > > another ACTIVE_SOMETHING() source, we will need to modify this code again.  While
> > > > this code do not care about qemu/something at all. So I still think we need a new
> > > > helper which doesn't depend on qemu or whatever else.
> > >
> > > Right, but this is definitely the outlier with respect to "live" systems.
> >
> > Can't understand...
>
> I mean that it's a special case of a "live" system.  For the most part it is, but
> there are a number of gotcha's that would need to be caught and addressed.

Yes, but (in this particular case) it is only "special" because it is remote. Nothing
else.

And again, again. My initial plan was

	- set

		pc->flags |= (LIVE_SYSTEM | REM_LIVE_SYSTEM);

	  in 10/10

	- use !REMOTE_ACTIVE() in the code above and other patches in 1-7.


But then I noticed that all this REM_ code is mostly broken. So I added the
new LOCAL_ACTIVE() helper, so these changes do not depend on REMOTE logic.

> > OK. But not in the case above? This particular /proc/$tc->pid doesn't need to
> > know if this (remote) live system is ACTIVE_QEMU() or ACTIVE_SOMETHING_ELSE()?
> > All it needs to know if we debug the (live) kernel on the same machine or not.
>
> I think we're still talking about the code segment in back_trace()?  All that
> is doing is recognizing that the "current" task no longer exists, and the backtrace
> attempt should fail.

Oh, I see. I didn't try to say this change is very important. Still this looks
just wrong.

> I don't know how
> you're going to easily determine whether task still exists.

I am not going to do this. Heh, I just noticed the patch is wrong! :/ Probably
this confused you, sorry.

	if (LOCAL_ACTIVE() && !INSTACK(esp, bt)) {
		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;
	}

of course, this is not what I actually tried to do. I meant something like

	if (ACTIVE() && !INSTACK(esp, bt)) {
		sprintf(buf, "/proc/%ld", bt->tc->pid);
		if (!LOCAL_ACTIVE() || !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;
	}

Damn ;)

Oleg.




More information about the Crash-utility mailing list