[PATCH 0/4] nfsd: fix some comments and code cleanup

[PATCH 0/4] nfsd: fix some comments and code cleanup

[Li Lingfeng]

Li Lingfeng (4):
  nfsd: fix the comment of nfs_get_root
  nfsd: fix some spelling errors in comments
  nfsd: remove the redundant blank line
  nfsd: remove unused parameter of nfsd_file_mark_find_or_create

 fs/nfs/getroot.c    | 2 +-
 fs/nfsd/filecache.c | 4 ++--
 fs/nfsd/nfs4proc.c  | 1 -
 fs/nfsd/nfs4state.c | 4 ++--
 4 files changed, 5 insertions(+), 6 deletions(-)

-- 
2.31.1

[PATCH 1/4] nfsd: fix the comment of nfs_get_root

[Li Lingfeng]

The comment for nfs_get_root() needs to be updated as it would also be
used by NFS4 as follows:
@x[
    nfs_get_root+1
    nfs_get_tree_common+1819
    nfs_get_tree+2594
    vfs_get_tree+73
    fc_mount+23
    do_nfs4_mount+498
    nfs4_try_get_tree+134
    nfs_get_tree+2562
    vfs_get_tree+73
    path_mount+2776
    do_mount+226
    __se_sys_mount+343
    __x64_sys_mount+106
    do_syscall_64+69
    entry_SYSCALL_64_after_hwframe+97
, mount.nfs4]: 1

Signed-off-by: Li Lingfeng <lilingfeng3@huawei.com>
---
 fs/nfs/getroot.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/nfs/getroot.c b/fs/nfs/getroot.c
