Re: [PATCH 133] stepping, accommodate to utrace-cleanup changes

2009-11-20 Thread Roland McGrath
The former is e.g. PTRACE_SINGLESTEP while an unrelated engine uses UTRACE_EVENT(SYSCALL_ENTRY). The ptrace engine's report_quiesce returns UTRACE_SINGLESTEP. finish_resume_report() calls user_enable_single_step(). That seems fine. Right? In this case ptrace_report_quiesce(event)

Re: [PATCH 133] stepping, accommodate to utrace-cleanup changes

2009-11-19 Thread Oleg Nesterov
On 11/18, Roland McGrath wrote: Hmm. not sure I understand how this can work. I mean, this won't enable stepping (in this particular case we are discussing). I might be confused. I'm thinking of two cases: the report_syscall_entry pass yields UTRACE_SINGLESTEP, or that it yields

Re: copy_process utrace_init_task (Was: [PATCH 133] stepping, accommodate to utrace-cleanup changes)

2009-11-18 Thread Roland McGrath
Found the trivial but nasty problem. Ah! Good catch. I added tracehook_init_task() in my tree. I don't see much benefit in sending any tracehook patch upstream for this. tracehook_init_task() corresponds to tracehook_free_task(), which is only added by utrace (and both would just be empty in

Re: copy_process utrace_init_task (Was: [PATCH 133] stepping, accommodate to utrace-cleanup changes)

2009-11-18 Thread Oleg Nesterov
On 11/18, Roland McGrath wrote: I added tracehook_init_task() in my tree. I don't see much benefit in sending any tracehook patch upstream for this. tracehook_init_task() corresponds to tracehook_free_task(), which is only added by utrace (and both would just be empty in a separate

Re: [PATCH 133] stepping, accommodate to utrace-cleanup changes

2009-11-18 Thread Oleg Nesterov
On 11/18, Roland McGrath wrote: Once again. The tracee sleeps in SYSCALL_ENTER. The tracer resumes the tracee via utrace_control(UTRACE_SINGLESTEP). Right. This is another strange corner. It doesn't necessarily make especially good sense intrinsically for single-step to have this meaning

Re: [PATCH 133] stepping, accommodate to utrace-cleanup changes

2009-11-18 Thread Oleg Nesterov
I'll reply tomorrow, it is to late for me. But I noticed the funny detail in your email, On 11/18, Roland McGrath wrote: Yes. But, hmm. utrace_stop(task, utrace, UTRACE_RESUME); /* XXX */ This XXX was there about passing UTRACE_RESUME, because that's not really right. I

copy_process utrace_init_task (Was: [PATCH 133] stepping, accommodate to utrace-cleanup changes)

2009-11-17 Thread Oleg Nesterov
On 11/16, Oleg Nesterov wrote: On 11/16, Oleg Nesterov wrote: And I didn't check make xcheck, I guess it still crashes the kernel. Yes it does. I am almost sure the bug should be trivial, but somehow can't find find it. Found the trivial but nasty problem.

tracehook_report_syscall_exit() PT_PTRACED (Was: [PATCH 133] stepping, accommodate to utrace-cleanup changes)

2009-11-17 Thread Oleg Nesterov
On 11/16, Roland McGrath wrote: The change we talked about before seems simple enough and should cover this without new kludges in the ptrace layer. I did this (commit f19442c). I will reply to this in the next email, I'd like to discuss another minor related issue first. I noticed this

Re: [PATCH 133] stepping, accommodate to utrace-cleanup changes

2009-11-17 Thread Oleg Nesterov
On 11/16, Roland McGrath wrote: Whatever temporary hacks are fine by me one way or the other. They will just be coming out later along with assorted other cleanup. We certainly do want to get this right in the utrace layer. Yes. But imho it is always good to test/review the patches against

Re: tracehook_report_syscall_exit() PT_PTRACED (Was: [PATCH 133] stepping, accommodate to utrace-cleanup changes)

2009-11-17 Thread Roland McGrath
but now I think perhaps it would be better to send ptrace-change-tracehook_report_syscall_exit-to-handle-stepping_fix to akpm right now: --- a/include/linux/tracehook.h +++ b/include/linux/tracehook.h @@ -134,7 +134,7 @@ static inline __must_check int tracehook

[PATCH 133] stepping, accommodate to utrace-cleanup changes

2009-11-16 Thread Oleg Nesterov
This needs more comment, I'll try to add them in a separate patch. With the recent changes in utrace API utrace_control(UTRACE_SINGLESTEP) postpones enable_step() to utrace_resume() stage. The tracee can pass tracehook_report_syscall_exit() without TIF_SINGLESTEP, this breaks the send_sigtrap()

Re: [PATCH 133] stepping, accommodate to utrace-cleanup changes

2009-11-16 Thread Oleg Nesterov
Just in case, forgot to clarify... On 11/16, Oleg Nesterov wrote: With this patch make check passes all tests again (to clarify, except some tests which upstream doesn't pass too), including this one: with this patch + [HACK] utrace: fix utrace_resume()-finish_resume_report() logic And I

Re: [PATCH 133] stepping, accommodate to utrace-cleanup changes

2009-11-16 Thread Oleg Nesterov
On 11/16, Oleg Nesterov wrote: And I didn't check make xcheck, I guess it still crashes the kernel. Yes it does. I am almost sure the bug should be trivial, but somehow can't find find it. Just fyi, to ensure this is connected to utrace-indirect I applied the hack below and the bug goes away.

Re: [PATCH 133] stepping, accommodate to utrace-cleanup changes

2009-11-16 Thread Roland McGrath
Whatever temporary hacks are fine by me one way or the other. They will just be coming out later along with assorted other cleanup. We certainly do want to get this right in the utrace layer. The change we talked about before seems simple enough and should cover this without new kludges in the

Re: [PATCH 133] stepping, accommodate to utrace-cleanup changes

2009-11-16 Thread Roland McGrath
Yes it does. I am almost sure the bug should be trivial, but somehow can't find find it. Just fyi, to ensure this is connected to utrace-indirect I applied the hack below and the bug goes away. Does s/kmem_cache_zalloc/kzalloc/ really have anything to do with it? Isn't it just the allocation