On Wed, 2024-11-13 at 16:38 +1100, NeilBrown wrote:
> When the client confirms that some currently allocated DRC slots are not
> used, free some. We currently keep 6 (NFSD_MAX_UNUSED_SLOTS) unused
> slots to support bursts of client activity. This could be tuned with a
> shrinker.
>
> When we free a slot we store the seqid in the slot pointer so that it can
> be restored when we reactivate the slot. The RFC requires the seqid for
> each slot to increment on each request and does not permit it ever to be
> reset.
>
I think we need to get some clarification on the above.
IMO, once you free a slot, it's gone and its seqid should also be
forgotten. A reactivated slot, IOW, is effectively "new" and its seqid
should start again at 1. I don't think the spec intended to require
that both ends to remember seqids for defunct slots.
> We decoding sa_highest_slotid into maxslots we need to add 1 - this
> matches how it is encoded for the reply.
>
> Signed-off-by: NeilBrown <neilb@xxxxxxx>
> ---
> fs/nfsd/nfs4state.c | 25 +++++++++++++++++++------
> fs/nfsd/nfs4xdr.c | 3 ++-
> 2 files changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index 683bb908039b..15de62416243 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -1910,14 +1910,21 @@ gen_sessionid(struct nfsd4_session *ses)
> #define NFSD_MIN_HDR_SEQ_SZ (24 + 12 + 44)
>
> static void
> -free_session_slots(struct nfsd4_session *ses)
> +free_session_slots(struct nfsd4_session *ses, int from)
> {
> int i;
>
> - for (i = 0; i < ses->se_fchannel.maxreqs; i++) {
> + if (from >= ses->se_fchannel.maxreqs)
> + return;
> +
> + for (i = from; i < ses->se_fchannel.maxreqs; i++) {
> + uintptr_t seqid = ses->se_slots[i]->sl_seqid;
> free_svc_cred(&ses->se_slots[i]->sl_cred);
> kfree(ses->se_slots[i]);
> + /* Save the seqid in case we reactivate this slot */
> + ses->se_slots[i] = (void *)seqid;
> }
> + ses->se_fchannel.maxreqs = from;
> }
>
> /*
> @@ -2069,7 +2076,7 @@ static void nfsd4_del_conns(struct nfsd4_session *s)
>
> static void __free_session(struct nfsd4_session *ses)
> {
> - free_session_slots(ses);
> + free_session_slots(ses, 0);
> kfree(ses);
> }
>
> @@ -4214,6 +4221,9 @@ nfsd4_sequence(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> if (status)
> goto out_put_session;
>
> + /* If there are lots of unused slots, free some */
> + free_session_slots(session, seq->maxslots + NFSD_MAX_UNUSED_SLOTS);
> +
> buflen = (seq->cachethis) ?
> session->se_fchannel.maxresp_cached :
> session->se_fchannel.maxresp_sz;
> @@ -4244,10 +4254,13 @@ nfsd4_sequence(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> session->se_fchannel.maxreqs < NFSD_MAX_SLOTS_PER_SESSION) {
> int s = session->se_fchannel.maxreqs;
>
> - session->se_slots[s] = kzalloc(slot_bytes(&session->se_fchannel),
> - GFP_NOWAIT);
> - if (session->se_slots[s])
> + struct nfsd4_slot *slot = kzalloc(slot_bytes(&session->se_fchannel),
> + GFP_NOWAIT);
> + if (slot) {
> + slot->sl_seqid = (uintptr_t)session->se_slots[s];
> + session->se_slots[s] = slot;
> session->se_fchannel.maxreqs += 1;
> + }
> }
> seq->maxslots = session->se_fchannel.maxreqs;
>
> diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
> index f118921250c3..846ed52fdaf5 100644
> --- a/fs/nfsd/nfs4xdr.c
> +++ b/fs/nfsd/nfs4xdr.c
> @@ -1884,7 +1884,8 @@ nfsd4_decode_sequence(struct nfsd4_compoundargs *argp,
> return nfserr_bad_xdr;
> seq->seqid = be32_to_cpup(p++);
> seq->slotid = be32_to_cpup(p++);
> - seq->maxslots = be32_to_cpup(p++);
> + /* sa_highest_slotid counts from 0 but maxslots counts from 1 ... */
> + seq->maxslots = be32_to_cpup(p++) + 1;
> seq->cachethis = be32_to_cpup(p);
>
> seq->status_flags = 0;
--
Jeff Layton <jlayton@xxxxxxxxxx>
