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.
--
Crash-utility mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/crash-utility
