Re: [PATCH v2 04/10] ftrace: Store direct called addresses in their ops

[Steven Rostedt]

On Thu, 16 Mar 2023 16:40:48 +0100
Florent Revest <revest@xxxxxxxxxxxx> wrote:

> > > @@ -5466,6 +5467,7 @@ __modify_ftrace_direct(struct ftrace_ops *ops, unsigned long addr)
> > >                       entry->direct = addr;
> > >               }
> > >       }
> > > +     WRITE_ONCE(ops->direct_call, addr);  
> >
> > I'm curious about the use of WRITE_ONCE(). It should not go outside the
> > mutex barrier.  
> 
> This WRITE_ONCE was originally suggested by Mark here:
> https://lore.kernel.org/all/Y9vW99htjOphDXqY@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/#t
> 
> My understanding is that it's not so much about avoiding re-ordering
> but rather about avoiding store tearing since a ftrace_caller
> trampoline could concurrently read ops->direct_call. Does that make
> sense ?

Yes, but a comment needs to be added:

     /* Prevent store tearing on some archs */
     WRITE_ONCE(ops->direct_call, addr);  

Or something to that affect. Otherwise I can see it confusing others in the
future. And probably me too, as I'll forget why it was a WRITE_ONCE() by
next month. ;-)

-- Steve