Re: [Crash-utility] PATCH 00/10] teach crash to work with "live" ramdump
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.