namei: Use READ_ONCE() for d_flags in __follow_mount_rcu()

[PATCH] namei: Use READ_ONCE() for d_flags in __follow_mount_rcu()

Posted by Ze Tan 1 week, 3 days ago

__follow_mount_rcu() reads dentry->d_flags without holding d_lock.

Use READ_ONCE() for these reads to make the lockless access clear.

Signed-off-by: Ze Tan <tanze@kylinos.cn>
---
 fs/namei.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 19ce43c9a6e6..cc8e8d961d7e 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1687,7 +1687,7 @@ EXPORT_SYMBOL(follow_down);
 static bool __follow_mount_rcu(struct nameidata *nd, struct path *path)
 {
 	struct dentry *dentry = path->dentry;
-	unsigned int flags = dentry->d_flags;
+	unsigned int flags = READ_ONCE(dentry->d_flags);
 
 	if (unlikely(nd->flags & LOOKUP_NO_XDEV))
 		return false;
@@ -1701,7 +1701,7 @@ static bool __follow_mount_rcu(struct nameidata *nd, struct path *path)
 			int res = dentry->d_op->d_manage(path, true);
 			if (res)
 				return res == -EISDIR;
-			flags = dentry->d_flags;
+			flags = READ_ONCE(dentry->d_flags);
 		}
 
 		if (flags & DCACHE_MOUNTED) {
@@ -1711,7 +1711,7 @@ static bool __follow_mount_rcu(struct nameidata *nd, struct path *path)
 				dentry = path->dentry = mounted->mnt.mnt_root;
 				nd->state |= ND_JUMPED;
 				nd->next_seq = read_seqcount_begin(&dentry->d_seq);
-				flags = dentry->d_flags;
+				flags = READ_ONCE(dentry->d_flags);
 				// makes sure that non-RCU pathwalk could reach
 				// this state.
 				if (read_seqretry(&mount_lock, nd->m_seq))
-- 
2.43.0

Re: [PATCH] namei: Use READ_ONCE() for d_flags in __follow_mount_rcu()

Posted by Jan Kara 1 week, 3 days ago

On Wed 15-07-26 20:50:57, Ze Tan wrote:
> __follow_mount_rcu() reads dentry->d_flags without holding d_lock.
> 
> Use READ_ONCE() for these reads to make the lockless access clear.
> 
> Signed-off-by: Ze Tan <tanze@kylinos.cn>

Hum, so we are somewhat sloppy with the access to d_flags. Strictly
speaking we should be using WRITE_ONCE when updating them (and we do in
some places but not in others) and READ_ONCE when accessing them. OTOH the
only practical problem is KCSAN complaining, the problems that the compiler
could somehow tear stores to the d_flags are IMHO theoretical.

Having two accesses without READ_ONCE more or less doesn't really matter in
my opinion. Cleaning up all the d_flags accesses would make some sense but
I'm not sure it's worth the churn for mostly theoretical issues (I believe
Al or Christian was against it if my memory serves well).

								Honza

> ---
>  fs/namei.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/namei.c b/fs/namei.c
> index 19ce43c9a6e6..cc8e8d961d7e 100644
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -1687,7 +1687,7 @@ EXPORT_SYMBOL(follow_down);
>  static bool __follow_mount_rcu(struct nameidata *nd, struct path *path)
>  {
>  	struct dentry *dentry = path->dentry;
> -	unsigned int flags = dentry->d_flags;
> +	unsigned int flags = READ_ONCE(dentry->d_flags);
>  
>  	if (unlikely(nd->flags & LOOKUP_NO_XDEV))
>  		return false;
> @@ -1701,7 +1701,7 @@ static bool __follow_mount_rcu(struct nameidata *nd, struct path *path)
>  			int res = dentry->d_op->d_manage(path, true);
>  			if (res)
>  				return res == -EISDIR;
> -			flags = dentry->d_flags;
> +			flags = READ_ONCE(dentry->d_flags);
>  		}
>  
>  		if (flags & DCACHE_MOUNTED) {
> @@ -1711,7 +1711,7 @@ static bool __follow_mount_rcu(struct nameidata *nd, struct path *path)
>  				dentry = path->dentry = mounted->mnt.mnt_root;
>  				nd->state |= ND_JUMPED;
>  				nd->next_seq = read_seqcount_begin(&dentry->d_seq);
> -				flags = dentry->d_flags;
> +				flags = READ_ONCE(dentry->d_flags);
>  				// makes sure that non-RCU pathwalk could reach
>  				// this state.
>  				if (read_seqretry(&mount_lock, nd->m_seq))
> -- 
> 2.43.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH] namei: Use READ_ONCE() for d_flags in __follow_mount_rcu()

Posted by Christian Brauner 3 days, 7 hours ago

On Wed, Jul 15, 2026 at 05:51:45PM +0200, Jan Kara wrote:
> On Wed 15-07-26 20:50:57, Ze Tan wrote:
> > __follow_mount_rcu() reads dentry->d_flags without holding d_lock.
> > 
> > Use READ_ONCE() for these reads to make the lockless access clear.
> > 
> > Signed-off-by: Ze Tan <tanze@kylinos.cn>
> 
> Hum, so we are somewhat sloppy with the access to d_flags. Strictly
> speaking we should be using WRITE_ONCE when updating them (and we do in
> some places but not in others) and READ_ONCE when accessing them. OTOH the
> only practical problem is KCSAN complaining, the problems that the compiler
> could somehow tear stores to the d_flags are IMHO theoretical.
> 
> Having two accesses without READ_ONCE more or less doesn't really matter in
> my opinion. Cleaning up all the d_flags accesses would make some sense but
> I'm not sure it's worth the churn for mostly theoretical issues (I believe
> Al or Christian was against it if my memory serves well).

I don't care for the churn but personally I wouldn't oppose the change.