> -----Original Message----- > From: J. Bruce Fields [mailto:bfields@xxxxxxxxxxxx] > Sent: Thursday, December 06, 2012 10:29 PM > To: Myklebust, Trond > Cc: linux-nfs@xxxxxxxxxxxxxxx > Subject: Re: [PATCH 4/4] NFSD: Add support for dynamic slot changes > > On Wed, Nov 28, 2012 at 10:17:43PM -0500, Trond Myklebust wrote: > > The algorithm chosen for growing/shrinking the dynamic slot table size > > is as follows: > > > > If the client is sending a sa_highest_slotid value that is greater > > than or equal to the our server target_highest_slotid, then multiply > > the value of the latter by 5/4. > > > > If, on the other hand, the sa_highest_slotid value is smaller than our > > target_highest_slotid, then try adjusting the latter to half its > > current value, or to the value of sa_highest_slotid+1, whichever is larger. > > > > Signed-off-by: Trond Myklebust <Trond.Myklebust@xxxxxxxxxx> > > --- > > fs/nfsd/nfs4state.c | 72 > +++++++++++++++++++++++++++++++++++++++++++++++++---- > > fs/nfsd/state.h | 2 ++ > > 2 files changed, 69 insertions(+), 5 deletions(-) > > > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index > > a1208cc..88d0de9 100644 > > --- a/fs/nfsd/nfs4state.c > > +++ b/fs/nfsd/nfs4state.c > > @@ -807,6 +807,7 @@ static bool nfsd4_grow_slot_table_locked(struct > nfsd4_slot_table *tbl, > > if (slot != NULL){ > > if (!nfsd4_get_drc_mem(tbl)) > > break; > > + slot->sl_generation = tbl->slt_generation - 1; > > slot->sl_slotid = next_slotid; > > list_add_tail(&slot->sl_list, &tbl->slt_head); > > } > > @@ -853,6 +854,30 @@ static void nfsd4_free_slot_table(struct > nfsd4_slot_table *tbl) > > spin_unlock(&tbl->slt_lock); > > } > > > > +static void nfsd4_fc_set_highest_slotid_locked(struct nfsd4_session > *session, > > + struct nfsd4_slot_table *tbl, > > + u32 target_highest_slotid, > > + u32 highest_slotid) > > +{ > > + /* Lower limit for target_highest_slotid is negotiated limit */ > > + target_highest_slotid = max(target_highest_slotid, > > + session->se_fchannel.maxreqs - 1); > > + /* Lower limit for highest_slotid is target_highest_slotid */ > > + highest_slotid = max(highest_slotid, target_highest_slotid); > > + > > + if (target_highest_slotid == tbl->slt_target_highest_slotid && > > + highest_slotid == tbl->slt_highest_slotid) > > + return; > > + > > + if (!nfsd4_grow_slot_table_locked(tbl, highest_slotid, GFP_KERNEL)) > > + return; > > + > > + tbl->slt_target_highest_slotid = target_highest_slotid; > > + tbl->slt_highest_slotid = highest_slotid; > > + tbl->slt_generation++; > > + nfsd4_truncate_slot_table_locked(tbl, tbl->slt_highest_slotid); } > > + > > static void nfsd4_init_slot_table(struct nfsd4_slot_table *tbl, > > size_t slotsize, u32 highest_slotid) { @@ -2039,6 +2064,45 > @@ out: > > return status; > > } > > > > +static void nfsd4_sequence_adjust_slot_table(struct nfsd4_session > *session, > > + struct nfsd4_slot *slot, u32 sa_highest_slotid, > > + struct nfsd4_sequence *res) > > +{ > > + struct nfsd4_slot_table *tbl = &session->se_slots; > > + u32 next_target; > > + u32 next_highest; > > + > > + spin_lock(&tbl->slt_lock); > > + /* > > + * If this slot hasn't seen our previous values, then don't trust > > + * that the client has seen them. Don't adjust the slot table yet. > > + */ > > + if (slot->sl_generation != tbl->slt_generation) > > + goto out; > > + > > + next_target = tbl->slt_target_highest_slotid; > > + > > + /* Is the client bumping against our current window limits? */ > > + if (sa_highest_slotid >= tbl->slt_target_highest_slotid) { > > + /* Yes! Grow the window size by 1/4 */ > > + next_target += 1 + (tbl->slt_target_highest_slotid >> 2); > > + next_highest = next_target; > > + } else { > > + /* No! Try to shrink the window size by 1/2 */ > > + next_target >>= 1; > > + if (sa_highest_slotid + 1 > next_target) > > + next_target = sa_highest_slotid + 1; > > + next_highest = tbl->slt_target_highest_slotid; > > + } > > + nfsd4_fc_set_highest_slotid_locked(session, tbl, > > + next_target, next_highest); > > +out: > > + slot->sl_generation = tbl->slt_generation; > > + res->highest_slotid = tbl->slt_highest_slotid; > > + res->target_highest_slotid = tbl->slt_target_highest_slotid; > > + spin_unlock(&tbl->slt_lock); > > +} > > + > > static struct nfsd4_conn *__nfsd4_find_conn(struct svc_xprt *xpt, > > struct nfsd4_session *s) { > > struct nfsd4_conn *c; > > @@ -2159,11 +2223,9 @@ nfsd4_sequence(struct svc_rqst *rqstp, > > else > > slot->sl_flags &= ~NFSD4_SLOT_CACHETHIS; > > > > - /* Retrieve new target/highest slotid values */ > > - spin_lock(&session->se_slots.slt_lock); > > - seq->target_highest_slotid = session- > >se_slots.slt_target_highest_slotid; > > - seq->highest_slotid = session->se_slots.slt_highest_slotid; > > - spin_unlock(&session->se_slots.slt_lock); > > + /* Adjust slot table, and retrieve new target/highest slotid values */ > > + nfsd4_sequence_adjust_slot_table(session, slot, > > + seq->highest_slotid, seq); > > This is allocating under a spin lock (see the client_lock that most of > nfsd4_sequence is under.) > > (But I'm not sure what we should be doing instead.) I've already fixed this issue in the latest patches: the right thing is simply to do all this after we drop the client lock. I'll send the updated patches when I get a moment: probably Saturday... Cheers Trond -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
