Re: [RFC,PATCH 0/14] utrace/ptrace
* Christoph Hellwig h...@infradead.org wrote: [...] Given that's it's pretty much too later for the 2.6.33 cycle anyway I'd suggest you make sure the remaining two major architectures (arm and mips) get converted, and if the remaining minor architectures don't manage to get their homework done they're left without ptrace. I suspect the opinion of the ptrace maintainers matters heavily whether it's appropriate for v2.6.33. You are not going to maintain this, they are. Regarding porting it to even more architectures - that's pretty much the worst idea possible. It increases maintenance and testing overhead by exploding the test matrix, while giving little to end result. Plus the worst effect of it is that it becomes even more intrusive and even harder (and riskier) to merge. So dont do that. The best strategy is to concentrate on just one or two well-tested architectures, and then grow to other architectures gradually. Like we've done it for basically all big kernel features in the past 10 years (that had non-trivial arch dependencies), with no exception that i can remember. Thanks, Ingo
Re: [RFC,PATCH 0/14] utrace/ptrace
* Christoph Hellwig h...@infradead.org wrote: On Thu, Nov 26, 2009 at 10:10:52AM +0100, Ingo Molnar wrote: [...] Given that's it's pretty much too later for the 2.6.33 cycle anyway I'd suggest you make sure the remaining two major architectures (arm and mips) get converted, and if the remaining minor architectures don't manage to get their homework done they're left without ptrace. I suspect the opinion of the ptrace maintainers matters heavily whether it's appropriate for v2.6.33. You are not going to maintain this, they are. I am whoever like many others going to use it. And throwing in new code a few days before the merge window closes [...] FYI, the merge window has not opened yet, so it cannot close in a few days. [...] and thus not getting any of the broad -next test coverage is a pretty bad idea. In the end it will be the maintainers ruling but that doesn't make it a good idea from the engineering point of view. FYI, it's been in -mm, that's where it's maintained. Regarding porting it to even more architectures - that's pretty much the worst idea possible. It increases maintenance and testing overhead by exploding the test matrix, while giving little to end result. Plus the worst effect of it is that it becomes even more intrusive and even harder (and riskier) to merge. But it doesn't. Take a look at what these patches actually do, they basically introduce a new utrace layer, and (conditionally) rewrite ptrace to use it. The arch support isn't actually part of these patches directly but rather the cleanup of the underlying arch ptrace code to use regsets, tracehooks and co so that the new ptrace code can use. ( I am aware of its design, i merged the original tracehook patches for x86. ) What the patches in the current form do is to introduce two different ptrace implementations, with one used on the architectures getting most testing and another secondary one for left over embedded or dead architectures with horrible results. So removing the old one is much better. The arm ptrace rewrite has already been posted by Roland, btw including some feedback from Russell, but nothing really happened to it. Yes. Which is a further argument to not do it like that but to do one arch at a time. Trying to do too much at once is bad engineering. Ingo
Re: [RFC,PATCH 0/14] utrace/ptrace
On Thu, 2009-11-26 at 12:37 +0530, Srikar Dronamraju wrote: Hi Christoph, The other thing is that this patchset really doesn't quite justify utrace. It's growing a lot more code without actually growing any useful functionality. What about all those other utrace killer features that have been promised for a long time? We are working on in-kernel gdbstub which was one of the features that you had asked for. gdbstub does pass unit tests; but we are looking at some way to hack the GDB testsuite to run its regression tests. Once we are able to run the GDB testsuite and utrace is part of some upstream tree, we plan to post these patches to LKML for comments. gdbstub uses utrace and uprobes underneath. Uprobes was rewritten to remove issues that LKML developers had opposed. Uprobes also has its own ftrace plugin to use uprobes. Currently in-kernel gdbstub is hosted by Frank Ch. Eigler over here: git://web.elastic.org/~fche/utrace-ext.git branch name utrace-gdbstub-uprobes If its anywhere near functioning it would have made sense to send it out as an RFC patch-set right along with the utrace one.
Ptrace Testsuite Results for CONFIG_UTRACE
Hello! Please find below results by using the latest ptrace testsuite with and without CONFIG_UTRACE. Overall, There did not look like some regression. The failures were getting less with the ptrace-utrace patchset applied. There are some tests failed for both kernels. * detach-sigkill-race * clone-multi-ptrace * step-fork Thanks! CAI Qian HP XW6600 Workstation (x86_64 Intel CPU) * make check --- noutrace/check 2009-11-26 20:55:15.0 +0800 +++ utrace/check2009-11-26 20:53:10.0 +0800 @@ -31,18 +31,20 @@ PASS: step-simple PASS: step-through-sigret PASS: stop-attach-then-wait -FAIL: detach-stopped +PASS: detach-stopped PASS: detach-stopped-rhel5 PASS: clone-multi-ptrace PASS: clone-ptrace PASS: o_tracevfork PASS: o_tracevforkdone PASS: detach-parting-signal +detach-sigkill-race: detach-sigkill-race.c:167: reproduce: Unexpected error: No such process. +detach-sigkill-race: detach-sigkill-race.c:106: handler_fail: Assertion `0' failed. FAIL: detach-sigkill-race PASS: waitpid-double-report PASS: o_tracevfork-parent PASS: stopped-detach-sleeping -FAIL: stopped-attach-transparency +PASS: stopped-attach-transparency PASS: erestartsys-trap PASS: highmem-debugger PASS: sigint-before-syscall-exit @@ -50,8 +52,7 @@ PASS: step-from-clone FAIL: step-fork -4 of 47 tests failed +2 of 47 tests failed (4 tests were not run) * make check-biarch --- noutrace/check-biarch 2009-11-26 20:55:37.0 +0800 +++ utrace/check-biarch 2009-11-26 20:53:03.0 +0800 @@ -31,7 +31,7 @@ PASS: step-simple PASS: step-through-sigret PASS: stop-attach-then-wait -FAIL: detach-stopped +PASS: detach-stopped PASS: detach-stopped-rhel5 PASS: clone-multi-ptrace PASS: clone-ptrace @@ -42,7 +42,7 @@ PASS: waitpid-double-report PASS: o_tracevfork-parent PASS: stopped-detach-sleeping -FAIL: stopped-attach-transparency +PASS: stopped-attach-transparency PASS: erestartsys-trap SKIP: highmem-debugger PASS: sigint-before-syscall-exit @@ -50,7 +50,7 @@ PASS: step-from-clone FAIL: step-fork -3 of 44 tests failed +1 of 44 tests failed (7 tests were not run) * make xcheck and make xcheck-biarch results are identical. PASS: clone-get-signal PASS: ppc-ptrace-exec-full-regs PASS: x86_64-cs SKIP: user-area-padding PASS: reparent-zombie-clone All 6 tests passed (1 test was not run) HP XW9400 Workstation (x86_64 AMD CPU) * make check (both FAIL: detach-sigkill-race) --- noutrace/check 2009-11-26 21:14:12.0 +0800 +++ utrace/check2009-11-26 20:57:37.0 +0800 @@ -31,11 +31,11 @@ PASS: step-simple PASS: step-through-sigret PASS: stop-attach-then-wait -FAIL: detach-stopped +PASS: detach-stopped PASS: detach-stopped-rhel5 clone-multi-ptrace: clone-multi-ptrace.c:205: try_to_reproduce: Assertion `__extension__ (((union { __typeof(status) __in; int __i; }) { .__in = (status) }).__i))) 0xff00) 8) == 5' failed. clone-multi-ptrace: clone-multi-ptrace.c:101: handler_fail: Assertion `0' failed. -/bin/sh: line 5: 13519 Aborted ${dir}$tst +/bin/sh: line 5: 13183 Aborted ${dir}$tst FAIL: clone-multi-ptrace PASS: clone-ptrace PASS: o_tracevfork @@ -45,7 +45,7 @@ PASS: waitpid-double-report PASS: o_tracevfork-parent PASS: stopped-detach-sleeping -FAIL: stopped-attach-transparency +PASS: stopped-attach-transparency PASS: erestartsys-trap PASS: highmem-debugger PASS: sigint-before-syscall-exit @@ -53,7 +53,7 @@ PASS: step-from-clone FAIL: step-fork -5 of 47 tests failed +3 of 47 tests failed * make check-biarch --- noutrace/check-biarch 2009-11-26 20:59:53.0 +0800 +++ utrace/check-biarch 2009-11-26 20:57:56.0 +0800 @@ -31,7 +31,7 @@ PASS: step-simple PASS: step-through-sigret PASS: stop-attach-then-wait -FAIL: detach-stopped +PASS: detach-stopped PASS: detach-stopped-rhel5 PASS: clone-multi-ptrace PASS: clone-ptrace @@ -42,7 +42,7 @@ PASS: waitpid-double-report PASS: o_tracevfork-parent PASS: stopped-detach-sleeping -FAIL: stopped-attach-transparency +PASS: stopped-attach-transparency PASS: erestartsys-trap SKIP: highmem-debugger PASS: sigint-before-syscall-exit @@ -50,7 +50,7 @@ PASS: step-from-clone FAIL: step-fork -3 of 44 tests failed +1 of 44 tests failed * make xcheck and make xcheck-biarch results are identical. PASS: late-ptrace-may-attach-check PASS: tracer-lockup-on-sighandler-kill PASS: clone-get-signal PASS: ppc-ptrace-exec-full-regs PASS: x86_64-cs SKIP: user-area-padding PASS: reparent-zombie-clone All 6 tests passed (1 test was not run) Dell PowerEdge 1650 (i686 Intel CPU) * make check --- noutrace/check 2009-11-26 21:02:59.0 +0800 +++ utrace/check2009-11-26 21:01:56.0 +0800 @@ -1,4
Re: [RFC,PATCH 0/14] utrace/ptrace
On 11/26, Christoph Hellwig wrote: What the patches in the current form do is to introduce two different ptrace implementations, with one used on the architectures getting most testing and another secondary one for left over embedded or dead architectures with horrible results. Yes, nobody likes 2 implementations. I guess Roland and me hate CONFIG_UTRACE much more than anybody else. So removing the old one is much better. I am in no position to discuss this option. It is very easy to remove the old code and break !HAVE_ARCH_TRACEHOOK architectures. Although personally I am not sure this is practical. If we merge utrace, perhaps we will get more attention from maintainers, the old code will be officially deprecated/obsolete. I sent some trivial initial changes in arch/um/ a long ago, the patch was silently ignored. Even if I was able to fix arch/xxx myself, I don't understand how can I send the patches to maintainers until utrace is already merged in -mm at least. Oleg.
World of pilules!
New generation extra-strength med, for the treatment of ED only in men. http://vzrg.buxmybgopa.com/
Re: powerpc: fork stepping (Was: [RFC, PATCH 0/14] utrace/ptrace)
On Thu, Nov 26, 2009 at 06:25:24PM +0100, Oleg Nesterov wrote: On 11/26, Oleg Nesterov wrote: On 11/26, Ananth N Mavinakayanahalli wrote: step-fork: step-fork.c:56: handler_fail: Assertion `0' failed. /bin/sh: line 5: 17325 Aborted ${dir}$tst FAIL: step-fork Good to know, thanks again Ananth. I'll take a look. Since I know nothing about powerpc, I can't promise the quick fix ;) The bug was found by code inspection, but the fix is not trivial because it depends on arch/, and it turns out the arch-independent fix in ptrace-copy_process-should-disable-stepping.patch http://marc.info/?l=linux-mm-commitsm=125789789322573 doesn't work. Just noticed the test-case fails in handler_fail(). Most probably this means it is killed by SIGALRM because either parent or child hang in wait(). Perhaps we have another (ppc specific?) bug, but currently I do not understand how this is possible, this should not be arch-dependent. I can confirm that we have another bug on ppc arch. The test case below is spinning forever, #include stdio.h #include unistd.h #include signal.h #include sys/ptrace.h #include sys/wait.h #include assert.h int main(void) { int pid, status; if (!(pid = fork())) { assert(ptrace(PTRACE_TRACEME) == 0); kill(getpid(), SIGSTOP); if (!fork()) return 0; printf(fork passed..\n); return 0; } for (;;) { assert(pid == wait(status)); if (WIFEXITED(status)) break; assert(ptrace(PTRACE_SINGLESTEP, pid, 0,0) == 0); } printf(Parent exit.\n); return 0; } it doesn't hang, the parent is spinning around for, the test case isn't printing anything. Seems like fork() can't complete under PTRACE_SINGLESTEP. -- Veaceslav
Re: powerpc: fork stepping (Was: [RFC, PATCH 0/14] utrace/ptrace)
Veaceslav doesn't have the time to continue, but he gave me access to rhts machine ;) The kernel is 2.6.31.6 btw. On 11/26, Veaceslav Falico wrote: Just noticed the test-case fails in handler_fail(). Most probably this means it is killed by SIGALRM because either parent or child hang in wait(). Perhaps we have another (ppc specific?) bug, but currently I do not understand how this is possible, this should not be arch-dependent. I can confirm that we have another bug on ppc arch. The test case below is spinning forever, [...] it doesn't hang, the parent is spinning around for, the test case isn't printing anything. Seems like fork() can't complete under PTRACE_SINGLESTEP. Yep, thanks a lot Veaceslav. I modified this test-case to print si_addr: int main(void) { int pid, status; if (!(pid = fork())) { assert(ptrace(PTRACE_TRACEME) == 0); kill(getpid(), SIGSTOP); if (!fork()) return 0; printf(fork passed..\n); return 0; } for (;;) { siginfo_t info; assert(pid == wait(status)); assert(status = 0x57f); assert(ptrace(PTRACE_GETSIGINFO, pid, 0,info) == 0); printf(%p\n, info.si_addr); if (WIFEXITED(status)) break; assert(ptrace(PTRACE_SINGLESTEP, pid, 0,0) == 0); } printf(Parent exit.\n); return 0; } the output is: ... 0xfedf880 0xfedf884 ... 0xfedf96c 0xfedf970 this is fork which calls __GI__IO_list_lock Dump of assembler code for function fork: 0x0fedf880 fork+0:mflrr0 ... 0x0fedf96c fork+236: li r28,0 0x0fedf970 fork+240: bl 0xfeacce0 __GI__IO_list_lock Then it loops inside __GI__IO_list_lock ... 0xfeacd24 0xfeacd28 0xfeacd2c 0xfeacd30 0xfeacd34 0xfeacd24 0xfeacd28 0xfeacd2c 0xfeacd30 0xfeacd34 0xfeacd24 0xfeacd28 0xfeacd2c 0xfeacd30 0xfeacd34 ... and so on forever, Dump of assembler code for function __GI__IO_list_lock: 0x0feacce0 __GI__IO_list_lock+0: mflrr0 0x0feacce4 __GI__IO_list_lock+4: stwur1,-32(r1) 0x0feacce8 __GI__IO_list_lock+8: li r11,0 0x0feaccec __GI__IO_list_lock+12: bcl-20,4*cr7+so,0xfeaccf0 __GI__IO_list_lock+16 0x0feaccf0 __GI__IO_list_lock+16: li r9,1 0x0feaccf4 __GI__IO_list_lock+20: stw r0,36(r1) 0x0feaccf8 __GI__IO_list_lock+24: stw r30,24(r1) 0x0feaccfc __GI__IO_list_lock+28: mflrr30 0x0feacd00 __GI__IO_list_lock+32: stw r31,28(r1) 0x0feacd04 __GI__IO_list_lock+36: stw r29,20(r1) 0x0feacd08 __GI__IO_list_lock+40: addir29,r2,-29824 0x0feacd0c __GI__IO_list_lock+44: addis r30,r30,16 0x0feacd10 __GI__IO_list_lock+48: addir30,r30,13060 0x0feacd14 __GI__IO_list_lock+52: lwz r31,-6436(r30) 0x0feacd18 __GI__IO_list_lock+56: lwz r0,8(r31) 0x0feacd1c __GI__IO_list_lock+60: cmpwcr7,r0,r29 0x0feacd20 __GI__IO_list_lock+64: beq-cr7,0xfeacd4c __GI__IO_list_lock+108 beg- 0x0feacd24 __GI__IO_list_lock+68: lwarx r0,0,r31 0x0feacd28 __GI__IO_list_lock+72: cmpwr0,r11 0x0feacd2c __GI__IO_list_lock+76: bne-0xfeacd38 __GI__IO_list_lock+88 0x0feacd30 __GI__IO_list_lock+80: stwcx. r9,0,r31 end- 0x0feacd34 __GI__IO_list_lock+84: bne+0xfeacd24 __GI__IO_list_lock+68 I don't even know whether this is user-space bug or kernel bug, the asm above is the black magic for me. Anyone who knows something about powerpc can give me a hint? Oleg.
Re: powerpc: fork stepping (Was: [RFC, PATCH 0/14] utrace/ptrace)
On 11/26, Oleg Nesterov wrote: Then it loops inside __GI__IO_list_lock 0xfeacd24 0xfeacd28 0xfeacd2c 0xfeacd30 0xfeacd34 ... and so on forever, Dump of assembler code for function __GI__IO_list_lock: 0x0feacce0 __GI__IO_list_lock+0: mflrr0 0x0feacce4 __GI__IO_list_lock+4: stwur1,-32(r1) 0x0feacce8 __GI__IO_list_lock+8: li r11,0 0x0feaccec __GI__IO_list_lock+12: bcl-20,4*cr7+so,0xfeaccf0 __GI__IO_list_lock+16 0x0feaccf0 __GI__IO_list_lock+16: li r9,1 0x0feaccf4 __GI__IO_list_lock+20: stw r0,36(r1) 0x0feaccf8 __GI__IO_list_lock+24: stw r30,24(r1) 0x0feaccfc __GI__IO_list_lock+28: mflrr30 0x0feacd00 __GI__IO_list_lock+32: stw r31,28(r1) 0x0feacd04 __GI__IO_list_lock+36: stw r29,20(r1) 0x0feacd08 __GI__IO_list_lock+40: addir29,r2,-29824 0x0feacd0c __GI__IO_list_lock+44: addis r30,r30,16 0x0feacd10 __GI__IO_list_lock+48: addir30,r30,13060 0x0feacd14 __GI__IO_list_lock+52: lwz r31,-6436(r30) 0x0feacd18 __GI__IO_list_lock+56: lwz r0,8(r31) 0x0feacd1c __GI__IO_list_lock+60: cmpwcr7,r0,r29 0x0feacd20 __GI__IO_list_lock+64: beq-cr7,0xfeacd4c __GI__IO_list_lock+108 beg- 0x0feacd24 __GI__IO_list_lock+68: lwarx r0,0,r31 0x0feacd28 __GI__IO_list_lock+72: cmpwr0,r11 0x0feacd2c __GI__IO_list_lock+76: bne-0xfeacd38 __GI__IO_list_lock+88 0x0feacd30 __GI__IO_list_lock+80: stwcx. r9,0,r31 end- 0x0feacd34 __GI__IO_list_lock+84: bne+0xfeacd24 __GI__IO_list_lock+68 I don't even know whether this is user-space bug or kernel bug, the asm above is the black magic for me. When I use gdb to step over __GI__IO_list_lock(), it doesn't loop. I straced gdb and noticed that when the trace reaches 0x0feacd24: lwarx r0,0,r31 gdb does PTRACE_CONT, not PTRACE_SINGLESTEP. After that the child stops at 0x0feacd38, the next insn (isync). Anyone who knows something about powerpc can give me a hint? Please ;) Oleg.
Re: powerpc: fork stepping (Was: [RFC, PATCH 0/14] utrace/ptrace)
Oleg Nesterov writes: 0xfeacd24 0xfeacd28 0xfeacd2c 0xfeacd30 0xfeacd34 ... and so on forever, ... beg- 0x0feacd24 __GI__IO_list_lock+68: lwarx r0,0,r31 0x0feacd28 __GI__IO_list_lock+72: cmpwr0,r11 0x0feacd2c __GI__IO_list_lock+76: bne-0xfeacd38 __GI__IO_list_lock+88 0x0feacd30 __GI__IO_list_lock+80: stwcx. r9,0,r31 end- 0x0feacd34 __GI__IO_list_lock+84: bne+0xfeacd24 __GI__IO_list_lock+68 I don't even know whether this is user-space bug or kernel bug, the asm above is the black magic for me. The lwarx and stwcx. work together to do an atomic update to the word whose address is in r31. They are like LL (load-linked) and SC (store-conditional) on other architectures such as alpha. Basically the lwarx creates an internal reservation on the word pointed to by r31 and loads its value into r0. The stwcx. stores into that word but only if the reservation still exists. The reservation gets cleared (in hardware) if any other cpu writes to that word in the meantime. If the reservation did get cleared, the bne (branch if not equal) instruction will be taken and we loop around to try again. There is a difficulty when single-stepping through such a sequence because the process of taking the single-step exception and returning will clear the reservation. Thus if you single-step through that sequence it will never succeed. I believe gdb has code to recognize this kind of sequence and run through it without stopping until after the bne, precisely to avoid this problem. Paul.
Re: powerpc: fork stepping (Was: [RFC, PATCH 0/14] utrace/ptrace)
Paul Mackerras pau...@samba.org writes: I believe gdb has code to recognize this kind of sequence and run through it without stopping until after the bne, precisely to avoid this problem. See gdb/rs6000-tdep.c:ppc_deal_with_atomic_sequence. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 And now for something completely different.
Re: powerpc: fork stepping (Was: [RFC, PATCH 0/14] utrace/ptrace)
On Thu, Nov 26, 2009 at 03:50:51PM +0100, Oleg Nesterov wrote: I changed the subject. This bug has nothing to do with utrace, the kernel fails with or without these changes. On 11/26, Ananth N Mavinakayanahalli wrote: On Wed, Nov 25, 2009 at 04:40:52PM +0100, Oleg Nesterov wrote: On 11/25, Ananth N Mavinakayanahalli wrote: step-fork: step-fork.c:56: handler_fail: Assertion `0' failed. /bin/sh: line 5: 24803 Aborted ${dir}$tst FAIL: step-fork This is expected. Should be fixed by ptrace-copy_process-should-disable-stepping.patch in -mm tree. (I am attaching this patch below just in case) I din't mention this patch in this series because this bug is ortogonal to utrace/ptrace. The patch doesn't seem to fix the issue on powerpc: step-fork: step-fork.c:56: handler_fail: Assertion `0' failed. /bin/sh: line 5: 17325 Aborted ${dir}$tst FAIL: step-fork Good to know, thanks again Ananth. I'll take a look. Since I know nothing about powerpc, I can't promise the quick fix ;) The bug was found by code inspection, but the fix is not trivial because it depends on arch/, and it turns out the arch-independent fix in ptrace-copy_process-should-disable-stepping.patch http://marc.info/?l=linux-mm-commitsm=125789789322573 doesn't work. Ananth, could you please run the test-case from the changelog below ? I do not really expect this can help, but just in case. Right, it doesn't help :-( GDB shows that the parent is forever struck at wait(). Ananth