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

NeilBrown
2024-11-19T22:47:26Z

On Wed, 20 Nov 2024, Jeff Layton wrote:
> 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).

Let's not. There are currently 1926 lines in .c and .h files in the
Linux kernel which contain "?:" and another 848 which contain "? :".
I think it is an established part of the kernel style.

This is admittedly dominated by bcachefs, but there is a long tail with
tools, net, crypto, drivers all contributing. Outside of bcachefs, fs/
contributes only 102 in 29 different filesystems.

Thanks,
NeilBrown