Re: [PATCH RFC 03/12] nfs: tell the VFS when two names can be one entry on the server

[NeilBrown]

On Fri, 30 Oct 2026, Jonathan Flynn wrote:
> A server that reports itself case-insensitive, or as not preserving
> case, can make one entry of two names that the client caches apart,
> because the client compares names byte for byte.  Mark the filesystem
> on such a server with SB_I_NAMES_COLLIDE, from "vfs: allow the parent
> lock to be shared for O_CREAT", so that the VFS keeps the exclusive
> directory lock for creates there: the per-name exclusion that replaces
> the lock for a create made with it held shared covers one byte string,
> not the names the server treats as equal.  The client learns this when
> it reads the server's capabilities, which it does again on remount, on
> migration and when a later mount reuses the filesystem, and it updates
> the mark each time.
> 
> This is a conservative fallback.  A server that declares itself
> case-insensitive should itself treat the two names as one, but one that
> did not was seen making two files of one name when such creates arrived
> together.  No filesystem relies on the mark yet, so nothing changes.

We don't normally work around server bugs in the client.  We tell the
server maintainers to fix their server.

But we *certainly* don't work around them in the VFS with
SB_I_NAMES_COLLIDE.  That sort of fs behaviour might be tolerable for
files but it is a real problem for directories!

Once we move directory locking into the filesystem, the NFS client code
might choose to add extra locking for servers that might be problematic.
It would then be entirely in the NFS code then, and up to the NFS
maintainers to decide what to do.
But this logic does not, I think, belong in the VFS layer at all.

This just confirms me in my preference to continue on the path I have
planned.

Hopefully next week I'll put together an RFC for the next steps, and
we pursue the prep patches that haven't been acked yet.

Thanks,
NeilBrown


> 
> Signed-off-by: Jonathan Flynn <jonathan.flynn@xxxxxxxxxxxxxxx>
> ---
>  fs/nfs/client.c   |  1 +
>  fs/nfs/internal.h | 35 +++++++++++++++++++++++++++++++++++
>  fs/nfs/nfs4proc.c |  1 +
>  fs/nfs/super.c    |  2 ++
>  4 files changed, 39 insertions(+)
> 
> diff --git a/fs/nfs/client.c b/fs/nfs/client.c
> index ea99b3c105..30206b309c 100644
> --- a/fs/nfs/client.c
> +++ b/fs/nfs/client.c
> @@ -975,6 +975,7 @@ static int nfs_probe_fsinfo(struct nfs_server *server, struct nfs_fh *mntfh, str
>  			server->caps &= ~(NFS_CAP_CASE_INSENSITIVE |
>  					  NFS_CAP_CASE_NONPRESERVING);
>  		}
> +		nfs_server_publish_name_caps(server);
>  	}
>  
>  	if (clp->rpc_ops->discover_trunking != NULL &&
> diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
> index 5af6a032f1..34b8aff4e0 100644
> --- a/fs/nfs/internal.h
> +++ b/fs/nfs/internal.h
> @@ -224,6 +224,41 @@ int nfs_init_server_rpcclient(struct nfs_server *, const struct rpc_timeout *t,
>  struct nfs_server *nfs_alloc_server(void);
>  void nfs_server_copy_userdata(struct nfs_server *, struct nfs_server *);
>  
> +/*
> + * Tell the VFS whether this server can make one directory entry of two names
> + * that differ as byte strings.  NFS installs no ->d_hash or ->d_compare, so
> + * the dcache compares names byte for byte, while a server that is
> + * case-insensitive, or does not preserve case, makes "foo" and "FOO" one
> + * entry.  On such a server SB_I_NAMES_COLLIDE keeps the parent of an O_CREAT
> + * open locked exclusive; see atomic_open_create_shared() in fs/namei.c.
> + *
> + * server->caps is derived again on every probe of the server: at mount, at
> + * remount, on migration, and when a later mount reuses this superblock
> + * (nfs4_proc_get_root()).  So this is called wherever the case bits of
> + * server->caps are written, once they have their final value, and when the
> + * superblock is set up, because the first probe runs before it exists.  The
> + * value is what the probed file handle reported, normally the mount root's;
> + * the homogeneous attribute is not fetched, so a server whose answer differs
> + * below the root is described by the root.  Migration runs the probe from
> + * the state manager without ->s_umount, so the update is atomic, and it is
> + * made only when the value changes.  Two probes of one server at once can
> + * leave the bit as the earlier of them derived it; that needs the server's
> + * answer to change between them, and server->caps has the same race.
> + */
> +static inline void nfs_server_publish_name_caps(struct nfs_server *server)
> +{
> +	struct super_block *sb = server->super;
> +	unsigned long bits = 0;
> +
> +	if (!sb)
> +		return;
> +	if (READ_ONCE(server->caps) & (NFS_CAP_CASE_INSENSITIVE |
> +				       NFS_CAP_CASE_NONPRESERVING))
> +		bits = SB_I_NAMES_COLLIDE;
> +	if ((READ_ONCE(sb->s_iflags) & SB_I_NAMES_COLLIDE) != bits)
> +		set_mask_bits(&sb->s_iflags, SB_I_NAMES_COLLIDE, bits);
> +}
> +
>  extern void nfs_put_client(struct nfs_client *);
>  extern void nfs_free_client(struct nfs_client *);
>  void nfs_cb_idr_remove(struct nfs_client *clp);
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 113c199a6f..fac8450b65 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -4020,6 +4020,7 @@ static int _nfs4_server_capabilities(struct nfs_server *server, struct nfs_fh *f
>  		if (nfs4_server_delegtime_capable(&res))
>  			caps |= NFS_CAP_DELEGTIME;
>  		WRITE_ONCE(server->caps, caps);
> +		nfs_server_publish_name_caps(server);
>  
>  		memcpy(server->cache_consistency_bitmask, res.attr_bitmask, sizeof(server->cache_consistency_bitmask));
>  		server->cache_consistency_bitmask[0] &= FATTR4_WORD0_CHANGE|FATTR4_WORD0_SIZE;
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index 23292680ad..e70cef88fb 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -1082,6 +1082,8 @@ static void nfs_fill_super(struct super_block *sb, struct nfs_fs_context *ctx)
>  			sb->s_export_op = &nfs_export_ops;
>  		break;
>  	}
> +	/* The first probe of the caps ran before this superblock existed */
> +	nfs_server_publish_name_caps(server);
>  
>  	sb->s_magic = NFS_SUPER_MAGIC;
>  
> -- 
> 2.43.0
> 
>