Re: [PATCH] ftrace: Synchronize the initialization of ftrace_ops

[Steven Rostedt <rostedt@goodmis.org>]

On Wed, 02 Sep 2026 00:33:43 +0000
sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] The newly introduced `ops_mutex` logic implements a Double-Checked Locking pattern for `ftrace_ops` initialization but misses required memory barriers, allowing concurrent threads to observe a partially initialized structure on weakly-ordered architectures.
> --
> 
> commit 0d99940712b506309368df1237eae674da6b9bc1
> Author: Steven Rostedt <rostedt@goodmis.org>
> 
> ftrace: Synchronize the initialization of ftrace_ops
> 
> This commit introduces a mutex to synchronize the initialization of ftrace_ops
> across multiple tasks. It implements a fast-path optimization by checking the
> FTRACE_OPS_FL_INITIALIZED flag before acquiring the lock.
> 
> Link: https://lore.kernel.org/all/20260829025528.49A831F000E9@smtp.kernel.org/
> 
> > diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> > index f9d80c7bd9f16..0dcc1ec1a366f 100644
> > --- a/kernel/trace/ftrace.c
> > +++ b/kernel/trace/ftrace.c  
> [ ... ]
> > @@ -160,6 +162,10 @@ static inline void ftrace_ops_init(struct ftrace_ops *ops)
> >  {
> >  #ifdef CONFIG_DYNAMIC_FTRACE
> >  	if (!(ops->flags & FTRACE_OPS_FL_INITIALIZED)) {
> > +		guard(mutex)(&ops_mutex);
> > +		/* Could have been initialized before lock taken */
> > +		if (unlikely(ops->flags & FTRACE_OPS_FL_INITIALIZED))
> > +			return;
> >  		mutex_init(&ops->local_hash.regex_lock);
> >  		INIT_LIST_HEAD(&ops->subop_list);
> >  		ops->func_hash = &ops->local_hash;  
> 
> [Severity: High]
> Does this double-checked locking pattern introduce a memory ordering
> regression on weakly-ordered architectures?
> 
> The initial lock-free fast-path check !(ops->flags & FTRACE_OPS_FL_INITIALIZED)
> uses a plain load, while the subsequent completion signal
> ops->flags |= FTRACE_OPS_FL_INITIALIZED (which occurs just after the quoted
> lines) uses a plain store.
> 
> Without an acquire memory barrier on the load side and a release memory
> barrier on the store side, is it possible for memory accesses to be
> reordered by the CPU?

Yeah yeah, I figured this would be triggered. I'll update with
smp_load_acquire() and smp_store_release().

-- Steve