diff mbox series

[v2,1/2] NFSD: keep track of the number of courtesy clients in the system

Message ID 1661734063-22023-2-git-send-email-dai.ngo@oracle.com (mailing list archive)
State New, archived
Headers show
Series NFSD: memory shrinker for NFSv4 clients | expand

Commit Message

Dai Ngo Aug. 29, 2022, 12:47 a.m. UTC 
Add counter nfs4_courtesy_client_count to nfsd_net to keep track
of the number of courtesy clients in the system.

Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
---
 fs/nfsd/netns.h     |  2 ++
 fs/nfsd/nfs4state.c | 20 +++++++++++++++-----
 2 files changed, 17 insertions(+), 5 deletions(-)

Comments

Jeff Layton Aug. 29, 2022, 4:48 p.m. UTC | #1 
On Sun, 2022-08-28 at 17:47 -0700, Dai Ngo wrote:
> Add counter nfs4_courtesy_client_count to nfsd_net to keep track
> of the number of courtesy clients in the system.
> 
> Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
> ---
>  fs/nfsd/netns.h     |  2 ++
>  fs/nfsd/nfs4state.c | 20 +++++++++++++++-----
>  2 files changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h
> index ffe17743cc74..2695dff1378a 100644
> --- a/fs/nfsd/netns.h
> +++ b/fs/nfsd/netns.h
> @@ -192,6 +192,8 @@ struct nfsd_net {
>  
>  	atomic_t		nfs4_client_count;
>  	int			nfs4_max_clients;
> +
> +	atomic_t		nfsd_courtesy_client_count;
>  };
>  
>  /* Simple check to find out if a given net was properly initialized */
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index c5d199d7e6b4..3d8d7ebb5b91 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -169,7 +169,8 @@ static __be32 get_client_locked(struct nfs4_client *clp)
>  	if (is_client_expired(clp))
>  		return nfserr_expired;
>  	atomic_inc(&clp->cl_rpc_users);
> -	clp->cl_state = NFSD4_ACTIVE;
> +	if (xchg(&clp->cl_state, NFSD4_ACTIVE) != NFSD4_ACTIVE)

The xchg calls seem like overkill. The cl_state is protected by the
nn->client_lock. Nothing else can race in and change it here.

