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
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
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().
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()
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
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
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
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
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.
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
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
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
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
13 matches
Mail list logo