[Devel] Re: [PATCH] RFC: s390: Move get_signal_to_deliver() up in do_signal
Serge E. Hallyn
serue at us.ibm.com
Thu Feb 11 09:29:17 PST 2010
Quoting Martin Schwidefsky (schwidefsky at de.ibm.com):
> On Wed, 10 Feb 2010 14:40:19 -0600
> "Serge E. Hallyn" <serue at us.ibm.com> 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.
But it sure would simplify checkpoint.
thanks,
-serge
================================================================
Results without signals patch
================================================================
sleeping program control-c'd under gdb:
(gdb) i r
r0 0x3ff00000000 4393751543808
r1 0x4d7dbeedb0 332822146480
r2 0xfffffffffffffffc -4
r3 0x3ffff806608 4398038148616
r4 0x0 0
r5 0x8 8
r6 0x80002b40 2147494720
r7 0x3ffff8068e0 4398038149344
r8 0x80000fcc 2147487692
r9 0x80002b44 2147494724
r10 0x3ffff806508 4398038148360
r11 0x3ffff806618 4398038148632
r12 0x4d7dbea000 332822126592
r13 0x4d7dbb3c40 332821904448
r14 0x4d7db2caaa 332821351082
r15 0x3ffff8063d0 4398038148048
pc 0x4d7db2ccfc 0x4d7db2ccfc <__nanosleep_nocancel+2>
cc 0x0 0
x/i $pswa
0x4d7db2ccfc <__nanosleep_nocancel+2>: lghi %r4,-4095
readc.c (getchar) ctrl-c'd in gdb:
(gdb) i r
r0 0xffffffff00000000 -4294967296
r1 0x4d7dbeedb0 332822146480
r2 0x0 0
r3 0x20000005000 2199023276032
r4 0x400 1024
r5 0x4d7daf8fd8 332821139416
r6 0x80000604 2147485188
r7 0x3ffffba43c0 4398041940928
r8 0x4d7dbeaf90 332822130576
r9 0x4d7dbea908 332822128904
r10 0x4d7da79c38 332820618296
r11 0x4d7dbea9e8 332822129128
r12 0x4d7dbea000 332822126592
r13 0x4d7dbb1b50 332821896016
r14 0x4d7dafa1ac 332821143980
r15 0x3ffffba3f28 4398041939752
pc 0x4d7db52f6a 0x4d7db52f6a <__read_nocancel>
cc 0x0 0
================================================================
Results with signals patch
================================================================
sleeping program control-c'd under gdb:
(gdb) i r
r0 0x3ff00000000 4393751543808
r1 0x4d7dbeedb0 332822146480
r2 0xfffffffffffffdfc -516
r3 0x3ffffa340c8 4398040432840
r4 0x0 0
r5 0x8 8
r6 0x80002b40 2147494720
r7 0x3ffffa343a0 4398040433568
r8 0x80000fcc 2147487692
r9 0x80002b44 2147494724
r10 0x3ffffa33fc8 4398040432584
r11 0x3ffffa340d8 4398040432856
r12 0x4d7dbea000 332822126592
r13 0x4d7dbb3c40 332821904448
r14 0x4d7db2caaa 332821351082
r15 0x3ffffa33e90 4398040432272
pc 0x4d7db2ccfc 0x4d7db2ccfc <__nanosleep_nocancel+2>
cc 0x0 0
x/i $pswa
0x4d7db2ccfc <__nanosleep_nocancel+2>: lghi %r4,-4095
readc.c (getchar), ctrl-c'd in gdb:
(gdb) i r
r0 0xffffffff00000000 -4294967296
r1 0x4d7dbeedb0 332822146480
r2 0xfffffffffffffe00 -512
r3 0x20000005000 2199023276032
r4 0x400 1024
r5 0x4d7daf8fd8 332821139416
r6 0x80000604 2147485188
r7 0x3fffff433c0 4398045737920
r8 0x4d7dbeaf90 332822130576
r9 0x4d7dbea908 332822128904
r10 0x4d7da79c38 332820618296
r11 0x4d7dbea9e8 332822129128
r12 0x4d7dbea000 332822126592
r13 0x4d7dbb1b50 332821896016
r14 0x4d7dafa1ac 332821143980
r15 0x3fffff42f28 4398045736744
pc 0x4d7db52f6c 0x4d7db52f6c <__read_nocancel+2>
cc 0x0 0
_______________________________________________
Containers mailing list
Containers at lists.linux-foundation.org
https://lists.linux-foundation.org/mailman/listinfo/containers
More information about the Devel
mailing list