On Wed, 02 Sep 2026 00:33:43 +0000 [email protected] 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 <[email protected]> > > 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/[email protected]/ > > > 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
