On 11/16, Roland McGrath wrote:
>
> > But this smp_rmb() in task_utrace_struct() is only needed when the
> > caller does something like
> >
> > if (task_utrace_flags(tsk))
> > do_something(task_utrace_struct());
>
> If you look at where task_utrace_struct() is used, it's basically always
> like this. All the tracehook.h callers into utrace.c do that.
>
> ...
>
> But that's not the only place, is it? Every utrace_report_* and most every
> other utrace.c entry point is called from tracehook.h code using this pattern.
> Those can have the same issue. So you'd have to make all of those do an:
> if (unlikely(!utrace))
> return;
> sort of check. Is that what you mean?
Yes, agreed, this doesn't look good.
So, I think it is better to add the barriers into task_utrace_struct()
like you already did.
(a very minor nit: s/read_barrier_depends/smp_read_barrier_depends/)
Just to complete the discussion, we could do
static inline unsigned long task_utrace_flags(struct task_struct *task)
{
if (!task->utrace)
return 0;
return task->utrace_flags;
}
to void rmb() without complicationg the code, but this still adds
some overhead to the hot path.
And a bit offtopic question. Apart from "task_utrace_flags() should
be as fast as possible", any other reason we can't move ->utrace_flags
into "struct utrace" ?
In the "likely" case we should worry about (the task was never
traced), ->utrace == NULL and
task_utrace_flags(struct task_struct *task)
{
if (likley(!task->utrace))
return 0;
return task->utrace->flags;
}
shouldn't be slower. If the task was ever traced and then utraced
this adds the extra dereference, yes.
Oleg.
