[PATCH 0/5] nfsd: some small cleanup patches

[PATCH 0/5] nfsd: some small cleanup patches

[Jeff Layton]

This is a pile of small patches that I've been collecting. None are
terribly critical.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
Jeff Layton (5):
      nfsd: reorganize struct nfs4_delegation for better packing
      nfsd: remove unneeded forward declaration of nfsd4_mark_cb_fault()
      nfsd: remove obsolete comment from nfs4_alloc_stid
      nfsd: clean up if statement in nfsd4_close_open_stateid()
      nfsd: use a long for the count in nfsd4_state_shrinker_count()

 fs/nfsd/nfs4callback.c |  2 --
 fs/nfsd/nfs4state.c    | 20 +++++---------------
 fs/nfsd/state.h        |  2 +-
 3 files changed, 6 insertions(+), 18 deletions(-)
---
base-commit: 7dc86d35a5f8a7ac24b53792c704b101e5041842
change-id: 20250213-nfsd-cleanup-b4bebb80472d

Best regards,
-- 
Jeff Layton <jlayton@kernel.org>

[PATCH 1/5] nfsd: reorganize struct nfs4_delegation for better packing

[Jeff Layton]

Move dl_type field above dl_time, which shaves 8 bytes off this struct.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/state.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 0537f5c001a4d581433ca5aca235188fe68b14e5..625a77107d29121d630d456183e01c9f903f758a 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -215,8 +215,8 @@ struct nfs4_delegation {
 	struct list_head	dl_perclnt;
 	struct list_head	dl_recall_lru;  /* delegation recalled */
 	struct nfs4_clnt_odstate *dl_clnt_odstate;
-	u32			dl_type;
 	time64_t		dl_time;
+	u32			dl_type;
 /* For recall: */
 	int			dl_retries;
 	struct nfsd4_callback	dl_recall;

-- 
2.48.1

[PATCH 2/5] nfsd: remove unneeded forward declaration of nfsd4_mark_cb_fault()

[Jeff Layton]

This isn't needed.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/nfs4callback.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
index ab81774bf1d2766272e610f5d081f7c04d9b5933..ccb00aa93be01961fb182eb05470a7bd8e642256 100644
--- a/fs/nfsd/nfs4callback.c
+++ b/fs/nfsd/nfs4callback.c
@@ -46,8 +46,6 @@
 
 #define NFSDDBG_FACILITY                NFSDDBG_PROC
 
-static void nfsd4_mark_cb_fault(struct nfs4_client *clp);
-
 #define NFSPROC4_CB_NULL 0
 #define NFSPROC4_CB_COMPOUND 1
 

-- 
2.48.1

[PATCH 3/5] nfsd: remove obsolete comment from nfs4_alloc_stid

[Jeff Layton]

idr_alloc_cyclic() is what guarantees that now, not this long-gone trick.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/nfs4state.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index e245c479ef271b6d6b20bcaf468f6de40d62ee00..a7bac93445e2fdbe743b77e66238d652094907cb 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -946,15 +946,6 @@ struct nfs4_stid *nfs4_alloc_stid(struct nfs4_client *cl, struct kmem_cache *sla
 	spin_lock_init(&stid->sc_lock);
 	INIT_LIST_HEAD(&stid->sc_cp_list);
 
-	/*
-	 * It shouldn't be a problem to reuse an opaque stateid value.
-	 * I don't think it is for 4.1.  But with 4.0 I worry that, for
-	 * example, a stray write retransmission could be accepted by
-	 * the server when it should have been rejected.  Therefore,
-	 * adopt a trick from the sctp code to attempt to maximize the
-	 * amount of time until an id is reused, by ensuring they always
-	 * "increase" (mod INT_MAX):
-	 */
 	return stid;
 out_free:
 	kmem_cache_free(slab, stid);

-- 
2.48.1

[PATCH 4/5] nfsd: clean up if statement in nfsd4_close_open_stateid()

[Jeff Layton]

Just set unhashed to false in the one case where we return that
explicitly, and drop the else.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/nfs4state.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index a7bac93445e2fdbe743b77e66238d652094907cb..1f3e9d42fcd784ea8d101ad3549702a30dfe9058 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -7644,12 +7644,11 @@ static bool nfsd4_close_open_stateid(struct nfs4_ol_stateid *s)
 		list_for_each_entry(stp, &reaplist, st_locks)
 			nfs4_free_cpntf_statelist(clp->net, &stp->st_stid);
 		free_ol_stateid_reaplist(&reaplist);
-		return false;
-	} else {
-		spin_unlock(&clp->cl_lock);
-		free_ol_stateid_reaplist(&reaplist);
-		return unhashed;
+		unhashed = false;
 	}
+	spin_unlock(&clp->cl_lock);
+	free_ol_stateid_reaplist(&reaplist);
+	return unhashed;
 }
 
 /*

-- 
2.48.1

[PATCH 5/5] nfsd: use a long for the count in nfsd4_state_shrinker_count()

[Jeff Layton]

If there are no courtesy clients then the return value from the
atomic_long_read() could overflow an int. Use a long to store the value
instead.

Signed-off-by: Jeff Layton <jlayton@kernel.org>
---
 fs/nfsd/nfs4state.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 1f3e9d42fcd784ea8d101ad3549702a30dfe9058..aa0afd1b19a254686ef1f15f5f11db1c79d69096 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -4819,7 +4819,7 @@ nfsd4_init_slabs(void)
 static unsigned long
 nfsd4_state_shrinker_count(struct shrinker *shrink, struct shrink_control *sc)
 {
-	int count;
+	long count;
 	struct nfsd_net *nn = shrink->private_data;
 
 	count = atomic_read(&nn->nfsd_courtesy_clients);

-- 
2.48.1

Re: [PATCH 0/5] nfsd: some small cleanup patches

[cel]

From: Chuck Lever <chuck.lever@oracle.com>

On Mon, 03 Mar 2025 12:25:58 -0500, Jeff Layton wrote:
> This is a pile of small patches that I've been collecting. None are
> terribly critical.
> 
> 

Applied to nfsd-testing, thanks!

[1/5] nfsd: reorganize struct nfs4_delegation for better packing
      commit: 125e6a10ea13220737fb44769e449a77609d0141
[2/5] nfsd: remove unneeded forward declaration of nfsd4_mark_cb_fault()
      commit: f47d831d1066a7f344bfe4d9046cc1d64855373e
[3/5] nfsd: remove obsolete comment from nfs4_alloc_stid
      commit: cb539e9d699a4507c28809edf2f8671fe1e1ef6e
[4/5] nfsd: clean up if statement in nfsd4_close_open_stateid()
      commit: a0f8129957c861f235abc7ae1c24ce11f9bb8d6d
[5/5] nfsd: use a long for the count in nfsd4_state_shrinker_count()
      commit: 2327318960a4c4ebb1745f09d5d310b9bb545e98

--
Chuck Lever

Re: [PATCH 4/5] nfsd: clean up if statement in nfsd4_close_open_stateid()

[Jeff Layton]

On Mon, 2025-03-03 at 12:26 -0500, Jeff Layton wrote:
> Just set unhashed to false in the one case where we return that
> explicitly, and drop the else.
> 
> Signed-off-by: Jeff Layton <jlayton@kernel.org>
> ---
>  fs/nfsd/nfs4state.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index a7bac93445e2fdbe743b77e66238d652094907cb..1f3e9d42fcd784ea8d101ad3549702a30dfe9058 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -7644,12 +7644,11 @@ static bool nfsd4_close_open_stateid(struct nfs4_ol_stateid *s)
>  		list_for_each_entry(stp, &reaplist, st_locks)
>  			nfs4_free_cpntf_statelist(clp->net, &stp->st_stid);
>  		free_ol_stateid_reaplist(&reaplist);
> -		return false;
> -	} else {
> -		spin_unlock(&clp->cl_lock);
> -		free_ol_stateid_reaplist(&reaplist);
> -		return unhashed;
> +		unhashed = false;
>  	}
> +	spin_unlock(&clp->cl_lock);
> +	free_ol_stateid_reaplist(&reaplist);
> +	return unhashed;
>  }
>  
>  /*
> 

My apologies, Chuck. This patch has a bug in it. Can you drop it from
nfsd-testing? I may or may not send a replacement.

Thanks,
-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH 4/5] nfsd: clean up if statement in nfsd4_close_open_stateid()

[Chuck Lever]

On 3/4/25 10:24 AM, Jeff Layton wrote:
> On Mon, 2025-03-03 at 12:26 -0500, Jeff Layton wrote:
>> Just set unhashed to false in the one case where we return that
>> explicitly, and drop the else.
>>
>> Signed-off-by: Jeff Layton <jlayton@kernel.org>
>> ---
>>  fs/nfsd/nfs4state.c | 9 ++++-----
>>  1 file changed, 4 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>> index a7bac93445e2fdbe743b77e66238d652094907cb..1f3e9d42fcd784ea8d101ad3549702a30dfe9058 100644
>> --- a/fs/nfsd/nfs4state.c
>> +++ b/fs/nfsd/nfs4state.c
>> @@ -7644,12 +7644,11 @@ static bool nfsd4_close_open_stateid(struct nfs4_ol_stateid *s)
>>  		list_for_each_entry(stp, &reaplist, st_locks)
>>  			nfs4_free_cpntf_statelist(clp->net, &stp->st_stid);
>>  		free_ol_stateid_reaplist(&reaplist);
>> -		return false;
>> -	} else {
>> -		spin_unlock(&clp->cl_lock);
>> -		free_ol_stateid_reaplist(&reaplist);
>> -		return unhashed;
>> +		unhashed = false;
>>  	}
>> +	spin_unlock(&clp->cl_lock);
>> +	free_ol_stateid_reaplist(&reaplist);
>> +	return unhashed;
>>  }
>>  
>>  /*
>>
> 
> My apologies, Chuck. This patch has a bug in it. Can you drop it from
> nfsd-testing? I may or may not send a replacement.

Done.


-- 
Chuck Lever