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

Steven Rostedt <[email protected]>
Newsgroups org.kernel.vger.linux-trace-kernel,org.kernel.vger.linux-kernel
Message-ID <20260808093755.342fb36b@robin>
On Fri, 7 Aug 2026 20:44:41 -0400
Steven Rostedt <[email protected]> wrote:

> diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c
> index 39c7a34531e8..7d2431f2bb08 100644
> --- a/fs/tracefs/event_inode.c
> +++ b/fs/tracefs/event_inode.c
> @@ -124,7 +124,17 @@ static inline void put_ei(struct eventfs_inode *ei)
>  static inline void free_ei(struct eventfs_inode *ei)
>  {
>  	if (ei) {
> +		/* The ei should have no children if it is being freed. */
> +		WARN_ON_ONCE(!list_empty(&ei->children));
>  		ei->is_freed = 1;
> +		/*
> +		 * 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();
>  		put_ei(ei);
>  	}
>  }
> @@ -627,6 +637,20 @@ static int eventfs_iterate(struct file *file, struct dir_context *ctx)
>  	list_for_each_entry_srcu(ei_child, &ei->children, list,
>  				 srcu_read_lock_held(&eventfs_srcu)) {
>  
> +		/*
> +		 * If the ei is being freed, then the ei->children may be
> +		 * being used as the rcu list, which means the next element
> +		 * may be garbage. The ei->is_free is set before switching
> +		 * the ei->children over to ei->rcu. The read memory barrier
> +		 * here makes sure the ei_child is read before is_free is
> +		 * updated.
> +		 *
> +		 * Matches the smp_wmb() in put_ei()

Bah! Now Sashiko is complaining that the wmb is in free_ei() and not
put_ei(). Yes it is correct, but I wish it pointed this out last time
:-p

I'll send a v3.

-- Steve


> +		 */
> +		smp_rmb();
> +		if (ei->is_freed)
> +			return -EINVAL;
> +
>  		if (c > 0) {
>  			c--;
>  			continue;
> diff --git a/fs/tracefs/internal.h b/fs/tracefs/internal.h
> index a4a7f8431aff..c61481d04c8e 100644
> --- a/fs/tracefs/internal.h
> +++ b/fs/tracefs/internal.h
> @@ -46,11 +46,11 @@ struct eventfs_attr {
>   * @ino:	The saved inode number
>   */
>  struct eventfs_inode {
> +	struct list_head	list;
>  	union {
> -		struct list_head	list;
> +		struct list_head	children;
>  		struct rcu_head		rcu;
>  	};
> -	struct list_head		children;
>  	const struct eventfs_entry	*entries;
>  	const char			*name;
>  	struct eventfs_attr		*entry_attrs;
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.