Re: [PATCH] nfs: remove SB_RDONLY when remounting nfs

[Li Lingfeng <lilingfeng3@huawei.com>]

在 2025/3/3 11:33, Zhang Yi 写道:
> On 2025/2/21 16:26, Li Lingfeng wrote:
>> In some scenarios, when mounting NFS, more than one superblock may be
>> created. The final superblock used is the last one created, but only the
>> first superblock carries the ro flag passed from user space. If a ro flag
>> is added to the superblock via remount, it will trigger the issue
>> described in Link[1].
>>
>> Link[2] attempted to address this by marking the superblock as ro during
>> the initial mount. However, this introduced a new problem in scenarios
>> where multiple mount points share the same superblock:
>> [root@a ~]# mount /dev/sdb /mnt/sdb
>> [root@a ~]# echo "/mnt/sdb *(rw,no_root_squash)" > /etc/exports
>> [root@a ~]# echo "/mnt/sdb/test_dir2 *(ro,no_root_squash)" >> /etc/exports
>> [root@a ~]# systemctl restart nfs-server
>> [root@a ~]# mount -t nfs -o rw 127.0.0.1:/mnt/sdb/test_dir1 /mnt/test_mp1
>> [root@a ~]# mount | grep nfs4
>> 127.0.0.1:/mnt/sdb/test_dir1 on /mnt/test_mp1 type nfs4 (rw,relatime,...
>> [root@a ~]# mount -t nfs -o ro 127.0.0.1:/mnt/sdb/test_dir2 /mnt/test_mp2
>> [root@a ~]# mount | grep nfs4
>> 127.0.0.1:/mnt/sdb/test_dir1 on /mnt/test_mp1 type nfs4 (ro,relatime,...
>> 127.0.0.1:/mnt/sdb/test_dir2 on /mnt/test_mp2 type nfs4 (ro,relatime,...
>> [root@a ~]#
>>
>> When mounting the second NFS, the shared superblock is marked as ro,
>> causing the previous NFS mount to become read-only.
>>
>> To resolve both issues, the ro flag is no longer applied to the superblock
>> during remount. Instead, the ro flag on the mount is used to control
>> whether the mount point is read-only.
>>
>> Fixes: 281cad46b34d ("NFS: Create a submount rpc_op")
>> Link[1]: https://lore.kernel.org/all/20240604112636.236517-3-lilingfeng@huaweicloud.com/
>> Link[2]: https://lore.kernel.org/all/20241130035818.1459775-1-lilingfeng3@huawei.com/
>> Signed-off-by: Li Lingfeng <lilingfeng3@huawei.com>
>> ---
>>   fs/nfs/super.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
>> index aeb715b4a690..f08e1d7fb179 100644
>> --- a/fs/nfs/super.c
>> +++ b/fs/nfs/super.c
>> @@ -1047,6 +1047,7 @@ int nfs_reconfigure(struct fs_context *fc)
>>   
>>   	sync_filesystem(sb);
>>   
>> +	fc->sb_flags &= ~SB_RDONLY;
> What about change sb_flags_mask instead? Something like below,
>
> 	fc->sb_flags_mask &= ~SB_RDONLY;
>
> and I'd also suggested to add a comment to explain the reason in detail.
>
> Thanks,
> Yi.

Thanks for your advice.  I will send v2 soon.