Hi Philip,
In the future, if you have questions about my patches, please include me
on the CC list.
This way, I won't accidentally miss the discussion and can address any
concerns promptly. :)
Here's my reply:
While investigating the non-root mount behavior differences between NFSv3
and NFSv4, I observed some characteristics that might explain the issue.
For NFSv3:
1) A single superblock is created for the initial mount.
2) When mounted read-only, this superblock carries the SB_RDONLY flag.
3) Before commit 52cb7f8f1778 ("nfs: ignore SB_RDONLY when mounting nfs"):
Subsequent rw mounts would not share the existing ro superblock due to
flag mismatch, creating a new superblock without SB_RDONLY.
After the commit:
The SB_RDONLY flag is ignored during superblock comparison, and this
leads
to sharing the existing superblock even for rw mounts.
Ultimately results in write operations being rejected at the VFS layer.
do_new_mount
vfs_get_tree
nfs_get_tree
nfs_fs_context_validate
nfs_parse_source
nfs_try_get_tree
nfs_try_mount_request
nfs_request_mount
nfs_mount
// "NFS: sending MNT request for localhost:/export/stuff"
nfs_get_tree_common
sget_fc // the only sb, with "ro"
For NFSv4:
1) Multiple superblocks are created and the last one will be kept.
2) The actually used superblock for ro mounts doesn't carry SB_RDONLY flag.
Therefore, commit 52cb7f8f1778 doesn't affect NFSv4 mounts.
do_new_mount
vfs_get_tree
nfs4_try_get_tree
do_nfs4_mount
fc_mount
vfs_get_tree
nfs_get_tree_common
sget_fc // sb for root, with "ro", will be free
mount_subtree
vfs_path_lookup
filename_lookup
path_lookupat
link_path_walk
step_into
__traverse_mounts
nfs_d_automount
nfs4_submount
nfs_do_submount
vfs_get_tree
nfs_get_tree_common
sget_fc // sb for subtree, without "ro"
This fundamental difference in superblock management between protocol
versions
explains why the regression only manifests in NFSv3 use cases.
Perhaps avoiding the SB_RDONLY flag on the superblock during the initial
mount
would be a good idea.
diff --git a/fs/nfs/fs_context.c b/fs/nfs/fs_context.c
index b069385eea17..67565e1fd3f0 100644
--- a/fs/nfs/fs_context.c
+++ b/fs/nfs/fs_context.c
@@ -1514,6 +1514,9 @@ static int nfs_get_tree(struct fs_context *fc)
if (err)
return err;
+
+ fc->sb_flags &= ~SB_RDONLY;
+
if (!ctx->internal)
return ctx->nfs_mod->rpc_ops->try_get_tree(fc);
else
I'll have another round of discussions with the team on the solution
approach
and perform more thorough testing.
Thanks.
在 2025/1/8 3:30, Philip Rowlands 写道:
Kernel 6.1.120 / 6.12.2 and others recently changed the definition of NFS_SB_MASK which is used by nfs_compare_super to find existing superblocks.
nfs: ignore SB_RDONLY when mounting nfs
[ Upstream commit 52cb7f8f177878b4f22397b9c4d2c8f743766be3 ]
As a result, we now see mount options shared at the superblock level which should not be shared, such as ro.
Steps to reproduce on Fedora 40:
mkdir -p /export/{stuff,things}/dir{1,2,3,4}
echo '/export/stuff *(rw)' >> /etc/exports
echo '/export/things *(rw)' >> /etc/exports
systemctl restart nfs-server
mount -t nfs -o ro,vers=3 localhost:/export/stuff /mnt/stuff
mount -t nfs -o rw,vers=3 localhost:/export/things /mnt/things
grep -w nfs /proc/mounts
# note that both mountpoints are ro, despite the explicit ro/rw options
# reversing the order of mounts gives a different result
I don’t fully understand the previous problem report regarding repeated mounts with different options to the same mountpoint, but by rolling back the specific patch, we no longer see the above unintended-ro issue.
Is there a reason that NFSv4 mounts don’t seem to trigger the bug?
Cheers,
Phil
