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

On Thu, 16 Mar 2023 16:40:48 +0100
Florent Revest [off-list ref] wrote:

quoted

quoted

@@ -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@FVFF77S0Q05N.cambridge.arm.com/#t (local)

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