NFSv4.1: fix backchannel slotid off-by-one bug

index 43926ad..54cea8a 100644
@@ -339,7 +339,7 @@ validate_seqid(struct nfs4_slot_table *tbl, struct cb_sequenceargs * args)
 	dprintk("%s enter. slotid %d seqid %d\n",
 		__func__, args->csa_slotid, args->csa_sequenceid);
 
-	if (args->csa_slotid > NFS41_BC_MAX_CALLBACKS)
+	if (args->csa_slotid >= NFS41_BC_MAX_CALLBACKS)
 		return htonl(NFS4ERR_BADSLOT);
 
 	slot = tbl->slots + args->csa_slotid;

index 54cea8a..8039a41 100644
@@ -342,7 +342,7 @@ validate_seqid(struct nfs4_slot_table *tbl, struct cb_sequenceargs * args)
 	if (args->csa_slotid >= NFS41_BC_MAX_CALLBACKS)
 		return htonl(NFS4ERR_BADSLOT);
 
-	slot = tbl->slots + args->csa_slotid;
+	slot = nfs4_lookup_slot_locked(tbl, args->csa_slotid);
 	dprintk("%s slot table seqid: %d\n", __func__, slot->seq_nr);
 
 	/* Normal */

@@ -377,6 +377,22 @@ out_ok:
 	return htonl(NFS4_OK);
 }
 
+static bool test_refer_slot(struct nfs4_slot_table *tbl,
+			    struct referring_call *ref)
+{
+	struct nfs4_slot *slot;
+	bool status = false;
+
+	spin_lock(&tbl->slot_tbl_lock);
+	if (test_bit(ref->rc_slotid, tbl->used_slots)) {
+		slot = nfs4_lookup_slot_locked(tbl, ref->rc_slotid);
+		if (slot->seq_nr == ref->rc_sequenceid)
+			status = true;
+	}
+	spin_unlock(&tbl->slot_tbl_lock);
+	return status;
+}
+
 /*
  * For each referring call triple, check the session's slot table for
  * a match.  If the slot is in use and the sequence numbers match, the

@@ -417,12 +433,7 @@ static bool referring_call_exists(struct nfs_client *clp,
 				((u32 *)&rclist->rcl_sessionid.data)[2],
 				((u32 *)&rclist->rcl_sessionid.data)[3],
 				ref->rc_sequenceid, ref->rc_slotid);
-
-			spin_lock(&tbl->slot_tbl_lock);
-			status = (test_bit(ref->rc_slotid, tbl->used_slots) &&
-				  tbl->slots[ref->rc_slotid].seq_nr ==
-					ref->rc_sequenceid);
-			spin_unlock(&tbl->slot_tbl_lock);
+			status = test_refer_slot(tbl, ref);
 			if (status)
 				goto out;
 		}

index c1a1bd8..4451abf 100644
@@ -354,6 +354,8 @@ extern int _nfs4_call_sync_session(struct rpc_clnt *clnt,
 				   struct nfs4_sequence_args *args,
 				   struct nfs4_sequence_res *res,
 				   int cache_reply);
+struct nfs4_slot *nfs4_lookup_slot_locked(struct nfs4_slot_table *tbl,
+					  u8 slotid);
 
 /*
  * Determine the device name as a string

index 693ae22..ff87169 100644
@@ -242,6 +242,7 @@ extern struct nfs4_session *nfs4_alloc_session(struct nfs_client *clp);
 extern int nfs4_proc_create_session(struct nfs_client *);
 extern int nfs4_proc_destroy_session(struct nfs4_session *);
 extern int nfs4_init_session(struct nfs_server *server);
+extern void nfs4_reduce_slots_locked(struct nfs4_slot_table *tbl);
 extern int nfs4_proc_get_lease_time(struct nfs_client *clp,
 		struct nfs_fsinfo *fsinfo);
 extern int nfs4_proc_layoutcommit(struct nfs4_layoutcommit_data *data,

452 lines not on this page · raw message

index 39914be..8260865 100644
@@ -1573,26 +1573,16 @@ static int nfs4_recall_slot(struct nfs_client *clp)
 {
 	struct nfs4_slot_table *fc_tbl = &clp->cl_session->fc_slot_table;
 	struct nfs4_channel_attrs *fc_attrs = &clp->cl_session->fc_attrs;
-	struct nfs4_slot *new, *old;
-	int i;
 
 	nfs4_begin_drain_session(clp);
-	new = kmalloc(fc_tbl->target_max_slots * sizeof(struct nfs4_slot),
-		      GFP_NOFS);
-        if (!new)
-		return -ENOMEM;
 
 	spin_lock(&fc_tbl->slot_tbl_lock);
-	for (i = 0; i < fc_tbl->target_max_slots; i++)
-		new[i].seq_nr = fc_tbl->slots[i].seq_nr;
-	old = fc_tbl->slots;
-	fc_tbl->slots = new;
 	fc_tbl->max_slots = fc_tbl->target_max_slots;
+	nfs4_reduce_slots_locked(fc_tbl);
 	fc_tbl->target_max_slots = 0;
 	fc_attrs->max_reqs = fc_tbl->max_slots;
 	spin_unlock(&fc_tbl->slot_tbl_lock);
 
-	kfree(old);
 	nfs4_end_drain_session(clp);
 	return 0;
 }

index e6161b2..f9fd96d 100644
@@ -1872,7 +1872,6 @@ static void encode_sequence(struct xdr_stream *xdr,
 #if defined(CONFIG_NFS_V4_1)
 	struct nfs4_session *session = args->sa_session;
 	struct nfs4_slot_table *tp;
-	struct nfs4_slot *slot;
 	__be32 *p;
 
 	if (!session)

@@ -1880,9 +1879,6 @@ static void encode_sequence(struct xdr_stream *xdr,
 
 	tp = &session->fc_slot_table;
 
-	WARN_ON(args->sa_slotid == NFS4_MAX_SLOT_TABLE);
-	slot = tp->slots + args->sa_slotid;
-
 	p = reserve_space(xdr, 4 + NFS4_MAX_SESSIONID_LEN + 16);
 	*p++ = cpu_to_be32(OP_SEQUENCE);
 

@@ -1896,11 +1892,11 @@ static void encode_sequence(struct xdr_stream *xdr,
 		((u32 *)session->sess_id.data)[1],
 		((u32 *)session->sess_id.data)[2],
 		((u32 *)session->sess_id.data)[3],
-		slot->seq_nr, args->sa_slotid,
+		args->sa_slot->seq_nr, args->sa_slot->slotid,
 		tp->highest_used_slotid, args->sa_cache_this);
 	p = xdr_encode_opaque_fixed(p, session->sess_id.data, NFS4_MAX_SESSIONID_LEN);
-	*p++ = cpu_to_be32(slot->seq_nr);
-	*p++ = cpu_to_be32(args->sa_slotid);
+	*p++ = cpu_to_be32(args->sa_slot->seq_nr);
+	*p++ = cpu_to_be32(args->sa_slot->slotid);
 	*p++ = cpu_to_be32(tp->highest_used_slotid);
 	*p = cpu_to_be32(args->sa_cache_this);
 	hdr->nops++;

@@ -5361,13 +5357,16 @@ static int decode_sequence(struct xdr_stream *xdr,
 	/* seqid */
 	dummy = be32_to_cpup(p++);
 	if (dummy != res->sr_slot->seq_nr) {
-		dprintk("%s Invalid sequence number\n", __func__);
+		dprintk("%s Invalid sequence number %d for "
+			"[slotid:seq_nr] [%d:%d]\n", __func__,
+			dummy, res->sr_slot->slotid, res->sr_slot->seq_nr);
 		goto out_err;
 	}
 	/* slot id */
 	dummy = be32_to_cpup(p++);
