Re: [PATCH] eventfs: Use children field for rcu head and add memory barriers

[Steven Rostedt]

On Sat, 8 Aug 2026 07:15:06 +0900
Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx> wrote:

 
> Looks good to me.
> 
> Reviewed-by: Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>
> 
> The critical issues Sashiko mentioned will be fixed by Shuangpeng's patch[1].

No ;-) The critical issues Sashiko mentioned was from a review of that
patch which stated:

  "This isn't a bug introduced by this patch, but does this still leave
   concurrent SRCU readers vulnerable to list pointer corruption?"

Thus Shuangpeng's patch doesn't address it.

I was in the process of applying Shuangpeng's patch and I look at the
Sashiko review and wrote this patch to fix that issue too. I plan on
applying both.


> [1] https://lore.kernel.org/all/20260806022719.375354-1-shuangpeng.kernel@xxxxxxxxx/
> 
> Thank you,
> 
> > Reported-by: Sashiko <sashiko-bot@xxxxxxxxxx>
> > Closes: https://sashiko.dev/#/patchset/20260806022719.375354-1-shuangpeng.kernel%40gmail.com
> > Signed-off-by: Steven Rostedt <rostedt@xxxxxxxxxxx>
> > ---
> >  fs/tracefs/event_inode.c | 24 ++++++++++++++++++++++++
> >  fs/tracefs/internal.h    |  4 ++--
> >  2 files changed, 26 insertions(+), 2 deletions(-)
> > 
> > diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c
> > index 39c7a34531e8..677c39b0d62f 100644
> > --- a/fs/tracefs/event_inode.c
> > +++ b/fs/tracefs/event_inode.c
> > @@ -124,6 +124,16 @@ static inline void put_ei(struct eventfs_inode *ei)
> >  static inline void free_ei(struct eventfs_inode *ei)
> >  {
> >  	if (ei) {
> > +		WARN_ON_ONCE(!list_empty(&ei->children));
> > +		/*
> > +		 * The ei should have no children if it is being freed.
> > +		 * The SRCU iteration has a smp_rmb() to make sure it
> > +		 * sees a child (that may have already been freed)
> > +		 * before it reads is_free. If is_free is set, it must
> > +		 * not use the child it acquired from ei->children, as
> > +		 * the list may be used for SRCU.
> > +		 */
> > +		smp_wmb();
> >  		ei->is_freed = 1;

And Sashiko found a bug here (from reading the comments). The smp_wmb() is
supposed to be *after* the ei->is_free = 1 !!!

I'll send a v2.

-- Steve

> >  		put_ei(ei);
> >  	}