[PATCH 0/5] Convert client back-channel to use session slot table

[PATCH 0/5] Convert client back-channel to use session slot table

[Trond Myklebust]

The following patchset is intended to convert the current ad-hoc backchannel
session semantics to use the generic slot table in fs/nfs/nfs4session.c
This should suffice to allow us to grow the size of the slot table at will.

Trond Myklebust (5):
  NFSv4.x: Remove hard coded slotids in callback channel
  NFSv4.x: CB_SEQUENCE should return NFS4ERR_DELAY if still executing
  NFSv4.x: Enforce the ca_maxresponsesize_cached on the back channel
  NFSv4.x: Fix wraparound issues when validing the callback sequence id
  NFSv4.x: Allow multiple callbacks in flight

 fs/nfs/callback.h      |  3 ++-
 fs/nfs/callback_proc.c | 64 ++++++++++++++++++++++++++------------------------
 fs/nfs/callback_xdr.c  | 12 ++++++----
 fs/nfs/nfs4proc.c      |  2 +-
 fs/nfs/nfs4session.c   | 54 ++++++++++++++++++++++++++++++++----------
 fs/nfs/nfs4session.h   |  8 +++++++
 6 files changed, 93 insertions(+), 50 deletions(-)

-- 
2.5.0

[PATCH 1/5] NFSv4.x: Remove hard coded slotids in callback channel

[Trond Myklebust]

Instead, use the values encoded in the slot table itself.

Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
---
 fs/nfs/callback_proc.c | 6 +++---
 fs/nfs/nfs4proc.c      | 2 +-
 2 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
index f0939d097406..83a66a8f40f2 100644
--- a/fs/nfs/callback_proc.c
+++ b/fs/nfs/callback_proc.c
@@ -361,7 +361,7 @@ validate_seqid(struct nfs4_slot_table *tbl, struct cb_sequenceargs * args)
 	dprintk("%s enter. slotid %u seqid %u\n",
 		__func__, args->csa_slotid, args->csa_sequenceid);
 
-	if (args->csa_slotid >= NFS41_BC_MAX_CALLBACKS)
+	if (args->csa_slotid > tbl->server_highest_slotid)
 		return htonl(NFS4ERR_BADSLOT);
 
 	slot = tbl->slots + args->csa_slotid;
@@ -489,8 +489,8 @@ __be32 nfs4_callback_sequence(struct cb_sequenceargs *args,
 	       sizeof(res->csr_sessionid));
 	res->csr_sequenceid = args->csa_sequenceid;
 	res->csr_slotid = args->csa_slotid;
-	res->csr_highestslotid = NFS41_BC_MAX_CALLBACKS - 1;
-	res->csr_target_highestslotid = NFS41_BC_MAX_CALLBACKS - 1;
+	res->csr_highestslotid = tbl->server_highest_slotid;
+	res->csr_target_highestslotid = tbl->target_highest_slotid;
 
 	status = validate_seqid(tbl, args);
 	if (status)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 4bfc33ad0563..be685e236bd5 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -7319,7 +7319,7 @@ static void nfs4_init_channel_attrs(struct nfs41_create_session_args *args)
 	args->bc_attrs.max_resp_sz = PAGE_SIZE;
 	args->bc_attrs.max_resp_sz_cached = 0;
 	args->bc_attrs.max_ops = NFS4_MAX_BACK_CHANNEL_OPS;
