Re: [PATCH] nfs: Fix incorrect read-only status reporting in mountstats

[Li Lingfeng <lilingfeng3@huawei.com>]

Apologies, I'm not well-versed in CIFS, but cifs_show_stats lacks the
intended implementation. From my understanding, altering the parameter
list of the show_stats callback does not affect CIFS.

Thanks.

在 2025/3/12 6:08, Steve French 写道:
> Do you have a repro scenario example of this for cifs.ko? the
> suggested change seems plausible, but wondering about how to recreate
> the case you mention where "mounting may generate multiple
> superblocks" (are there DFS examples that this would help?)
>
> On Mon, Mar 3, 2025 at 8:18 AM Li Lingfeng <lilingfeng3@huawei.com> wrote:
>> In the process of read-only mounting of NFS, only the first generated
>> superblock carries the ro flag passed from the user space.
>> However, NFS mounting may generate multiple superblocks, and the last
>> generated superblock is the one actually used. This leads to a situation
>> where the superblock of a read-only NFS file system may not have the ro
>> flag. Therefore, using s_flags to determine whether an NFS file system is
>> read-only is incorrect.
>>
>> Use mnt_flags instead of s_flags to decide whether the file system state
>> displayed by the /proc/self/mountstats interface is read-only or not.
>>
>> Signed-off-by: Li Lingfeng <lilingfeng3@huawei.com>
>> ---
>>   fs/nfs/internal.h      |  2 +-
>>   fs/nfs/super.c         | 12 ++++++------
>>   fs/proc_namespace.c    |  2 +-
>>   fs/smb/client/cifsfs.c |  2 +-
>>   include/linux/fs.h     |  2 +-
>>   5 files changed, 10 insertions(+), 10 deletions(-)
>>
>> diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
>> index fae2c7ae4acc..14076fc9b1e8 100644
>> --- a/fs/nfs/internal.h
>> +++ b/fs/nfs/internal.h
>> @@ -565,7 +565,7 @@ int  nfs_statfs(struct dentry *, struct kstatfs *);
>>   int  nfs_show_options(struct seq_file *, struct dentry *);
>>   int  nfs_show_devname(struct seq_file *, struct dentry *);
>>   int  nfs_show_path(struct seq_file *, struct dentry *);
>> -int  nfs_show_stats(struct seq_file *, struct dentry *);
>> +int  nfs_show_stats(struct seq_file *, struct vfsmount *);
>>   int  nfs_reconfigure(struct fs_context *);
>>
>>   /* write.c */
>> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
>> index aeb715b4a690..62dfba216f7f 100644
>> --- a/fs/nfs/super.c
>> +++ b/fs/nfs/super.c
>> @@ -662,10 +662,10 @@ EXPORT_SYMBOL_GPL(nfs_show_path);
>>   /*
>>    * Present statistical information for this VFS mountpoint
>>    */
>> -int nfs_show_stats(struct seq_file *m, struct dentry *root)
>> +int nfs_show_stats(struct seq_file *m, struct vfsmount *mnt)
>>   {
>>          int i, cpu;
>> -       struct nfs_server *nfss = NFS_SB(root->d_sb);
>> +       struct nfs_server *nfss = NFS_SB(mnt->mnt_sb);
>>          struct rpc_auth *auth = nfss->client->cl_auth;
>>          struct nfs_iostats totals = { };
>>
>> @@ -675,10 +675,10 @@ int nfs_show_stats(struct seq_file *m, struct dentry *root)
>>           * Display all mount option settings
>>           */
>>          seq_puts(m, "\n\topts:\t");
>> -       seq_puts(m, sb_rdonly(root->d_sb) ? "ro" : "rw");
>> -       seq_puts(m, root->d_sb->s_flags & SB_SYNCHRONOUS ? ",sync" : "");
>> -       seq_puts(m, root->d_sb->s_flags & SB_NOATIME ? ",noatime" : "");
>> -       seq_puts(m, root->d_sb->s_flags & SB_NODIRATIME ? ",nodiratime" : "");
>> +       seq_puts(m, (mnt->mnt_flags & MNT_READONLY) ? "ro" : "rw");
>> +       seq_puts(m, mnt->mnt_sb->s_flags & SB_SYNCHRONOUS ? ",sync" : "");
>> +       seq_puts(m, mnt->mnt_sb->s_flags & SB_NOATIME ? ",noatime" : "");
>> +       seq_puts(m, mnt->mnt_sb->s_flags & SB_NODIRATIME ? ",nodiratime" : "");
>>          nfs_show_mount_options(m, nfss, 1);
>>
>>          seq_printf(m, "\n\tage:\t%lu", (jiffies - nfss->mount_time) / HZ);
>> diff --git a/fs/proc_namespace.c b/fs/proc_namespace.c
>> index e133b507ddf3..1310c655f33f 100644
>> --- a/fs/proc_namespace.c
>> +++ b/fs/proc_namespace.c
>> @@ -227,7 +227,7 @@ static int show_vfsstat(struct seq_file *m, struct vfsmount *mnt)
>>          /* optional statistics */
>>          if (sb->s_op->show_stats) {
>>                  seq_putc(m, ' ');
>> -               err = sb->s_op->show_stats(m, mnt_path.dentry);
>> +               err = sb->s_op->show_stats(m, mnt);
>>          }
>>
>>          seq_putc(m, '\n');
>> diff --git a/fs/smb/client/cifsfs.c b/fs/smb/client/cifsfs.c
>> index 6a3bd652d251..f3bf2c62e195 100644
>> --- a/fs/smb/client/cifsfs.c
>> +++ b/fs/smb/client/cifsfs.c
>> @@ -836,7 +836,7 @@ static int cifs_freeze(struct super_block *sb)
>>   }
>>
>>   #ifdef CONFIG_CIFS_STATS2
>> -static int cifs_show_stats(struct seq_file *s, struct dentry *root)
>> +static int cifs_show_stats(struct seq_file *s, struct vfsmount *mnt)
>>   {
>>          /* BB FIXME */
>>          return 0;
>> diff --git a/include/linux/fs.h b/include/linux/fs.h
>> index 2788df98080f..94ad8bdb409b 100644
>> --- a/include/linux/fs.h
>> +++ b/include/linux/fs.h
>> @@ -2308,7 +2308,7 @@ struct super_operations {
>>          int (*show_options)(struct seq_file *, struct dentry *);
>>          int (*show_devname)(struct seq_file *, struct dentry *);
>>          int (*show_path)(struct seq_file *, struct dentry *);
>> -       int (*show_stats)(struct seq_file *, struct dentry *);
>> +       int (*show_stats)(struct seq_file *, struct vfsmount *);
>>   #ifdef CONFIG_QUOTA
>>          ssize_t (*quota_read)(struct super_block *, int, char *, size_t, loff_t);
>>          ssize_t (*quota_write)(struct super_block *, int, const char *, size_t, loff_t);
>> --
>> 2.31.1
>>
>>
>