Serge E. Hallyn wrote:
Quoting Martin Schwidefsky (schwidefsky@xxxxxxxxxx):
On Wed, 10 Feb 2010 14:40:19 -0600
"Serge E. Hallyn" <serue@xxxxxxxxxx> wrote:
The current placement of get_signal_to_deliver() means that
try_to_freeze() in get_signal_to_deliver() will happen after
regs->psw.addr, regs->svcnr, and regs->gprs[2] may have been
mangled. Since the app may get checkpointed while frozen and
then restarted, this means we have to attempt a complicated
and subtle re-calculation of the initial conditions.
If we just move the get_signal_to_deliver() above the
immediately preceding block, we enourmously simplify the
arch-specific checkpoint/restart code.
A full ltp run seems to show no regressions do to this move,
though I'm not familiar enough with the entry_64.S code in
particular to be absolutely confident.
Is this a bad idea?
I think it is a bad idea. The comment of get_signal_to_deliver tells
you that the debugger is invoked and may want to change the registers.
If the get_signal_to_deliver calls is moved then the debugger sees
the unmodified registers which is imho wrong. A comparison of the
gdb testsuite with and without the patch will tell us more.
Right, but I guess what's confounding me is exactly why the values
being set for the debugger make more sense to the debugger than the
initial ones. Note that they're not actually the same as they will
be upon exit, for instance in the -ERESTARTNOHAND case if certain
signals are delivered we'll change psw.addr back after all and set
-EINTR.
So yeah, with this patch, if I send a signal to a program being
debugged and then do 'i r' I see -516 instead of the -4 which I
otherwise see, and a different $pswa. Results for 'sleep' (which
is ERESTART_RESTARTBLOCK) and 'getchar' (which is not) being
interupted is below. Frankly I think the info you see with the
patch is more informative, not less, and the debugger certainly
functions as well as it did before.
Of course there is probably fancier userspace tracing/debugging
code out there which depends on the current behavior? And the
most convincing argument might be that it's all so magical that
changing it is begging for trouble.
I suppose it also changes the behavior/ output of strace/ltrace ?
But it sure would simplify checkpoint.
If this doesn't get through, then an alternative would be to
save the original state -- namely, svcnr, pswa, and gprs[2] --
on somewhere that is accessible to the checkpoint code ?
Oren.
--
To unsubscribe from this list: send the line "unsubscribe linux-s390" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
