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? If Thread A completes the initialization and writes the flag, could Thread B bypass the mutex by observing the initialized flag as true, but still read a stale uninitialized or NULL value for ops->func_hash? If Thread B then immediately dereferences ops->func_hash, for instance in ftrace_startup_subops(): kernel/trace/ftrace.c:ftrace_startup_subops() { ... if (!ops->func_hash->filter_hash) ... } Would this result in an invalid memory access and a kernel panic? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/20260901202020.09a1119a@robin?part=1