-	args->bc_attrs.max_reqs = 1;
+	args->bc_attrs.max_reqs = NFS41_BC_MAX_CALLBACKS;
 
 	dprintk("%s: Back Channel : max_rqst_sz=%u max_resp_sz=%u "
 		"max_resp_sz_cached=%u max_ops=%u max_reqs=%u\n",
-- 
2.5.0

[PATCH 2/5] NFSv4.x: CB_SEQUENCE should return NFS4ERR_DELAY if still executing

[Trond Myklebust]

See RFC5661 Section 2.10.6.2: if retrying a request, and the old one is
still in progress, we must return NFS4ERR_DELAY as the reply to sequence.

Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
---
 fs/nfs/callback_proc.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
index 83a66a8f40f2..e0844fdbd9ac 100644
--- a/fs/nfs/callback_proc.c
+++ b/fs/nfs/callback_proc.c
@@ -375,6 +375,8 @@ validate_seqid(struct nfs4_slot_table *tbl, struct cb_sequenceargs * args)
 	if (args->csa_sequenceid == slot->seq_nr) {
 		dprintk("%s seqid %u is a replay\n",
 			__func__, args->csa_sequenceid);
+		if (tbl->highest_used_slotid != NFS4_NO_SLOT)
+			return htonl(NFS4ERR_DELAY);
 		/* Signal process_op to set this error on next op */
 		if (args->csa_cachethis == 0)
 			return htonl(NFS4ERR_RETRY_UNCACHED_REP);
-- 
2.5.0

[PATCH 3/5] NFSv4.x: Enforce the ca_maxresponsesize_cached on the back channel

[Trond Myklebust]

We have no duplicate reply cache, so we always set the back channel
ca_maxresponsesize_cached to zero when negotiating the session.
That means we should always error out as soon as we see the server
set args->csa_cachethis.

Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
---
 fs/nfs/callback_proc.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
index e0844fdbd9ac..345df6309017 100644
--- a/fs/nfs/callback_proc.c
+++ b/fs/nfs/callback_proc.c
@@ -381,9 +381,8 @@ validate_seqid(struct nfs4_slot_table *tbl, struct cb_sequenceargs * args)
 		if (args->csa_cachethis == 0)
 			return htonl(NFS4ERR_RETRY_UNCACHED_REP);
 
-		/* The ca_maxresponsesize_cached is 0 with no DRC */
-		else if (args->csa_cachethis == 1)
-			return htonl(NFS4ERR_REP_TOO_BIG_TO_CACHE);
+		/* Liar! We never allowed you to set csa_cachethis != 0 */
+		return htonl(NFS4ERR_SEQ_FALSE_RETRY);
 	}
 
 	/* Wraparound */
@@ -500,6 +499,10 @@ __be32 nfs4_callback_sequence(struct cb_sequenceargs *args,
 
 	cps->slotid = args->csa_slotid;
 
+	/* The ca_maxresponsesize_cached is 0 with no DRC */
+	if (args->csa_cachethis != 0)
+		return htonl(NFS4ERR_REP_TOO_BIG_TO_CACHE);
+
 	/*
 	 * Check for pending referring calls.  If a match is found, a
 	 * related callback was received before the response to the original
-- 
2.5.0

[PATCH 4/5] NFSv4.x: Fix wraparound issues when validing the callback sequence id

[Trond Myklebust]

We need to make sure that we don't allow args->csa_sequenceid == 0.

Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
---
 fs/nfs/callback_proc.c | 43 +++++++++++++++++--------------------------
 1 file changed, 17 insertions(+), 26 deletions(-)

diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
index 345df6309017..79c93b3bbfec 100644
--- a/fs/nfs/callback_proc.c
+++ b/fs/nfs/callback_proc.c
@@ -354,23 +354,15 @@ out:
  * a single outstanding callback request at a time.
  */
 static __be32
-validate_seqid(struct nfs4_slot_table *tbl, struct cb_sequenceargs * args)
+validate_seqid(const struct nfs4_slot_table *tbl, const struct nfs4_slot *slot,
+		const struct cb_sequenceargs * args)
 {
-	struct nfs4_slot *slot;
-
-	dprintk("%s enter. slotid %u seqid %u\n",
-		__func__, args->csa_slotid, args->csa_sequenceid);
+	dprintk("%s enter. slotid %u seqid %u, slot table seqid: %u\n",
+		__func__, args->csa_slotid, args->csa_sequenceid, slot->seq_nr);
 
 	if (args->csa_slotid > tbl->server_highest_slotid)
 		return htonl(NFS4ERR_BADSLOT);
 
-	slot = tbl->slots + args->csa_slotid;
-	dprintk("%s slot table seqid: %u\n", __func__, slot->seq_nr);
-
-	/* Normal */
-	if (likely(args->csa_sequenceid == slot->seq_nr + 1))
-		goto out_ok;
-
 	/* Replay */
 	if (args->csa_sequenceid == slot->seq_nr) {
 		dprintk("%s seqid %u is a replay\n",
@@ -386,16 +378,14 @@ validate_seqid(struct nfs4_slot_table *tbl, struct cb_sequenceargs * args)
 	}
 
 	/* Wraparound */
-	if (args->csa_sequenceid == 1 && (slot->seq_nr + 1) == 0) {
-		slot->seq_nr = 1;
-		goto out_ok;
-	}
+	if (unlikely(slot->seq_nr == 0xFFFFFFFFU)) {
+		if (args->csa_sequenceid == 1)
+			return htonl(NFS4_OK);
+	} else if (likely(args->csa_sequenceid == slot->seq_nr + 1))
+		return htonl(NFS4_OK);
 
 	/* Misordered request */
 	return htonl(NFS4ERR_SEQ_MISORDERED);
-out_ok:
-	tbl->highest_used_slotid = args->csa_slotid;
-	return htonl(NFS4_OK);
 }
 
 /*
@@ -486,6 +476,13 @@ __be32 nfs4_callback_sequence(struct cb_sequenceargs *args,
 		goto out_unlock;
 	}
 
+	status = validate_seqid(tbl, slot, args);
+	if (status)
+		goto out_unlock;
+
+	cps->slotid = args->csa_slotid;
+	tbl->highest_used_slotid = args->csa_slotid;
+
 	memcpy(&res->csr_sessionid, &args->csa_sessionid,
 	       sizeof(res->csr_sessionid));
 	res->csr_sequenceid = args->csa_sequenceid;
@@ -493,12 +490,6 @@ __be32 nfs4_callback_sequence(struct cb_sequenceargs *args,
 	res->csr_highestslotid = tbl->server_highest_slotid;
 	res->csr_target_highestslotid = tbl->target_highest_slotid;
 
-	status = validate_seqid(tbl, args);
-	if (status)
-		goto out_unlock;
-
-	cps->slotid = args->csa_slotid;
-
 	/* The ca_maxresponsesize_cached is 0 with no DRC */
 	if (args->csa_cachethis != 0)
 		return htonl(NFS4ERR_REP_TOO_BIG_TO_CACHE);
@@ -518,7 +509,7 @@ __be32 nfs4_callback_sequence(struct cb_sequenceargs *args,
 	 * If CB_SEQUENCE returns an error, then the state of the slot
 	 * (sequence ID, cached reply) MUST NOT change.
 	 */
-	slot->seq_nr++;
+	slot->seq_nr = args->csa_sequenceid;
 out_unlock:
 	spin_unlock(&tbl->slot_tbl_lock);
 
-- 
2.5.0

[PATCH 5/5] NFSv4.x: Allow multiple callbacks in flight

[Trond Myklebust]

Hook the callback channel into the same session management machinery
as we use for the forward channel.

Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
---
 fs/nfs/callback.h      |  3 ++-
 fs/nfs/callback_proc.c | 14 +++++++++----
 fs/nfs/callback_xdr.c  | 12 ++++++-----
 fs/nfs/nfs4session.c   | 54 +++++++++++++++++++++++++++++++++++++++-----------
 fs/nfs/nfs4session.h   |  8 ++++++++
 5 files changed, 69 insertions(+), 22 deletions(-)

diff --git a/fs/nfs/callback.h b/fs/nfs/callback.h
index ff8195bd75ea..5fe1cecbf9f0 100644
--- a/fs/nfs/callback.h
+++ b/fs/nfs/callback.h
@@ -37,10 +37,11 @@ enum nfs4_callback_opnum {
 	OP_CB_ILLEGAL = 10044,
 };
 
+struct nfs4_slot;
 struct cb_process_state {
 	__be32			drc_status;
 	struct nfs_client	*clp;
-	u32			slotid;
+	struct nfs4_slot	*slot;
 	u32			minorversion;
 	struct net		*net;
 };
diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
index 79c93b3bbfec..efd079d28946 100644
--- a/fs/nfs/callback_proc.c
+++ b/fs/nfs/callback_proc.c
@@ -367,7 +367,7 @@ validate_seqid(const struct nfs4_slot_table *tbl, const struct nfs4_slot *slot,
 	if (args->csa_sequenceid == slot->seq_nr) {
 		dprintk("%s seqid %u is a replay\n",
 			__func__, args->csa_sequenceid);
-		if (tbl->highest_used_slotid != NFS4_NO_SLOT)
+		if (nfs4_test_locked_slot(tbl, slot->slot_nr))
 			return htonl(NFS4ERR_DELAY);
 		/* Signal process_op to set this error on next op */
 		if (args->csa_cachethis == 0)
@@ -476,12 +476,18 @@ __be32 nfs4_callback_sequence(struct cb_sequenceargs *args,
 		goto out_unlock;
 	}
 
+	status = htonl(NFS4ERR_BADSLOT);
+	slot = nfs4_lookup_slot(tbl, args->csa_slotid);
+	if (IS_ERR(slot))
+		goto out_unlock;
 	status = validate_seqid(tbl, slot, args);
 	if (status)
 		goto out_unlock;
-
-	cps->slotid = args->csa_slotid;
-	tbl->highest_used_slotid = args->csa_slotid;
+	if (!nfs4_try_to_lock_slot(tbl, slot)) {
+		status = htonl(NFS4ERR_DELAY);
+		goto out_unlock;
+	}
+	cps->slot = slot;
 
 	memcpy(&res->csr_sessionid, &args->csa_sessionid,
 	       sizeof(res->csr_sessionid));
diff --git a/fs/nfs/callback_xdr.c b/fs/nfs/callback_xdr.c
index 646cdac73488..976c90608e56 100644
--- a/fs/nfs/callback_xdr.c
+++ b/fs/nfs/callback_xdr.c
@@ -752,7 +752,8 @@ preprocess_nfs41_op(int nop, unsigned int op_nr, struct callback_op **op)
 	return htonl(NFS_OK);
 }
 
-static void nfs4_callback_free_slot(struct nfs4_session *session)
+static void nfs4_callback_free_slot(struct nfs4_session *session,
+		struct nfs4_slot *slot)
 {
 	struct nfs4_slot_table *tbl = &session->bc_slot_table;
 
@@ -761,15 +762,17 @@ static void nfs4_callback_free_slot(struct nfs4_session *session)
 	 * Let the state manager know callback processing done.
 	 * A single slot, so highest used slotid is either 0 or -1
 	 */
-	tbl->highest_used_slotid = NFS4_NO_SLOT;
+	nfs4_free_slot(tbl, slot);
 	nfs4_slot_tbl_drain_complete(tbl);
 	spin_unlock(&tbl->slot_tbl_lock);
 }
 
 static void nfs4_cb_free_slot(struct cb_process_state *cps)
 {
-	if (cps->slotid != NFS4_NO_SLOT)
-		nfs4_callback_free_slot(cps->clp->cl_session);
+	if (cps->slot) {
+		nfs4_callback_free_slot(cps->clp->cl_session, cps->slot);
+		cps->slot = NULL;
+	}
 }
 
 #else /* CONFIG_NFS_V4_1 */
@@ -893,7 +896,6 @@ static __be32 nfs4_callback_compound(struct svc_rqst *rqstp, void *argp, void *r
 	struct cb_process_state cps = {
 		.drc_status = 0,
 		.clp = NULL,
-		.slotid = NFS4_NO_SLOT,
 		.net = SVC_NET(rqstp),
 	};
 	unsigned int nops = 0;
diff --git a/fs/nfs/nfs4session.c b/fs/nfs/nfs4session.c
index e23366effcfb..332d06e64fa9 100644
--- a/fs/nfs/nfs4session.c
+++ b/fs/nfs/nfs4session.c
@@ -135,6 +135,43 @@ static struct nfs4_slot *nfs4_find_or_create_slot(struct nfs4_slot_table  *tbl,
 	return ERR_PTR(-ENOMEM);
 }
 
+static void nfs4_lock_slot(struct nfs4_slot_table *tbl,
+		struct nfs4_slot *slot)
+{
+	u32 slotid = slot->slot_nr;
+
+	__set_bit(slotid, tbl->used_slots);
+	if (slotid > tbl->highest_used_slotid ||
+	    tbl->highest_used_slotid == NFS4_NO_SLOT)
+		tbl->highest_used_slotid = slotid;
+	slot->generation = tbl->generation;
+}
+
+/*
+ * nfs4_try_to_lock_slot - Given a slot try to allocate it
+ *
+ * Note: must be called with the slot_tbl_lock held.
+ */
+bool nfs4_try_to_lock_slot(struct nfs4_slot_table *tbl, struct nfs4_slot *slot)
+{
+	if (nfs4_test_locked_slot(tbl, slot->slot_nr))
+		return false;
+	nfs4_lock_slot(tbl, slot);
+	return true;
+}
+
+/*
+ * nfs4_lookup_slot - Find a slot but don't allocate it
+ *
+ * Note: must be called with the slot_tbl_lock held.
+ */
+struct nfs4_slot *nfs4_lookup_slot(struct nfs4_slot_table *tbl, u32 slotid)
+{
+	if (slotid <= tbl->max_slotid)
+		return nfs4_find_or_create_slot(tbl, slotid, 1, GFP_NOWAIT);
+	return ERR_PTR(-E2BIG);
+}
+
 /*
  * nfs4_alloc_slot - efficiently look for a free slot
  *
@@ -153,18 +190,11 @@ struct nfs4_slot *nfs4_alloc_slot(struct nfs4_slot_table *tbl)
 		__func__, tbl->used_slots[0], tbl->highest_used_slotid,
 		tbl->max_slotid + 1);
 	slotid = find_first_zero_bit(tbl->used_slots, tbl->max_slotid + 1);
-	if (slotid > tbl->max_slotid)
-		goto out;
-	ret = nfs4_find_or_create_slot(tbl, slotid, 1, GFP_NOWAIT);
-	if (IS_ERR(ret))
-		goto out;
-	__set_bit(slotid, tbl->used_slots);
-	if (slotid > tbl->highest_used_slotid ||
-			tbl->highest_used_slotid == NFS4_NO_SLOT)
-		tbl->highest_used_slotid = slotid;
-	ret->generation = tbl->generation;
-
-out:
+	if (slotid <= tbl->max_slotid) {
+		ret = nfs4_find_or_create_slot(tbl, slotid, 1, GFP_NOWAIT);
+		if (!IS_ERR(ret))
+			nfs4_lock_slot(tbl, ret);
+	}
 	dprintk("<-- %s used_slots=%04lx highest_used=%u slotid=%u\n",
 		__func__, tbl->used_slots[0], tbl->highest_used_slotid,
 		!IS_ERR(ret) ? ret->slot_nr : NFS4_NO_SLOT);
diff --git a/fs/nfs/nfs4session.h b/fs/nfs/nfs4session.h
index e3ea2c5324d6..9d4d1af237a3 100644
--- a/fs/nfs/nfs4session.h
+++ b/fs/nfs/nfs4session.h
@@ -77,6 +77,8 @@ extern int nfs4_setup_slot_table(struct nfs4_slot_table *tbl,
 		unsigned int max_reqs, const char *queue);
 extern void nfs4_shutdown_slot_table(struct nfs4_slot_table *tbl);
 extern struct nfs4_slot *nfs4_alloc_slot(struct nfs4_slot_table *tbl);
+extern struct nfs4_slot *nfs4_lookup_slot(struct nfs4_slot_table *tbl, u32 slotid);
+extern bool nfs4_try_to_lock_slot(struct nfs4_slot_table *tbl, struct nfs4_slot *slot);
 extern void nfs4_free_slot(struct nfs4_slot_table *tbl, struct nfs4_slot *slot);
 extern void nfs4_slot_tbl_drain_complete(struct nfs4_slot_table *tbl);
 bool nfs41_wake_and_assign_slot(struct nfs4_slot_table *tbl,
@@ -125,6 +127,12 @@ static inline void nfs4_copy_sessionid(struct nfs4_sessionid *dst,
 	memcpy(dst->data, src->data, NFS4_MAX_SESSIONID_LEN);
 }
 
+static inline bool nfs4_test_locked_slot(const struct nfs4_slot_table *tbl,
+		u32 slotid)
+{
+	return test_bit(slotid, tbl->used_slots) != 0;
+}
+
 #ifdef CONFIG_CRC32
 /*
  * nfs_session_id_hash - calculate the crc32 hash for the session id
-- 
2.5.0

Re: [PATCH 5/5] NFSv4.x: Allow multiple callbacks in flight

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 6641 bytes --]

Hi Trond,

[auto build test ERROR on v4.5-rc1]
[also build test ERROR on next-20160125]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Trond-Myklebust/Convert-client-back-channel-to-use-session-slot-table/20160125-221141
config: x86_64-randconfig-x013-01250526 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All error/warnings (new ones prefixed by >>):

   In file included from include/linux/linkage.h:4:0,
                    from include/linux/kernel.h:6,
                    from fs/nfs/nfs4session.c:7:
   fs/nfs/nfs4session.c: In function 'nfs4_try_to_lock_slot':
>> fs/nfs/nfs4session.c:157:6: error: implicit declaration of function 'nfs4_test_locked_slot' [-Werror=implicit-function-declaration]
     if (nfs4_test_locked_slot(tbl, slot->slot_nr))
         ^
   include/linux/compiler.h:147:28: note: in definition of macro '__trace_if'
     if (__builtin_constant_p((cond)) ? !!(cond) :   \
                               ^
>> fs/nfs/nfs4session.c:157:2: note: in expansion of macro 'if'
     if (nfs4_test_locked_slot(tbl, slot->slot_nr))
     ^
   cc1: some warnings being treated as errors

vim +/nfs4_test_locked_slot +157 fs/nfs/nfs4session.c

     1	/*
     2	 * fs/nfs/nfs4session.c
     3	 *
     4	 * Copyright (c) 2012 Trond Myklebust <Trond.Myklebust@netapp.com>
     5	 *
     6	 */
   > 7	#include <linux/kernel.h>
     8	#include <linux/errno.h>
     9	#include <linux/string.h>
    10	#include <linux/printk.h>
    11	#include <linux/slab.h>
    12	#include <linux/sunrpc/sched.h>
    13	#include <linux/sunrpc/bc_xprt.h>
    14	#include <linux/nfs.h>
    15	#include <linux/nfs4.h>
    16	#include <linux/nfs_fs.h>
    17	#include <linux/module.h>
    18	
    19	#include "nfs4_fs.h"
    20	#include "internal.h"
    21	#include "nfs4session.h"
    22	#include "callback.h"
    23	
    24	#define NFSDBG_FACILITY		NFSDBG_STATE
    25	
    26	static void nfs4_init_slot_table(struct nfs4_slot_table *tbl, const char *queue)
    27	{
    28		tbl->highest_used_slotid = NFS4_NO_SLOT;
    29		spin_lock_init(&tbl->slot_tbl_lock);
    30		rpc_init_priority_wait_queue(&tbl->slot_tbl_waitq, queue);
    31		init_completion(&tbl->complete);
    32	}
    33	
    34	/*
    35	 * nfs4_shrink_slot_table - free retired slots from the slot table
    36	 */
    37	static void nfs4_shrink_slot_table(struct nfs4_slot_table  *tbl, u32 newsize)
    38	{
    39		struct nfs4_slot **p;
    40		if (newsize >= tbl->max_slots)
    41			return;
    42	
    43		p = &tbl->slots;
    44		while (newsize--)
    45			p = &(*p)->next;
    46		while (*p) {
    47			struct nfs4_slot *slot = *p;
    48	
    49			*p = slot->next;
    50			kfree(slot);
    51			tbl->max_slots--;
    52		}
    53	}
    54	
    55	/**
    56	 * nfs4_slot_tbl_drain_complete - wake waiters when drain is complete
    57	 * @tbl - controlling slot table
    58	 *
    59	 */
    60	void nfs4_slot_tbl_drain_complete(struct nfs4_slot_table *tbl)
    61	{
    62		if (nfs4_slot_tbl_draining(tbl))
    63			complete(&tbl->complete);
    64	}
    65	
    66	/*
    67	 * nfs4_free_slot - free a slot and efficiently update slot table.
    68	 *
    69	 * freeing a slot is trivially done by clearing its respective bit
    70	 * in the bitmap.
    71	 * If the freed slotid equals highest_used_slotid we want to update it
    72	 * so that the server would be able to size down the slot table if needed,
    73	 * otherwise we know that the highest_used_slotid is still in use.
    74	 * When updating highest_used_slotid there may be "holes" in the bitmap
    75	 * so we need to scan down from highest_used_slotid to 0 looking for the now
    76	 * highest slotid in use.
    77	 * If none found, highest_used_slotid is set to NFS4_NO_SLOT.
    78	 *
    79	 * Must be called while holding tbl->slot_tbl_lock
    80	 */
    81	void nfs4_free_slot(struct nfs4_slot_table *tbl, struct nfs4_slot *slot)
    82	{
    83		u32 slotid = slot->slot_nr;
    84	
    85		/* clear used bit in bitmap */
    86		__clear_bit(slotid, tbl->used_slots);
    87	
    88		/* update highest_used_slotid when it is freed */
    89		if (slotid == tbl->highest_used_slotid) {
    90			u32 new_max = find_last_bit(tbl->used_slots, slotid);
    91			if (new_max < slotid)
    92				tbl->highest_used_slotid = new_max;
    93			else {
    94				tbl->highest_used_slotid = NFS4_NO_SLOT;
    95				nfs4_slot_tbl_drain_complete(tbl);
    96			}
    97		}
    98		dprintk("%s: slotid %u highest_used_slotid %u\n", __func__,
    99			slotid, tbl->highest_used_slotid);
   100	}
   101	
   102	static struct nfs4_slot *nfs4_new_slot(struct nfs4_slot_table  *tbl,
   103			u32 slotid, u32 seq_init, gfp_t gfp_mask)
   104	{
   105		struct nfs4_slot *slot;
   106	
   107		slot = kzalloc(sizeof(*slot), gfp_mask);
   108		if (slot) {
   109			slot->table = tbl;
   110			slot->slot_nr = slotid;
   111			slot->seq_nr = seq_init;
   112		}
   113		return slot;
   114	}
   115	
   116	static struct nfs4_slot *nfs4_find_or_create_slot(struct nfs4_slot_table  *tbl,
   117			u32 slotid, u32 seq_init, gfp_t gfp_mask)
   118	{
   119		struct nfs4_slot **p, *slot;
   120	
   121		p = &tbl->slots;
   122		for (;;) {
   123			if (*p == NULL) {
   124				*p = nfs4_new_slot(tbl, tbl->max_slots,
   125						seq_init, gfp_mask);
   126				if (*p == NULL)
   127					break;
   128				tbl->max_slots++;
   129			}
   130			slot = *p;
   131			if (slot->slot_nr == slotid)
   132				return slot;
   133			p = &slot->next;
   134		}
   135		return ERR_PTR(-ENOMEM);
   136	}
   137	
   138	static void nfs4_lock_slot(struct nfs4_slot_table *tbl,
   139			struct nfs4_slot *slot)
   140	{
   141		u32 slotid = slot->slot_nr;
   142	
   143		__set_bit(slotid, tbl->used_slots);
   144		if (slotid > tbl->highest_used_slotid ||
   145		    tbl->highest_used_slotid == NFS4_NO_SLOT)
   146			tbl->highest_used_slotid = slotid;
   147		slot->generation = tbl->generation;
   148	}
   149	
   150	/*
   151	 * nfs4_try_to_lock_slot - Given a slot try to allocate it
   152	 *
   153	 * Note: must be called with the slot_tbl_lock held.
   154	 */
   155	bool nfs4_try_to_lock_slot(struct nfs4_slot_table *tbl, struct nfs4_slot *slot)
   156	{
 > 157		if (nfs4_test_locked_slot(tbl, slot->slot_nr))
   158			return false;
   159		nfs4_lock_slot(tbl, slot);
   160		return true;

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 23635 bytes --]

Re: [PATCH 0/5] Convert client back-channel to use session slot table

[Chuck Lever]

> On Jan 25, 2016, at 9:09 AM, Trond Myklebust <trond.myklebust@primarydata.com> wrote:
> 
> The following patchset is intended to convert the current ad-hoc backchannel
> session semantics to use the generic slot table in fs/nfs/nfs4session.c
> This should suffice to allow us to grow the size of the slot table at will.

We may be able to exercise this series at Connectathon with
some RDMA servers that advertise quite a few backchannel
session slots.

Reviewed-by: Chuck Lever <chuck.lever@oracle.com>


> Trond Myklebust (5):
>  NFSv4.x: Remove hard coded slotids in callback channel
>  NFSv4.x: CB_SEQUENCE should return NFS4ERR_DELAY if still executing
>  NFSv4.x: Enforce the ca_maxresponsesize_cached on the back channel
>  NFSv4.x: Fix wraparound issues when validing the callback sequence id
>  NFSv4.x: Allow multiple callbacks in flight
> 
> fs/nfs/callback.h      |  3 ++-
> fs/nfs/callback_proc.c | 64 ++++++++++++++++++++++++++------------------------
> fs/nfs/callback_xdr.c  | 12 ++++++----
> fs/nfs/nfs4proc.c      |  2 +-
> fs/nfs/nfs4session.c   | 54 ++++++++++++++++++++++++++++++++----------
> fs/nfs/nfs4session.h   |  8 +++++++
> 6 files changed, 93 insertions(+), 50 deletions(-)
> 
> -- 
> 2.5.0

--
Chuck Lever

Re: [PATCH 4/5] NFSv4.x: Fix wraparound issues when validing the callback sequence id

[Kinglong Mee]

On 1/25/2016 22:09, Trond Myklebust wrote:
> We need to make sure that we don't allow args->csa_sequenceid == 0.
> 
> Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
> ---
>  fs/nfs/callback_proc.c | 43 +++++++++++++++++--------------------------
>  1 file changed, 17 insertions(+), 26 deletions(-)
... snip ...
> @@ -486,6 +476,13 @@ __be32 nfs4_callback_sequence(struct cb_sequenceargs *args,
>  		goto out_unlock;
>  	}
>  
> +	status = validate_seqid(tbl, slot, args);
> +	if (status)
> +		goto out_unlock;

For NFS4ERR_RETRY_UNCACHED_REP error, nfs should initialize
cb_sequenceres information, but goto out_unlock will skip it.

thanks,
Kinglong Mee

> +
> +	cps->slotid = args->csa_slotid;
> +	tbl->highest_used_slotid = args->csa_slotid;
> +
>  	memcpy(&res->csr_sessionid, &args->csa_sessionid,
>  	       sizeof(res->csr_sessionid));
>  	res->csr_sequenceid = args->csa_sequenceid;
> @@ -493,12 +490,6 @@ __be32 nfs4_callback_sequence(struct cb_sequenceargs *args,
>  	res->csr_highestslotid = tbl->server_highest_slotid;
>  	res->csr_target_highestslotid = tbl->target_highest_slotid;
>  
> -	status = validate_seqid(tbl, args);
> -	if (status)
> -		goto out_unlock;
> -
> -	cps->slotid = args->csa_slotid;
> -
>  	/* The ca_maxresponsesize_cached is 0 with no DRC */
>  	if (args->csa_cachethis != 0)
>  		return htonl(NFS4ERR_REP_TOO_BIG_TO_CACHE);
> @@ -518,7 +509,7 @@ __be32 nfs4_callback_sequence(struct cb_sequenceargs *args,
>  	 * If CB_SEQUENCE returns an error, then the state of the slot
>  	 * (sequence ID, cached reply) MUST NOT change.
>  	 */
> -	slot->seq_nr++;
> +	slot->seq_nr = args->csa_sequenceid;
>  out_unlock:
>  	spin_unlock(&tbl->slot_tbl_lock);
>  
> 

Re: [PATCH 4/5] NFSv4.x: Fix wraparound issues when validing the callback sequence id

[Trond Myklebust]

On Sun, Jan 31, 2016 at 5:40 AM, Kinglong Mee <kinglongmee@gmail.com> wrote:
>
>
> On 1/25/2016 22:09, Trond Myklebust wrote:
> > We need to make sure that we don't allow args->csa_sequenceid == 0.
> >
> > Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
> > ---
> >  fs/nfs/callback_proc.c | 43 +++++++++++++++++--------------------------
> >  1 file changed, 17 insertions(+), 26 deletions(-)
> ... snip ...
> > @@ -486,6 +476,13 @@ __be32 nfs4_callback_sequence(struct cb_sequenceargs *args,
> >               goto out_unlock;
> >       }
> >
> > +     status = validate_seqid(tbl, slot, args);
> > +     if (status)
> > +             goto out_unlock;
>
> For NFS4ERR_RETRY_UNCACHED_REP error, nfs should initialize
> cb_sequenceres information, but goto out_unlock will skip it.
>

Good point! I've appended a patch that fixes this.

Thanks!
  Trond