On 03/18, Roland McGrath wrote:
>
> > Yes, but the other side lacks a barrier. UTRACE_REPORT does
> >
> > utrace->report = 1;
> > wmb(); // actually mb, but wmb is enough
> > set _TIF_NOTIFY_RESUME;
> >
> > do_notify_resume()->utrace_resume()->start_report() path does
> >
> > if (_TIF_NOTIFY_RESUME)
> > // !!! we need rmb in between !!!
> > if (utrace->report)
> > ...
> >
> > and it can miss ->report.
>
> I see. We have a similar problem for (the first) attach, too, right?
> utrace_add_engine does:
Yes sure. I just meant the barrier was needed even before you changed
utrace_add_engine() to set ->report.
> --- a/include/linux/tracehook.h
> +++ b/include/linux/tracehook.h
> @@ -616,6 +616,12 @@ static inline void set_notify_resume(struct task_struct
> *task)
> static inline void tracehook_notify_resume(struct pt_regs *regs)
> {
> struct task_struct *task = current;
> + /*
> + * This pairs with the barrier implicit in set_notify_resume().
> + * It ensures that we read the nonzero utrace_flags set before
> + * set_notify_resume() was called by utrace setup.
> + */
> + smp_rmb();
smp_mb__after_clear_bit() is enough, but I agree, smp_rmb() is more
understandable.
Oleg.
