Re: [PATCH] utrace: finish_report() must never set -resume = UTRACE_STOP

2009-11-16 Thread Roland McGrath
Forgot to mention, your tree lacks these patches we sent upstream: Right. I'll merge these into some requirements branch (or maybe just the existing tracehook branch) and merge that into utrace. Thanks, Roland

Re: [PATCH] utrace: utrace_attach_task() forgets to return when -utrace == NULL

2009-11-16 Thread Roland McGrath
Now, if we race with another task doing utrace_task_alloc() and see -utrace != NULL, why should we see the correctly initialized *utrace? utrace_task_alloc() needs wmb(), and the code like above read_barrier_depends(). Ok. Please review what I put in (esp. commit c575558) and send patches

[HACK] utrace: fix utrace_resume()-finish_resume_report() logic

2009-11-16 Thread Oleg Nesterov
In short, it is just wrong to call finish_resume_report() in utrace_resume() without reporting loop, because utrace never clears TIF_NOTIFY_RESUME. It is very possible we enter utrace_resume() with utrace-resume == UTRACE_RESUME, in this case finish_resume_report() does user_disable_single_step().

[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] utrace: utrace_attach_task() forgets to return when -utrace == NULL

2009-11-16 Thread Oleg Nesterov
On 11/16, Roland McGrath wrote: Now, if we race with another task doing utrace_task_alloc() and see -utrace != NULL, why should we see the correctly initialized *utrace? utrace_task_alloc() needs wmb(), and the code like above read_barrier_depends(). Ok. Please review what I put in

Re: [HACK] utrace: fix utrace_resume()-finish_resume_report() logic

2009-11-16 Thread Roland McGrath
In short, it is just wrong to call finish_resume_report() in utrace_resume() without reporting loop, because utrace never clears TIF_NOTIFY_RESUME. It's not supposed to. The arch code clears TIF_NOTIFY_RESUME before calling tracehook_notify_resume(). This implies that utrace must keep its

Re: [PATCH] utrace: utrace_attach_task() forgets to return when -utrace == NULL

2009-11-16 Thread Roland McGrath
Just in case fyi, I cooked the almost identical patch yesterday, but it didn't help: make xcheck crashes. Not that I really expected it can help on x86. Right. Not sure I understand exactly... Yes, sometimes we know we don't need read_barrier_depends(). For example, @@ -302,7

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

Re: [PATCH] utrace: utrace_attach_task() forgets to return when -utrace == NULL

2009-11-16 Thread Oleg Nesterov
On 11/16, Roland McGrath wrote: But read_barrier_depends() does nothig except on alpha, why should we care? I thought this was the barrier you were talking about. Anyway, we should be pedantically correct about these. (People do even still build Linux/Alpha.) Yes, sure. I meant, we

Re: [PATCH] utrace: utrace_attach_task() forgets to return when -utrace == NULL

2009-11-16 Thread Roland McGrath
Yes, sure. I meant, we shouldn't worry that this barrier adds too much overhead to task_utrace_struct(). Ah! Sorry, I misread you. Yes, good point. We don't have the explicit barrier, but I think it is not needed. In this case utrace_attach_task() does unlock+lock at least once before