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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux