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

[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 (esp. commit c575558)

Yes, I think this is correct.

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.

> Perhaps it is overkill to
> do read_barrier_depends() in task_utrace_struct().

Yes, perhaps. But this is safer.

> Do you think it's not really needed in all those places we extract the
> pointer before spin_lock et al?

Not sure I understand exactly... Yes, sometimes we know we don't need
read_barrier_depends(). For example,

	@@ -302,7 +309,7 @@ struct utrace_engine *utrace_attach_task(
		if (!utrace) {
			if (unlikely(!utrace_task_alloc(target)))
				return ERR_PTR(-ENOMEM);
	-               utrace = target->utrace;
	+               utrace = task_utrace_struct(target);
		}

I think this change is not necessary (but imho good anyway), this path
must take task_lock() and thus it must always see the correct *utrace.

But read_barrier_depends() does nothig except on alpha, why should we
care?


This reminds me, utrace_interrupt_pending() and similar code needs rmb()
or task->utrace != NULL check.

Oleg.