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.