On Tue, 2012-02-14 at 19:53 -0500, J. Bruce Fields wrote: > On Wed, Feb 15, 2012 at 12:43:08AM +0000, Myklebust, Trond wrote: > > Then we have a problem. The zero initialisation is already in use out > > there in both commercial and non-commercial versions of Linux. It is too > > late to change that now. > > > > Furthermore, since none of the servers we've tested against in earlier > > Bakeathons and Connectathons have complained, I suggest that we rather > > change the spec with an errata. > > Argh. > > I just noticed that the server was crashing intermittently and traced it > to incorrect handling of the case where the client sends a one-op > SEQUENCE compound with seqid 0. I'm not sure why it started popping up > just in 3.3--perhaps some change in the way the client uses slots made > that more likely. > > The server code actually looks like it did assume initial seqid 1, but > accepted initial seqid 0 (except in this one case) basically by mistake. > > In any case, I think it should be easy enough to teach it just to accept > any seqid on a previously unused slot, so I'll do that.... Hang on... Looking at 3.2, I think you're correct. We used to initialise to 1, and now we're initialising to 0. Ah... I see what happened. commit aacd5537270a752fe12a9914a207284fc2341c6d (NFSv4.1: cleanup init and reset of session slot tables) seems to have removed the call to nfs4_reset_slot_tables() that was doing that initialisation. OK... So this is a Linux 3.3-rcX only regression, and should be treated as such... How about the following instead so that we ensure we fully share the initialisation code between nfs4_reset_slot_tables and nfs4_init_slot_tables? 8<------------------------------------------------------------------------- >From 8ae8d177cc106ecdb3e29b48c78ec37cf76ea549 Mon Sep 17 00:00:00 2001 From: Trond Myklebust <Trond.Myklebust@xxxxxxxxxx> Date: Tue, 14 Feb 2012 20:33:19 -0500 Subject: [PATCH] NFSv4.1: Fix a NFSv4.1 session initialisation regression Commit aacd553 (NFSv4.1: cleanup init and reset of session slot tables) introduces a regression in the session initialisation code. New tables now find their sequence ids initialised to 0, rather than the mandated value of 1 (see RFC5661). Reported-by: Vitaliy Gusev <gusev.vitaliy@xxxxxxxxxxx> Signed-off-by: Trond Myklebust <Trond.Myklebust@xxxxxxxxxx> --- fs/nfs/nfs4proc.c | 58 ++++++++++++++++++++++++++++++++--------------------- 1 files changed, 35 insertions(+), 23 deletions(-) diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index d202e04..9f76ce9 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -5008,37 +5008,53 @@ int nfs4_proc_get_lease_time(struct nfs_client *clp, struct nfs_fsinfo *fsinfo) return status; } +static struct nfs4_slot *nfs4_session_alloc_slots(u32 max_slots, gfp_t gfp_flags) +{ + return kcalloc(max_slots, sizeof(struct nfs4_slot), gfp_flags); +} + +static void nfs4_session_add_slots(struct nfs4_slot_table *tbl, + struct nfs4_slot *new, + u32 max_slots, + u32 ivalue) +{ + struct nfs4_slot *old = NULL; + u32 i; + + spin_lock(&tbl->slot_tbl_lock); + if (new) { + old = tbl->slots; + tbl->slots = new; + tbl->max_slots = max_slots; + } + tbl->highest_used_slotid = -1; /* no slot is currently used */ + for (i = 0; i < tbl->max_slots; i++) + tbl->slots[i].seq_nr = ivalue; + spin_unlock(&tbl->slot_tbl_lock); + kfree(old); +} + /* * Reset a slot table */ static int nfs4_reset_slot_table(struct nfs4_slot_table *tbl, u32 max_reqs, - int ivalue) + u32 ivalue) { struct nfs4_slot *new = NULL; - int i; - int ret = 0; + int ret = -ENOMEM; dprintk("--> %s: max_reqs=%u, tbl->max_slots %d\n", __func__, max_reqs, tbl->max_slots); /* Does the newly negotiated max_reqs match the existing slot table? */ if (max_reqs != tbl->max_slots) { - ret = -ENOMEM; - new = kmalloc(max_reqs * sizeof(struct nfs4_slot), - GFP_NOFS); + new = nfs4_session_alloc_slots(max_reqs, GFP_NOFS); if (!new) goto out; - ret = 0; - kfree(tbl->slots); } - spin_lock(&tbl->slot_tbl_lock); - if (new) { - tbl->slots = new; - tbl->max_slots = max_reqs; - } - for (i = 0; i < tbl->max_slots; ++i) - tbl->slots[i].seq_nr = ivalue; - spin_unlock(&tbl->slot_tbl_lock); + ret = 0; + + nfs4_session_add_slots(tbl, new, max_reqs, ivalue); dprintk("%s: tbl=%p slots=%p max_slots=%d\n", __func__, tbl, tbl->slots, tbl->max_slots); out: @@ -5064,7 +5080,7 @@ static void nfs4_destroy_slot_tables(struct nfs4_session *session) * Initialize slot table */ static int nfs4_init_slot_table(struct nfs4_slot_table *tbl, - int max_slots, int ivalue) + u32 max_slots, u32 ivalue) { struct nfs4_slot *slot; int ret = -ENOMEM; @@ -5073,16 +5089,12 @@ static int nfs4_init_slot_table(struct nfs4_slot_table *tbl, dprintk("--> %s: max_reqs=%u\n", __func__, max_slots); - slot = kcalloc(max_slots, sizeof(struct nfs4_slot), GFP_NOFS); + slot = nfs4_session_alloc_slots(max_slots, GFP_NOFS); if (!slot) goto out; ret = 0; - spin_lock(&tbl->slot_tbl_lock); - tbl->max_slots = max_slots; - tbl->slots = slot; - tbl->highest_used_slotid = -1; /* no slot is currently used */ - spin_unlock(&tbl->slot_tbl_lock); + nfs4_session_add_slots(tbl, slot, max_slots, ivalue); dprintk("%s: tbl=%p slots=%p max_slots=%d\n", __func__, tbl, tbl->slots, tbl->max_slots); out: -- 1.7.7.6 -- Trond Myklebust Linux NFS client maintainer NetApp Trond.Myklebust@xxxxxxxxxx www.netapp.com ��.n��������+%������w��{.n�����{��w���jg��������ݢj����G�������j:+v���w�m������w�������h�����٥