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. 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 -- 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