> +		atomic_add_unless(&nn->nfsd_courtesy_client_count, -1, 0);
>  	return nfs_ok;
>  }
>  
> @@ -190,7 +191,8 @@ renew_client_locked(struct nfs4_client *clp)
>  
>  	list_move_tail(&clp->cl_lru, &nn->client_lru);
>  	clp->cl_time = ktime_get_boottime_seconds();
> -	clp->cl_state = NFSD4_ACTIVE;
> +	if (xchg(&clp->cl_state, NFSD4_ACTIVE) != NFSD4_ACTIVE)
> +		atomic_add_unless(&nn->nfsd_courtesy_client_count, -1, 0);
>  }
>  
>  static void put_client_renew_locked(struct nfs4_client *clp)
> @@ -2233,6 +2235,8 @@ __destroy_client(struct nfs4_client *clp)
>  	if (clp->cl_cb_conn.cb_xprt)
>  		svc_xprt_put(clp->cl_cb_conn.cb_xprt);
>  	atomic_add_unless(&nn->nfs4_client_count, -1, 0);
> +	if (clp->cl_state != NFSD4_ACTIVE)
> +		atomic_add_unless(&nn->nfsd_courtesy_client_count, -1, 0);
>  	free_client(clp);
>  	wake_up_all(&expiry_wq);
>  }
> @@ -4356,6 +4360,8 @@ void nfsd4_init_leases_net(struct nfsd_net *nn)
>  	max_clients = (u64)si.totalram * si.mem_unit / (1024 * 1024 * 1024);
>  	max_clients *= NFS4_CLIENTS_PER_GB;
>  	nn->nfs4_max_clients = max_t(int, max_clients, NFS4_CLIENTS_PER_GB);
> +
> +	atomic_set(&nn->nfsd_courtesy_client_count, 0);
>  }
>  
>  static void init_nfs4_replay(struct nfs4_replay *rp)
> @@ -5864,7 +5870,7 @@ static void
>  nfs4_get_client_reaplist(struct nfsd_net *nn, struct list_head *reaplist,
>  				struct laundry_time *lt)
>  {
> -	unsigned int maxreap, reapcnt = 0;
> +	unsigned int oldstate, maxreap, reapcnt = 0;
>  	struct list_head *pos, *next;
>  	struct nfs4_client *clp;
>  
> @@ -5878,8 +5884,12 @@ nfs4_get_client_reaplist(struct nfsd_net *nn, struct list_head *reaplist,
>  			goto exp_client;
>  		if (!state_expired(lt, clp->cl_time))
>  			break;
> -		if (!atomic_read(&clp->cl_rpc_users))
> -			clp->cl_state = NFSD4_COURTESY;
> +		oldstate = NFSD4_ACTIVE;
> +		if (!atomic_read(&clp->cl_rpc_users)) {
> +			oldstate = xchg(&clp->cl_state, NFSD4_COURTESY);
> +			if (oldstate == NFSD4_ACTIVE)
> +				atomic_inc(&nn->nfsd_courtesy_client_count);
> +		}
>  		if (!client_has_state(clp))
>  			goto exp_client;
>  		if (!nfs4_anylock_blockers(clp))
Dai Ngo Aug. 29, 2022, 6:24 p.m. UTC | #2 
On 8/29/22 9:48 AM, Jeff Layton wrote:
> On Sun, 2022-08-28 at 17:47 -0700, Dai Ngo wrote:
>> Add counter nfs4_courtesy_client_count to nfsd_net to keep track
>> of the number of courtesy clients in the system.
>>
>> Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
>> ---
>>   fs/nfsd/netns.h     |  2 ++
>>   fs/nfsd/nfs4state.c | 20 +++++++++++++++-----
>>   2 files changed, 17 insertions(+), 5 deletions(-)
>>
>> diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h
>> index ffe17743cc74..2695dff1378a 100644
>> --- a/fs/nfsd/netns.h
>> +++ b/fs/nfsd/netns.h
>> @@ -192,6 +192,8 @@ struct nfsd_net {
>>   
>>   	atomic_t		nfs4_client_count;
>>   	int			nfs4_max_clients;
>> +
>> +	atomic_t		nfsd_courtesy_client_count;
>>   };
>>   
>>   /* Simple check to find out if a given net was properly initialized */
>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>> index c5d199d7e6b4..3d8d7ebb5b91 100644
>> --- a/fs/nfsd/nfs4state.c
>> +++ b/fs/nfsd/nfs4state.c
>> @@ -169,7 +169,8 @@ static __be32 get_client_locked(struct nfs4_client *clp)
>>   	if (is_client_expired(clp))
>>   		return nfserr_expired;
>>   	atomic_inc(&clp->cl_rpc_users);
>> -	clp->cl_state = NFSD4_ACTIVE;
>> +	if (xchg(&clp->cl_state, NFSD4_ACTIVE) != NFSD4_ACTIVE)
> The xchg calls seem like overkill. The cl_state is protected by the
> nn->client_lock. Nothing else can race in and change it here.

I use the 'xchg' calls for convenience and readability and not for
protection in this case. But if you think this is overkill or
unnecessary then I remove it.

Fix in v2.

Thanks,
-Dai

>
>> +		atomic_add_unless(&nn->nfsd_courtesy_client_count, -1, 0);
>>   	return nfs_ok;
>>   }
>>   
>> @@ -190,7 +191,8 @@ renew_client_locked(struct nfs4_client *clp)
>>   
>>   	list_move_tail(&clp->cl_lru, &nn->client_lru);
>>   	clp->cl_time = ktime_get_boottime_seconds();
>> -	clp->cl_state = NFSD4_ACTIVE;
>> +	if (xchg(&clp->cl_state, NFSD4_ACTIVE) != NFSD4_ACTIVE)
>> +		atomic_add_unless(&nn->nfsd_courtesy_client_count, -1, 0);
>>   }
>>   
>>   static void put_client_renew_locked(struct nfs4_client *clp)
>> @@ -2233,6 +2235,8 @@ __destroy_client(struct nfs4_client *clp)
>>   	if (clp->cl_cb_conn.cb_xprt)
>>   		svc_xprt_put(clp->cl_cb_conn.cb_xprt);
>>   	atomic_add_unless(&nn->nfs4_client_count, -1, 0);
>> +	if (clp->cl_state != NFSD4_ACTIVE)
>> +		atomic_add_unless(&nn->nfsd_courtesy_client_count, -1, 0);
>>   	free_client(clp);
>>   	wake_up_all(&expiry_wq);
>>   }
>> @@ -4356,6 +4360,8 @@ void nfsd4_init_leases_net(struct nfsd_net *nn)
>>   	max_clients = (u64)si.totalram * si.mem_unit / (1024 * 1024 * 1024);
>>   	max_clients *= NFS4_CLIENTS_PER_GB;
>>   	nn->nfs4_max_clients = max_t(int, max_clients, NFS4_CLIENTS_PER_GB);
>> +
>> +	atomic_set(&nn->nfsd_courtesy_client_count, 0);
>>   }
>>   
>>   static void init_nfs4_replay(struct nfs4_replay *rp)
>> @@ -5864,7 +5870,7 @@ static void
>>   nfs4_get_client_reaplist(struct nfsd_net *nn, struct list_head *reaplist,
>>   				struct laundry_time *lt)
>>   {
>> -	unsigned int maxreap, reapcnt = 0;
>> +	unsigned int oldstate, maxreap, reapcnt = 0;
>>   	struct list_head *pos, *next;
>>   	struct nfs4_client *clp;
>>   
>> @@ -5878,8 +5884,12 @@ nfs4_get_client_reaplist(struct nfsd_net *nn, struct list_head *reaplist,
>>   			goto exp_client;
>>   		if (!state_expired(lt, clp->cl_time))
>>   			break;
>> -		if (!atomic_read(&clp->cl_rpc_users))
>> -			clp->cl_state = NFSD4_COURTESY;
>> +		oldstate = NFSD4_ACTIVE;
>> +		if (!atomic_read(&clp->cl_rpc_users)) {
>> +			oldstate = xchg(&clp->cl_state, NFSD4_COURTESY);
>> +			if (oldstate == NFSD4_ACTIVE)
>> +				atomic_inc(&nn->nfsd_courtesy_client_count);
>> +		}
>>   		if (!client_has_state(clp))
>>   			goto exp_client;
>>   		if (!nfs4_anylock_blockers(clp))
Jeff Layton Aug. 29, 2022, 6:27 p.m. UTC | #3 
On Mon, 2022-08-29 at 11:24 -0700, dai.ngo@oracle.com wrote:
> On 8/29/22 9:48 AM, Jeff Layton wrote:
> > On Sun, 2022-08-28 at 17:47 -0700, Dai Ngo wrote:
> > > Add counter nfs4_courtesy_client_count to nfsd_net to keep track
> > > of the number of courtesy clients in the system.
> > > 
> > > Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
> > > ---
> > >   fs/nfsd/netns.h     |  2 ++
> > >   fs/nfsd/nfs4state.c | 20 +++++++++++++++-----
> > >   2 files changed, 17 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h
> > > index ffe17743cc74..2695dff1378a 100644
> > > --- a/fs/nfsd/netns.h
> > > +++ b/fs/nfsd/netns.h
> > > @@ -192,6 +192,8 @@ struct nfsd_net {
> > >   
> > >   	atomic_t		nfs4_client_count;
> > >   	int			nfs4_max_clients;
> > > +
> > > +	atomic_t		nfsd_courtesy_client_count;
> > >   };
> > >   
> > >   /* Simple check to find out if a given net was properly initialized */
> > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > > index c5d199d7e6b4..3d8d7ebb5b91 100644
> > > --- a/fs/nfsd/nfs4state.c
> > > +++ b/fs/nfsd/nfs4state.c
> > > @@ -169,7 +169,8 @@ static __be32 get_client_locked(struct nfs4_client *clp)
> > >   	if (is_client_expired(clp))
> > >   		return nfserr_expired;
> > >   	atomic_inc(&clp->cl_rpc_users);
> > > -	clp->cl_state = NFSD4_ACTIVE;
> > > +	if (xchg(&clp->cl_state, NFSD4_ACTIVE) != NFSD4_ACTIVE)
> > The xchg calls seem like overkill. The cl_state is protected by the
> > nn->client_lock. Nothing else can race in and change it here.
> 
> I use the 'xchg' calls for convenience and readability and not for
> protection in this case. But if you think this is overkill or
> unnecessary then I remove it.
> 
> Fix in v2.
> 
> Thanks,
> -Dai

No worries, it's a minor thing. It probably doesn't hurt anything in
this context, but the xchg operation itself has to be atomic. We already
hold the spinlock in these places, so there's no need to demand that
here.

> > > +		atomic_add_unless(&nn->nfsd_courtesy_client_count, -1, 0);
> > >   	return nfs_ok;
> > >   }
> > >   
> > > @@ -190,7 +191,8 @@ renew_client_locked(struct nfs4_client *clp)
> > >   
> > >   	list_move_tail(&clp->cl_lru, &nn->client_lru);
> > >   	clp->cl_time = ktime_get_boottime_seconds();
> > > -	clp->cl_state = NFSD4_ACTIVE;
> > > +	if (xchg(&clp->cl_state, NFSD4_ACTIVE) != NFSD4_ACTIVE)
> > > +		atomic_add_unless(&nn->nfsd_courtesy_client_count, -1, 0);
> > >   }
> > >   
> > >   static void put_client_renew_locked(struct nfs4_client *clp)
> > > @@ -2233,6 +2235,8 @@ __destroy_client(struct nfs4_client *clp)
> > >   	if (clp->cl_cb_conn.cb_xprt)
> > >   		svc_xprt_put(clp->cl_cb_conn.cb_xprt);
> > >   	atomic_add_unless(&nn->nfs4_client_count, -1, 0);
> > > +	if (clp->cl_state != NFSD4_ACTIVE)
> > > +		atomic_add_unless(&nn->nfsd_courtesy_client_count, -1, 0);
> > >   	free_client(clp);
> > >   	wake_up_all(&expiry_wq);
> > >   }
> > > @@ -4356,6 +4360,8 @@ void nfsd4_init_leases_net(struct nfsd_net *nn)
> > >   	max_clients = (u64)si.totalram * si.mem_unit / (1024 * 1024 * 1024);
> > >   	max_clients *= NFS4_CLIENTS_PER_GB;
> > >   	nn->nfs4_max_clients = max_t(int, max_clients, NFS4_CLIENTS_PER_GB);
> > > +
> > > +	atomic_set(&nn->nfsd_courtesy_client_count, 0);
> > >   }
> > >   
> > >   static void init_nfs4_replay(struct nfs4_replay *rp)
> > > @@ -5864,7 +5870,7 @@ static void
> > >   nfs4_get_client_reaplist(struct nfsd_net *nn, struct list_head *reaplist,
> > >   				struct laundry_time *lt)
> > >   {
> > > -	unsigned int maxreap, reapcnt = 0;
> > > +	unsigned int oldstate, maxreap, reapcnt = 0;
> > >   	struct list_head *pos, *next;
> > >   	struct nfs4_client *clp;
> > >   
> > > @@ -5878,8 +5884,12 @@ nfs4_get_client_reaplist(struct nfsd_net *nn, struct list_head *reaplist,
> > >   			goto exp_client;
> > >   		if (!state_expired(lt, clp->cl_time))
> > >   			break;
> > > -		if (!atomic_read(&clp->cl_rpc_users))
> > > -			clp->cl_state = NFSD4_COURTESY;
> > > +		oldstate = NFSD4_ACTIVE;
> > > +		if (!atomic_read(&clp->cl_rpc_users)) {
> > > +			oldstate = xchg(&clp->cl_state, NFSD4_COURTESY);
> > > +			if (oldstate == NFSD4_ACTIVE)
> > > +				atomic_inc(&nn->nfsd_courtesy_client_count);
> > > +		}
> > >   		if (!client_has_state(clp))
> > >   			goto exp_client;
> > >   		if (!nfs4_anylock_blockers(clp))
diff mbox series

Patch

diff --git a/fs/nfsd/netns.h b/fs/nfsd/netns.h
index ffe17743cc74..2695dff1378a 100644
--- a/fs/nfsd/netns.h
+++ b/fs/nfsd/netns.h
@@ -192,6 +192,8 @@  struct nfsd_net {
 
 	atomic_t		nfs4_client_count;
 	int			nfs4_max_clients;
+
+	atomic_t		nfsd_courtesy_client_count;
 };
 
 /* Simple check to find out if a given net was properly initialized */
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index c5d199d7e6b4..3d8d7ebb5b91 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -169,7 +169,8 @@  static __be32 get_client_locked(struct nfs4_client *clp)
 	if (is_client_expired(clp))
 		return nfserr_expired;
 	atomic_inc(&clp->cl_rpc_users);
-	clp->cl_state = NFSD4_ACTIVE;
+	if (xchg(&clp->cl_state, NFSD4_ACTIVE) != NFSD4_ACTIVE)
+		atomic_add_unless(&nn->nfsd_courtesy_client_count, -1, 0);
 	return nfs_ok;
 }
 
@@ -190,7 +191,8 @@  renew_client_locked(struct nfs4_client *clp)
 
 	list_move_tail(&clp->cl_lru, &nn->client_lru);
 	clp->cl_time = ktime_get_boottime_seconds();
-	clp->cl_state = NFSD4_ACTIVE;
+	if (xchg(&clp->cl_state, NFSD4_ACTIVE) != NFSD4_ACTIVE)
+		atomic_add_unless(&nn->nfsd_courtesy_client_count, -1, 0);
 }
 
 static void put_client_renew_locked(struct nfs4_client *clp)
