Re: [PATCH 6/6] nfsd: add shrinker to reduce number of slots allocated per session

Jeff Layton
2024-11-19T21:17:08Z

On Tue, 2024-11-19 at 11:41 +1100, NeilBrown wrote:
> Add a shrinker which frees unused slots and may ask the clients to use
> fewer slots on each session.
>
> Each session now tracks se_client_maxreqs which is the most recent
> max-requests-in-use reported by the client, and se_target_maxreqs which
> is a target number of requests which is reduced by the shrinker.
>
> The shrinker iterates over all sessions on all client in all
> net-namespaces and reduces the target by 1 for each. The shrinker may
> get called multiple times to reduce by more than 1 each.
>
> If se_target_maxreqs is above se_client_maxreqs, those slots can be
> freed immediately. If not the client will be ask to reduce its usage
> and as the usage goes down slots will be freed.
>
> Once the usage has dropped to match the target, the target can be
> increased if the client uses all available slots and if a GFP_NOWAIT
> allocation succeeds.
>
> Signed-off-by: NeilBrown <neilb@suse.de>
> ---
> fs/nfsd/nfs4state.c | 72 ++++++++++++++++++++++++++++++++++++++++++---
> fs/nfsd/state.h | 1 +
> 2 files changed, 69 insertions(+), 4 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 0625b0aec6b8..ac49c3bd0dcb 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -1909,6 +1909,16 @@ gen_sessionid(struct nfsd4_session *ses)
> */
> #define NFSD_MIN_HDR_SEQ_SZ (24 + 12 + 44)
>
> +static struct shrinker *nfsd_slot_shrinker;
> +static DEFINE_SPINLOCK(nfsd_session_list_lock);
> +static LIST_HEAD(nfsd_session_list);
> +/* The sum of "target_slots-1" on every session. The shrinker can push this
> + * down, though it can take a little while for the memory to actually
> + * be freed. The "-1" is because we can never free slot 0 while the
> + * session is active.
> + */
> +static atomic_t nfsd_total_target_slots = ATOMIC_INIT(0);
> +
> static void
> free_session_slots(struct nfsd4_session *ses, int from)
> {
> @@ -1931,11 +1941,14 @@ free_session_slots(struct nfsd4_session *ses, int from)
> kfree(slot);
> }
> ses->se_fchannel.maxreqs = from;
> - if (ses->se_target_maxslots > from)
> - ses->se_target_maxslots = from;
> + if (ses->se_target_maxslots > from) {
> + int new_target = from ?: 1;

Let's make that "from ? from : 1". The above is a non-standard gcc-ism
(AIUI).

> + atomic_sub(ses->se_target_maxslots - new_target, &nfsd_total_target_slots);
> + ses->se_target_maxslots = new_target;
> + }
> }
>
> -static int __maybe_unused
> +static int
> reduce_session_slots(struct nfsd4_session *ses, int dec)
> {
> struct nfsd_net *nn = net_generic(ses->se_client->net,
> @@ -1948,6 +1961,7 @@ reduce_session_slots(struct nfsd4_session *ses, int dec)
> return ret;
> ret = min(dec, ses->se_target_maxslots-1);
> ses->se_target_maxslots -= ret;
> + atomic_sub(ret, &nfsd_total_target_slots);
> ses->se_slot_gen += 1;
> if (ses->se_slot_gen == 0) {
> int i;
> @@ -2006,6 +2020,7 @@ static struct nfsd4_session *alloc_session(struct nfsd4_channel_attrs *fattrs,
> fattrs->maxreqs = i;
> memcpy(&new->se_fchannel, fattrs, sizeof(struct nfsd4_channel_attrs));
> new->se_target_maxslots = i;
> + atomic_add(i - 1, &nfsd_total_target_slots);
> new->se_cb_slot_avail = ~0U;
> new->se_cb_highest_slot = min(battrs->maxreqs - 1,
> NFSD_BC_SLOT_TABLE_SIZE - 1);
> @@ -2130,6 +2145,36 @@ static void free_session(struct nfsd4_session *ses)
> __free_session(ses);
> }
>
> +static unsigned long
> +nfsd_slot_count(struct shrinker *s, struct shrink_control *sc)
> +{
> + unsigned long cnt = atomic_read(&nfsd_total_target_slots);
> +
> + return cnt ? cnt : SHRINK_EMPTY;
> +}
> +
> +static unsigned long
> +nfsd_slot_scan(struct shrinker *s, struct shrink_control *sc)
> +{
> + struct nfsd4_session *ses;
> + unsigned long scanned = 0;
> + unsigned long freed = 0;
> +
> + spin_lock(&nfsd_session_list_lock);
> + list_for_each_entry(ses, &nfsd_session_list, se_all_sessions) {
> + freed += reduce_session_slots(ses, 1);
> + scanned += 1;
> + if (scanned >= sc->nr_to_scan) {
> + /* Move starting point for next scan */
> + list_move(&nfsd_session_list, &ses->se_all_sessions);
> + break;
> + }
> + }
> + spin_unlock(&nfsd_session_list_lock);
> + sc->nr_scanned = scanned;
> + return freed;
> +}
> +
> static void init_session(struct svc_rqst *rqstp, struct nfsd4_session *new, struct nfs4_client *clp, struct nfsd4_create_session *cses)
> {
> int idx;
> @@ -2154,6 +2199,10 @@ static void init_session(struct svc_rqst *rqstp, struct nfsd4_session *new, stru
> list_add(&new->se_perclnt, &clp->cl_sessions);
> spin_unlock(&clp->cl_lock);
>
> + spin_lock(&nfsd_session_list_lock);
> + list_add_tail(&new->se_all_sessions, &nfsd_session_list);
> + spin_unlock(&nfsd_session_list_lock);
> +
> {
> struct sockaddr *sa = svc_addr(rqstp);
> /*
> @@ -2223,6 +2272,9 @@ unhash_session(struct nfsd4_session *ses)
> spin_lock(&ses->se_client->cl_lock);
> list_del(&ses->se_perclnt);
> spin_unlock(&ses->se_client->cl_lock);
> + spin_lock(&nfsd_session_list_lock);
> + list_del(&ses->se_all_sessions);
> + spin_unlock(&nfsd_session_list_lock);
> }
>
> /* SETCLIENTID and SETCLIENTID_CONFIRM Helper functions */
> @@ -4335,6 +4387,7 @@ nfsd4_sequence(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> slot->sl_seqid = seq->seqid;
> slot->sl_flags &= ~NFSD4_SLOT_REUSED;
> slot->sl_flags |= NFSD4_SLOT_INUSE;
> + slot->sl_generation = session->se_slot_gen;
> if (seq->cachethis)
> slot->sl_flags |= NFSD4_SLOT_CACHETHIS;
> else
> @@ -4371,6 +4424,8 @@ nfsd4_sequence(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> if (slot && !xa_is_err(xa_store(&session->se_slots, s, slot,
> GFP_ATOMIC))) {
> session->se_fchannel.maxreqs += 1;
> + atomic_add(session->se_fchannel.maxreqs - session->se_target_maxslots,
> + &nfsd_total_target_slots);
> session->se_target_maxslots = session->se_fchannel.maxreqs;
> } else {
> kfree(slot);
> @@ -8779,7 +8834,6 @@ nfs4_state_start_net(struct net *net)
> }
>
> /* initialization to perform when the nfsd service is started: */
> -
> int
> nfs4_state_start(void)
> {
> @@ -8789,6 +8843,15 @@ nfs4_state_start(void)
> if (ret)
> return ret;
>
> + nfsd_slot_shrinker = shrinker_alloc(0, "nfsd-DRC-slot");
> + if (!nfsd_slot_shrinker) {
> + rhltable_destroy(&nfs4_file_rhltable);
> + return -ENOMEM;
> + }
> + nfsd_slot_shrinker->count_objects = nfsd_slot_count;
> + nfsd_slot_shrinker->scan_objects = nfsd_slot_scan;
> + shrinker_register(nfsd_slot_shrinker);
> +
> set_max_delegations();
> return 0;
> }
> @@ -8830,6 +8893,7 @@ void
> nfs4_state_shutdown(void)
> {
> rhltable_destroy(&nfs4_file_rhltable);
> + shrinker_free(nfsd_slot_shrinker);
> }
>
> static void
> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> index ea6659d52be2..0e320ba097f2 100644
> --- a/fs/nfsd/state.h
> +++ b/fs/nfsd/state.h
> @@ -345,6 +345,7 @@ struct nfsd4_session {
> bool se_dead;
> struct list_head se_hash; /* hash by sessionid */
> struct list_head se_perclnt;
> + struct list_head se_all_sessions;/* global list of sessions */
> struct nfs4_client *se_client;
> struct nfs4_sessionid se_sessionid;
> struct nfsd4_channel_attrs se_fchannel;

--
Jeff Layton <jlayton@kernel.org>