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