index 11ff2b2e060f..f13d25d95b85 100644
--- a/fs/nfs/getroot.c
+++ b/fs/nfs/getroot.c
@@ -62,7 +62,7 @@ static int nfs_superblock_set_dummy_root(struct super_block *sb, struct inode *i
 }
 
 /*
- * get an NFS2/NFS3 root dentry from the root filehandle
+ * get a root dentry from the root filehandle
  */
 int nfs_get_root(struct super_block *s, struct fs_context *fc)
 {
-- 
2.31.1

Re: [PATCH 1/4] nfsd: fix the comment of nfs_get_root

[Jeff Layton]

On Fri, 2024-08-23 at 15:00 +0800, Li Lingfeng wrote:
> The comment for nfs_get_root() needs to be updated as it would also be
> used by NFS4 as follows:
> @x[
>     nfs_get_root+1
>     nfs_get_tree_common+1819
>     nfs_get_tree+2594
>     vfs_get_tree+73
>     fc_mount+23
>     do_nfs4_mount+498
>     nfs4_try_get_tree+134
>     nfs_get_tree+2562
>     vfs_get_tree+73
>     path_mount+2776
>     do_mount+226
>     __se_sys_mount+343
>     __x64_sys_mount+106
>     do_syscall_64+69
>     entry_SYSCALL_64_after_hwframe+97
> , mount.nfs4]: 1
> 
> Signed-off-by: Li Lingfeng <lilingfeng3@huawei.com>
> ---
>  fs/nfs/getroot.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/nfs/getroot.c b/fs/nfs/getroot.c
> index 11ff2b2e060f..f13d25d95b85 100644
> --- a/fs/nfs/getroot.c
> +++ b/fs/nfs/getroot.c
> @@ -62,7 +62,7 @@ static int nfs_superblock_set_dummy_root(struct super_block *sb, struct inode *i
>  }
>  
>  /*
> - * get an NFS2/NFS3 root dentry from the root filehandle
> + * get a root dentry from the root filehandle
>   */
>  int nfs_get_root(struct super_block *s, struct fs_context *fc)
>  {

Note that this is a NFS client patch, not a nfsd patch like the title
suggests.

Acked-by: Jeff Layton <jlayton@kernel.org>

[PATCH 2/4] nfsd: fix some spelling errors in comments

[Li Lingfeng]

Fix spelling errors in comments of nfsd4_release_lockowner and
nfs4_set_delegation.

Signed-off-by: Li Lingfeng <lilingfeng3@huawei.com>
---
 fs/nfsd/nfs4state.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index a20c2c9d7d45..66a0c76850f3 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -5856,7 +5856,7 @@ nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
 
 	/*
 	 * Now that the deleg is set, check again to ensure that nothing
-	 * raced in and changed the mode while we weren't lookng.
+	 * raced in and changed the mode while we weren't looking.
 	 */
 	status = nfsd4_verify_setuid_write(open, fp->fi_deleg_file);
 	if (status)
@@ -8335,7 +8335,7 @@ check_for_locks(struct nfs4_file *fp, struct nfs4_lockowner *lowner)
  * @cstate: NFSv4 COMPOUND state
  * @u: RELEASE_LOCKOWNER arguments
  *
- * Check if theree are any locks still held and if not - free the lockowner
+ * Check if there are any locks still held and if not - free the lockowner
  * and any lock state that is owned.
  *
  * Return values:
-- 
2.31.1

Re: [PATCH 2/4] nfsd: fix some spelling errors in comments

[Jeff Layton]

On Fri, 2024-08-23 at 15:00 +0800, Li Lingfeng wrote:
> Fix spelling errors in comments of nfsd4_release_lockowner and
> nfs4_set_delegation.
> 
> Signed-off-by: Li Lingfeng <lilingfeng3@huawei.com>
> ---
>  fs/nfsd/nfs4state.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index a20c2c9d7d45..66a0c76850f3 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -5856,7 +5856,7 @@ nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
>  
>  	/*
>  	 * Now that the deleg is set, check again to ensure that nothing
> -	 * raced in and changed the mode while we weren't lookng.
> +	 * raced in and changed the mode while we weren't looking.
>  	 */
>  	status = nfsd4_verify_setuid_write(open, fp->fi_deleg_file);
>  	if (status)
> @@ -8335,7 +8335,7 @@ check_for_locks(struct nfs4_file *fp, struct nfs4_lockowner *lowner)
>   * @cstate: NFSv4 COMPOUND state
>   * @u: RELEASE_LOCKOWNER arguments
>   *
> - * Check if theree are any locks still held and if not - free the lockowner
> + * Check if there are any locks still held and if not - free the lockowner

This is probably better grammatically:

    Check if there are any locks still held and if not, free the lockowner

>   * and any lock state that is owned.
>   *
>   * Return values:

-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH 2/4] nfsd: fix some spelling errors in comments

[Li Lingfeng]

在 2024/8/23 20:07, Jeff Layton 写道:
> On Fri, 2024-08-23 at 15:00 +0800, Li Lingfeng wrote:
>> Fix spelling errors in comments of nfsd4_release_lockowner and
>> nfs4_set_delegation.
>>
>> Signed-off-by: Li Lingfeng <lilingfeng3@huawei.com>
>> ---
>>   fs/nfsd/nfs4state.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>> index a20c2c9d7d45..66a0c76850f3 100644
>> --- a/fs/nfsd/nfs4state.c
>> +++ b/fs/nfsd/nfs4state.c
>> @@ -5856,7 +5856,7 @@ nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
>>   
>>   	/*
>>   	 * Now that the deleg is set, check again to ensure that nothing
>> -	 * raced in and changed the mode while we weren't lookng.
>> +	 * raced in and changed the mode while we weren't looking.
>>   	 */
>>   	status = nfsd4_verify_setuid_write(open, fp->fi_deleg_file);
>>   	if (status)
>> @@ -8335,7 +8335,7 @@ check_for_locks(struct nfs4_file *fp, struct nfs4_lockowner *lowner)
>>    * @cstate: NFSv4 COMPOUND state
>>    * @u: RELEASE_LOCKOWNER arguments
>>    *
>> - * Check if theree are any locks still held and if not - free the lockowner
>> + * Check if there are any locks still held and if not - free the lockowner
> This is probably better grammatically:
>
>      Check if there are any locks still held and if not, free the lockowner
Thanks for your advice, it does look better.
>>    * and any lock state that is owned.
>>    *
>>    * Return values:

[PATCH 3/4] nfsd: remove the redundant blank line

[Li Lingfeng]

Just remove the redundant blank line in do_open_permission. No function
change.

Signed-off-by: Li Lingfeng <lilingfeng3@huawei.com>
---
 fs/nfsd/nfs4proc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index 2e39cf2e502a..0068db924060 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -141,7 +141,6 @@ fh_dup2(struct svc_fh *dst, struct svc_fh *src)
 static __be32
 do_open_permission(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nfsd4_open *open, int accmode)
 {
-
 	if (open->op_truncate &&
 		!(open->op_share_access & NFS4_SHARE_ACCESS_WRITE))
 		return nfserr_inval;
-- 
2.31.1

Re: [PATCH 3/4] nfsd: remove the redundant blank line

[Jeff Layton]

On Fri, 2024-08-23 at 15:00 +0800, Li Lingfeng wrote:
> Just remove the redundant blank line in do_open_permission. No function
> change.
> 
> Signed-off-by: Li Lingfeng <lilingfeng3@huawei.com>
> ---
>  fs/nfsd/nfs4proc.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index 2e39cf2e502a..0068db924060 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -141,7 +141,6 @@ fh_dup2(struct svc_fh *dst, struct svc_fh *src)
>  static __be32
>  do_open_permission(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nfsd4_open *open, int accmode)
>  {
> -
>  	if (open->op_truncate &&
>  		!(open->op_share_access & NFS4_SHARE_ACCESS_WRITE))
>  		return nfserr_inval;

NACK. In general, we don't take whitespace patches. They just make
backporting harder later if you have to do it. If you have a material
change to make in this area, then go ahead and fix the whitespace, but
otherwise don't bother.
-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH 3/4] nfsd: remove the redundant blank line

[Li Lingfeng]

Thank you for your response. I appreciate you taking the time to address 
my concerns. Moving forward, I will be more mindful and refrain from 
submitting patches like this.

在 2024/8/23 20:08, Jeff Layton 写道:
> On Fri, 2024-08-23 at 15:00 +0800, Li Lingfeng wrote:
>> Just remove the redundant blank line in do_open_permission. No function
>> change.
>>
>> Signed-off-by: Li Lingfeng <lilingfeng3@huawei.com>
>> ---
>>   fs/nfsd/nfs4proc.c | 1 -
>>   1 file changed, 1 deletion(-)
>>
>> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
>> index 2e39cf2e502a..0068db924060 100644
>> --- a/fs/nfsd/nfs4proc.c
>> +++ b/fs/nfsd/nfs4proc.c
>> @@ -141,7 +141,6 @@ fh_dup2(struct svc_fh *dst, struct svc_fh *src)
>>   static __be32
>>   do_open_permission(struct svc_rqst *rqstp, struct svc_fh *current_fh, struct nfsd4_open *open, int accmode)
>>   {
>> -
>>   	if (open->op_truncate &&
>>   		!(open->op_share_access & NFS4_SHARE_ACCESS_WRITE))
>>   		return nfserr_inval;
> NACK. In general, we don't take whitespace patches. They just make
> backporting harder later if you have to do it. If you have a material
> change to make in this area, then go ahead and fix the whitespace, but
> otherwise don't bother.

[PATCH 4/4] nfsd: remove unused parameter of nfsd_file_mark_find_or_create

[Li Lingfeng]

Commit 427f5f83a319 ("NFSD: Ensure nf_inode is never dereferenced") passes
inode directly to nfsd_file_mark_find_or_create instead of getting it from
nf, so there is no need to pass nf.

Signed-off-by: Li Lingfeng <lilingfeng3@huawei.com>
---
 fs/nfsd/filecache.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c
index f4704f5d4086..376ec62e7794 100644
--- a/fs/nfsd/filecache.c
+++ b/fs/nfsd/filecache.c
@@ -151,7 +151,7 @@ nfsd_file_mark_put(struct nfsd_file_mark *nfm)
 }
 
 static struct nfsd_file_mark *
-nfsd_file_mark_find_or_create(struct nfsd_file *nf, struct inode *inode)
+nfsd_file_mark_find_or_create(struct inode *inode)
 {
 	int			err;
 	struct fsnotify_mark	*mark;
@@ -1074,7 +1074,7 @@ nfsd_file_do_acquire(struct svc_rqst *rqstp, struct svc_fh *fhp,
 
 open_file:
 	trace_nfsd_file_alloc(nf);
-	nf->nf_mark = nfsd_file_mark_find_or_create(nf, inode);
+	nf->nf_mark = nfsd_file_mark_find_or_create(inode);
 	if (nf->nf_mark) {
 		if (file) {
 			get_file(file);
-- 
2.31.1

Re: [PATCH 4/4] nfsd: remove unused parameter of nfsd_file_mark_find_or_create

[Jeff Layton]

On Fri, 2024-08-23 at 15:00 +0800, Li Lingfeng wrote:
> Commit 427f5f83a319 ("NFSD: Ensure nf_inode is never dereferenced") passes
> inode directly to nfsd_file_mark_find_or_create instead of getting it from
> nf, so there is no need to pass nf.
> 
> Signed-off-by: Li Lingfeng <lilingfeng3@huawei.com>
> ---
>  fs/nfsd/filecache.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/nfsd/filecache.c b/fs/nfsd/filecache.c
> index f4704f5d4086..376ec62e7794 100644
> --- a/fs/nfsd/filecache.c
> +++ b/fs/nfsd/filecache.c
> @@ -151,7 +151,7 @@ nfsd_file_mark_put(struct nfsd_file_mark *nfm)
>  }
>  
>  static struct nfsd_file_mark *
> -nfsd_file_mark_find_or_create(struct nfsd_file *nf, struct inode *inode)
> +nfsd_file_mark_find_or_create(struct inode *inode)
>  {
>  	int			err;
>  	struct fsnotify_mark	*mark;
> @@ -1074,7 +1074,7 @@ nfsd_file_do_acquire(struct svc_rqst *rqstp, struct svc_fh *fhp,
>  
>  open_file:
>  	trace_nfsd_file_alloc(nf);
> -	nf->nf_mark = nfsd_file_mark_find_or_create(nf, inode);
> +	nf->nf_mark = nfsd_file_mark_find_or_create(inode);
>  	if (nf->nf_mark) {
>  		if (file) {
>  			get_file(file);

Nice cleanup.

Reviewed-by: Jeff Layton <jlayton@kernel.org>

Re: [PATCH 4/4] nfsd: remove unused parameter of nfsd_file_mark_find_or_create

[Chuck Lever]

On Fri, Aug 23, 2024 at 03:00:49PM +0800, Li Lingfeng wrote:
> Commit 427f5f83a319 ("NFSD: Ensure nf_inode is never dereferenced") passes
> inode directly to nfsd_file_mark_find_or_create instead of getting it from
> nf, so there is no need to pass nf.
> 
> [...]

Applied to nfsd-next for v6.12, thanks!

[4/4] nfsd: remove unused parameter of nfsd_file_mark_find_or_create
      commit: c78a662dfe4ea208e05c7ba78949a239e31c360d

-- 
Chuck Lever