@@ -2233,6 +2235,8 @@  __destroy_client(struct nfs4_client *clp)
 	if (clp->cl_cb_conn.cb_xprt)
 		svc_xprt_put(clp->cl_cb_conn.cb_xprt);
 	atomic_add_unless(&nn->nfs4_client_count, -1, 0);
+	if (clp->cl_state != NFSD4_ACTIVE)
+		atomic_add_unless(&nn->nfsd_courtesy_client_count, -1, 0);
 	free_client(clp);
 	wake_up_all(&expiry_wq);
 }
@@ -4356,6 +4360,8 @@  void nfsd4_init_leases_net(struct nfsd_net *nn)
 	max_clients = (u64)si.totalram * si.mem_unit / (1024 * 1024 * 1024);
 	max_clients *= NFS4_CLIENTS_PER_GB;
 	nn->nfs4_max_clients = max_t(int, max_clients, NFS4_CLIENTS_PER_GB);
+
+	atomic_set(&nn->nfsd_courtesy_client_count, 0);
 }
 
 static void init_nfs4_replay(struct nfs4_replay *rp)
@@ -5864,7 +5870,7 @@  static void
 nfs4_get_client_reaplist(struct nfsd_net *nn, struct list_head *reaplist,
 				struct laundry_time *lt)
 {
-	unsigned int maxreap, reapcnt = 0;
+	unsigned int oldstate, maxreap, reapcnt = 0;
 	struct list_head *pos, *next;
 	struct nfs4_client *clp;
 
@@ -5878,8 +5884,12 @@  nfs4_get_client_reaplist(struct nfsd_net *nn, struct list_head *reaplist,
 			goto exp_client;
 		if (!state_expired(lt, clp->cl_time))
 			break;
-		if (!atomic_read(&clp->cl_rpc_users))
-			clp->cl_state = NFSD4_COURTESY;
+		oldstate = NFSD4_ACTIVE;
+		if (!atomic_read(&clp->cl_rpc_users)) {
+			oldstate = xchg(&clp->cl_state, NFSD4_COURTESY);
+			if (oldstate == NFSD4_ACTIVE)
+				atomic_inc(&nn->nfsd_courtesy_client_count);
+		}
 		if (!client_has_state(clp))
 			goto exp_client;
 		if (!nfs4_anylock_blockers(clp))