| Message ID | 20241113055345.494856-4-neilb@suse.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | nfsd: allocate/free session-based DRC slots on demand | expand |
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@suse.de> > --- > 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;
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;
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. 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@suse.de> --- fs/nfsd/nfs4state.c | 25 +++++++++++++++++++------ fs/nfsd/nfs4xdr.c | 3 ++- 2 files changed, 21 insertions(+), 7 deletions(-)