-	if (dummy != res->sr_slot - res->sr_session->fc_slot_table.slots) {
-		dprintk("%s Invalid slot id\n", __func__);
+	if (dummy != res->sr_slot->slotid) {
+		dprintk("%s Invalid slot id %d for [slotid:seq_nr] [%d:%d]\n",
+		__func__, dummy, res->sr_slot->slotid, res->sr_slot->seq_nr);
 		goto out_err;
 	}
 	/* highest slot id - currently not processed */

index b5479df..11a15c0 100644
@@ -193,12 +193,15 @@ struct nfs_server {
 
 /* Sessions */
 #define SLOT_TABLE_SZ (NFS4_MAX_SLOT_TABLE/(8*sizeof(long)))
+#define SLOT_TABLE_BITS		5
+#define SLOT_TABLE_SIZE		(1 << SLOT_TABLE_BITS)
 struct nfs4_slot_table {
-	struct nfs4_slot *slots;		/* seqid per slot */
+	struct hlist_head slots[SLOT_TABLE_SIZE];  /* seqid per slot */
 	unsigned long   used_slots[SLOT_TABLE_SZ]; /* used/unused bitmap */
 	spinlock_t	slot_tbl_lock;
 	struct rpc_wait_queue	slot_tbl_waitq;	/* allocators may wait here */
-	int		max_slots;		/* # slots in table */
+	int		allocated_slots;	/* # slots in table */
+	int		max_slots;		/* max # slots in table */
 	int		highest_used_slotid;	/* sent to server on each SEQ.
 						 * op for dynamic resizing */
 	int		target_max_slots;	/* Set by CB_RECALL_SLOT as

@@ -206,11 +209,6 @@ struct nfs4_slot_table {
 	struct completion complete;
 };
 
-static inline int slot_idx(struct nfs4_slot_table *tbl, struct nfs4_slot *sp)
-{
-	return sp - tbl->slots;
-}
-
 /*
  * Session related parameters
  */

index c74595b..1ceedc5 100644
@@ -168,12 +168,15 @@ struct nfs4_channel_attrs {
 
 /* nfs41 sessions slot seqid */
 struct nfs4_slot {
+	struct hlist_node	slot_node;
+	atomic_t		ref;
 	u32		 	seq_nr;
+	u8			slotid;
 };
 
 struct nfs4_sequence_args {
 	struct nfs4_session	*sa_session;
-	u8			sa_slotid;
+	struct nfs4_slot	*sa_slot;
 	u8			sa_cache_this;
 };
 

index 9c8afc1..deb88d9 100644
@@ -438,7 +438,7 @@ nfs4_dynamic_alloc_slot(struct nfs4_slot_table *tbl, u8 slotid)
 }
 
 /*
- * Return the slot associated with the slotid returned by nfs4_find_slot.
+ * Return the slot associated with the slotid returned by nfs4_find_slotid.
  * If the slot is not found, it has not been allocated: the NULL return
  * puts the request on the slot table waitq and dynamically allocates a
  * new slot.

@@ -522,9 +522,9 @@ void nfs4_reduce_slots_locked(struct nfs4_slot_table *tbl)
 }
 
 /*
- * nfs4_free_slot - free a slot and efficiently update slot table.
+ * nfs4_free_slotid - free a slotid and efficiently update slot table.
  *
- * freeing a slot is trivially done by clearing its respective bit
+ * freeing a slotid is trivially done by clearing its respective bit
  * in the bitmap.
  * If the freed slotid equals highest_used_slotid we want to update it
  * so that the server would be able to size down the slot table if needed,

@@ -537,7 +537,7 @@ void nfs4_reduce_slots_locked(struct nfs4_slot_table *tbl)
  * Must be called while holding tbl->slot_tbl_lock
  */
 static void
-nfs4_free_slot(struct nfs4_slot_table *tbl, u8 free_slotid)
+nfs4_free_slotid(struct nfs4_slot_table *tbl, u8 free_slotid)
 {
 	int slotid = free_slotid;
 

@@ -603,7 +603,7 @@ static void nfs41_sequence_free_slot(struct nfs4_sequence_res *res)
 	}
 
 	spin_lock(&tbl->slot_tbl_lock);
-	nfs4_free_slot(tbl, res->sr_slot->slotid);
+	nfs4_free_slotid(tbl, res->sr_slot->slotid);
 	nfs4_put_slot_locked(res->sr_slot);
 	nfs4_check_drain_fc_complete(res->sr_session);
 	spin_unlock(&tbl->slot_tbl_lock);

@@ -674,17 +674,17 @@ static int nfs4_sequence_done(struct rpc_task *task,
 }
 
 /*
- * nfs4_find_slot - efficiently look for a free slot
+ * nfs4_find_slotid - efficiently look for a free slotid
  *
- * nfs4_find_slot looks for an unset bit in the used_slots bitmap.
- * If found, we mark the slot as used, update the highest_used_slotid,
+ * nfs4_find_slotid looks for an unset bit in the used_slots bitmap.
+ * If found, we mark the slotid as used, update the highest_used_slotid,
  * and respectively set up the sequence operation args.
- * The slot number is returned if found, or NFS4_MAX_SLOT_TABLE otherwise.
+ * The slotid is returned if found, or NFS4_MAX_SLOT_TABLE otherwise.
  *
  * Note: must be called with under the slot_tbl_lock.
  */
 static u8
-nfs4_find_slot(struct nfs4_slot_table *tbl)
+nfs4_find_slotid(struct nfs4_slot_table *tbl)
 {
 	int slotid;
 	u8 ret_id = NFS4_MAX_SLOT_TABLE;

@@ -744,7 +744,7 @@ int nfs41_setup_sequence(struct nfs4_session *session,
 		return -EAGAIN;
 	}
 
-	slotid = nfs4_find_slot(tbl);
+	slotid = nfs4_find_slotid(tbl);
 	if (slotid == NFS4_MAX_SLOT_TABLE) {
 		rpc_sleep_on(&tbl->slot_tbl_waitq, task, NULL);
 		spin_unlock(&tbl->slot_tbl_lock);

@@ -756,7 +756,7 @@ int nfs41_setup_sequence(struct nfs4_session *session,
 		/* keep FIFO order */
 		rpc_task_set_priority(task, RPC_PRIORITY_PRIVILEGED);
 		rpc_sleep_on(&tbl->slot_tbl_waitq, task, NULL);
-		nfs4_free_slot(tbl, slotid);
+		nfs4_free_slotid(tbl, slotid);
 		spin_unlock(&tbl->slot_tbl_lock);
 		dprintk("%s task pid %5u sleeping for dynamic slot %d\n",
 			__func__, task->tk_pid, slotid);

index 8039a41..4e4ee2a 100644
@@ -578,8 +578,12 @@ __be32 nfs4_callback_recallslot(struct cb_recallslotargs *args, void *dummy,
 	if (args->crsa_target_max_slots == fc_tbl->max_slots)
 		goto out;
 
+        spin_lock(&fc_tbl->slot_tbl_lock);
 	fc_tbl->target_max_slots = args->crsa_target_max_slots;
-	nfs41_handle_recall_slot(cps->clp);
+        fc_tbl->max_slots = fc_tbl->target_max_slots;
+        nfs4_reduce_slots_locked(fc_tbl);
+        cps->clp->cl_session->fc_attrs.max_reqs = fc_tbl->max_slots;
+        spin_unlock(&fc_tbl->slot_tbl_lock);
 out:
 	dprintk("%s: exit with status = %d\n", __func__, ntohl(status));
 	return status;

index ff87169..a1f889a 100644
@@ -22,7 +22,6 @@ enum nfs4_client_state {
 	NFS4CLNT_DELEGRETURN,
 	NFS4CLNT_LAYOUTRECALL,
 	NFS4CLNT_SESSION_RESET,
-	NFS4CLNT_RECALL_SLOT,
 	NFS4CLNT_LEASE_CONFIRM,
 	NFS4CLNT_SERVER_SCOPE_MISMATCH,
 };

index 8260865..c08fb3b 100644
@@ -1479,12 +1479,6 @@ void nfs4_schedule_session_recovery(struct nfs4_session *session)
 }
 EXPORT_SYMBOL_GPL(nfs4_schedule_session_recovery);
 
-void nfs41_handle_recall_slot(struct nfs_client *clp)
-{
-	set_bit(NFS4CLNT_RECALL_SLOT, &clp->cl_state);
-	nfs4_schedule_state_manager(clp);
-}
-
 static void nfs4_reset_all_state(struct nfs_client *clp)
 {
 	if (test_and_set_bit(NFS4CLNT_LEASE_EXPIRED, &clp->cl_state) == 0) {

@@ -1559,8 +1553,6 @@ static int nfs4_reset_session(struct nfs_client *clp)
 		goto out;
 	}
 	clear_bit(NFS4CLNT_SESSION_RESET, &clp->cl_state);
-	/* create_session negotiated new slot table */
-	clear_bit(NFS4CLNT_RECALL_SLOT, &clp->cl_state);
 
 	 /* Let the state manager reestablish state */
 	if (!test_bit(NFS4CLNT_LEASE_EXPIRED, &clp->cl_state))

@@ -1569,28 +1561,9 @@ out:
 	return status;
 }
 
-static int nfs4_recall_slot(struct nfs_client *clp)
-{
-	struct nfs4_slot_table *fc_tbl = &clp->cl_session->fc_slot_table;
-	struct nfs4_channel_attrs *fc_attrs = &clp->cl_session->fc_attrs;
-
-	nfs4_begin_drain_session(clp);
-
-	spin_lock(&fc_tbl->slot_tbl_lock);
-	fc_tbl->max_slots = fc_tbl->target_max_slots;
-	nfs4_reduce_slots_locked(fc_tbl);
-	fc_tbl->target_max_slots = 0;
-	fc_attrs->max_reqs = fc_tbl->max_slots;
-	spin_unlock(&fc_tbl->slot_tbl_lock);
-
-	nfs4_end_drain_session(clp);
-	return 0;
-}
-
 #else /* CONFIG_NFS_V4_1 */
 static int nfs4_reset_session(struct nfs_client *clp) { return 0; }
 static int nfs4_end_drain_session(struct nfs_client *clp) { return 0; }
-static int nfs4_recall_slot(struct nfs_client *clp) { return 0; }
 #endif /* CONFIG_NFS_V4_1 */
 
 /* Set NFS4CLNT_LEASE_EXPIRED for all v4.0 errors and for recoverable errors

@@ -1699,15 +1672,6 @@ static void nfs4_state_manager(struct nfs_client *clp)
 			nfs_client_return_marked_delegations(clp);
 			continue;
 		}
-		/* Recall session slots */
-		if (test_and_clear_bit(NFS4CLNT_RECALL_SLOT, &clp->cl_state)
-		   && nfs4_has_session(clp)) {
-			status = nfs4_recall_slot(clp);
-			if (status < 0)
-				goto out_error;
-			continue;
-		}
-
 
 		nfs4_clear_state_manager_bit(clp);
 		/* Did we race with an attempt to give us more work? */