On Thu, 2024-10-24 at 09:10 +1100, NeilBrown wrote:
> Failing OP_SETCLIENTID or OP_EXCHANGE_ID should only happen if there is
> memory allocation failure. Putting a hard limit on the number of
> clients is really helpful as it will either happen too early and prevent
"unhelpful" ?
> clients that the server can easily handle, or too late and allow clients
> when the server is swamped.
>
> The calculated limit is still useful for expiring courtesy clients where
> there are "too many" clients, but it shouldn't prevent the creation of
> active clients.
>
> Testing of lots of clients against small-mem servers reports repeated
> NFS4ERR_DELAY responses which doesn't seem helpful. There may have been
> reports of similar problems in production use.
>
> Also remove an outdated comment - we do use a slab cache.
>
> Signed-off-by: NeilBrown <neilb@xxxxxxx>
> ---
> fs/nfsd/nfs4state.c | 11 +++--------
> 1 file changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index d585c267731b..0791a43b19e6 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -2218,21 +2218,16 @@ STALE_CLIENTID(clientid_t *clid, struct nfsd_net *nn)
> return 1;
> }
>
> -/*
> - * XXX Should we use a slab cache ?
> - * This type of memory management is somewhat inefficient, but we use it
> - * anyway since SETCLIENTID is not a common operation.
> - */
> static struct nfs4_client *alloc_client(struct xdr_netobj name,
> struct nfsd_net *nn)
> {
> struct nfs4_client *clp;
> int i;
>
> - if (atomic_read(&nn->nfs4_client_count) >= nn->nfs4_max_clients) {
> + if (atomic_read(&nn->nfs4_client_count) >= nn->nfs4_max_clients &&
> + atomic_read(&nn->nfsd_courtesy_clients) > 0)
> mod_delayed_work(laundry_wq, &nn->laundromat_work, 0);
> - return NULL;
> - }
> +
> clp = kmem_cache_zalloc(client_slab, GFP_KERNEL);
> if (clp == NULL)
> return NULL;
Do we even need to check nn->nfs4_max_clients at all?
Maybe we should just kick the laundromat whenever
nfsd_courtesy_clients > 0. I would suggest just removing
nfs4_max_clients altogether, but it looks like
nfs4_get_client_reaplist() uses it and it's not clear to me what would
better replace it.
--
Jeff Layton <jlayton@xxxxxxxxxx>
