[pnfs] Server sessions slot table patches

[Benny Halevy]

Andy, my comments in "=>"
Benny

0003-Declare-server-sessionid_t-and-rename-server-session.patch:
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
@@ -369,15 +362,15 @@ add_to_sessionid_hashtbl(clientid_t *clientid, sessionid_t *sessionid)
         if (!new)
                 goto out;
 
-        memcpy(&new->clientid, clientid, sizeof(*clientid));
-        memcpy(&new->sessionid, *sessionid, sizeof(sessionid_t));
+        memcpy(&new->se_clientid, clientid, sizeof(*clientid));
+        memcpy(&new->se_sessionid, *sessionid, sizeof(sessionid_t));

=> (isn't dereferencing sessionid a bug here?)
+        memcpy(&new->se_sessionid, sessionid, sizeof(sessionid_t));


0007-Slot-table-sequence-number-processing-for-server.patch:
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
@@ -2181,17 +2200,47 @@ int
 nfsd4_sequence(struct svc_rqst *r, sessionid_t *current_sid, struct nfsd4_sequence *seq)
 {
         struct nfs41_session *elem;
-        elem = find_in_sessionid_hashtbl(&seq->sessionid);
+	struct nfs41_slot *slot;
+	int status = nfserr_stale_clientid;
 
+	nfs4_lock_state();
+        elem = find_in_sessionid_hashtbl(&seq->sessionid);

=> replace leading spaces with tab
+	elem = find_in_sessionid_hashtbl(&seq->sessionid);

@@ -2181,17 +2200,47 @@ int
 nfsd4_sequence(struct svc_rqst *r, sessionid_t *current_sid, struct nfsd4_sequence *seq)
 {
         struct nfs41_session *elem;
-        elem = find_in_sessionid_hashtbl(&seq->sessionid);
+	struct nfs41_slot *slot;
+	int status = nfserr_stale_clientid;
=>	int status = nfserr_badsession;
 
+	nfs4_lock_state();
+        elem = find_in_sessionid_hashtbl(&seq->sessionid);
         if (!elem) {
                 printk(KERN_EMERG "stale clientid supplied to sequence!!\n");
=>		printk(KERN_EMERG "bad sessionid supplied in sequence!!\n");
=>		dump_sessionid(__FUNCTION__, &seq->sessionid);
                 return nfserr_stale_clientid;
=>		goto out;
(need to unlock state)
         }
+	if (STALE_CLIENTID(&elem->se_client->cl_clientid))
=>	{
=>		printk(KERN_EMERG "stale clientid supplied to sequence!!\n");
=>		status = nfserr_stale_clientid;
+		goto out;
=>	}

+
+	/* XXX status = nfserr_badslot; */
+	status = nfserr_inval;
+	if (seq->slotid >= elem->se_fnumslots)
+		goto out;
+
+	slot = &elem->se_slots[seq->slotid];
+	dprintk("%s slotid %d \n", __FUNCTION__, seq->slotid);
=>	dprintk("%s slotid %d\n", __FUNCTION__, seq->slotid);
+	status = check_slot_seqid(seq->seqid, slot);
+	if (status == NFSERR_REPLAY_ME)
+		goto replay;
+	else if (status)
+		goto out;
+
+	/* Success! bump slot seqid */
+	slot->sl_seqid++;
 
 	memcpy(current_sid, &seq->sessionid, sizeof(sessionid_t));
 	renew_client(elem->se_client);
-	return nfs_ok;
+	status = nfs_ok;
+out:
+	dprintk("%s returns %d\n", __FUNCTION__, status);
+	nfs4_unlock_state();
+	return status;
+replay:
+	dprintk("%s REPLAY - AKKKK! no code yet! return STALE CLIENTID\n",
+					__FUNCTION__);
+	status = nfserr_stale_clientid;
=>	status = nfserr_badsession;
(stale_client_id is too strong, restarting the session should be good enough here)
+	goto out;
 }

0008-Expand-current_sid-to-include-the-current-slot.patch:
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c

=> this is not directly related but the calls to kfree() can be conditional
with no additional cost

@@ -968,8 +968,17 @@ out:
 	if (current_fh)
 		fh_put(current_fh);
 	kfree(current_fh);
=> 	if (current_fh) {
=>		fh_put(current_fh);
=>		kfree(current_fh);
=>	}
 	if (save_fh)
 		fh_put(save_fh);
=>	if (save_fh) {
=>		fh_put(save_fh);
=>		kfree(save_fh);
=>	}
-	if (current_sid)
-		kfree(current_sid);
+	if (current_ses) {
+		if (current_ses->cs_slot) {
+			if (op && op->status != nfserr_dropit) {
+				dprintk("%s SET SLOT STATE TO AVAILABLE\n",
+							__FUNCTION__);
+				current_ses->cs_slot->sl_state = NFS4_SLOT_AVAILABLE;
+			}
+			nfs41_put_session(current_ses->cs_slot->sl_session);
+		}
+		kfree(current_ses);
+	}
 	kfree(save_fh);
=> kfree(save_fh) moved into the "if (save_fh) {}" block, where it belongs
 	return status;
 }

0009-Server-exchange_id-to-draft-ietf-nfsv4-minorversion1.patch:
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
@@ -443,16 +446,28 @@ check:
 	return elem;
 }
 
-void
-release_session(struct nfs41_session *ses)
+static void
+nfs41_free_session(struct kref *kref)
 {
+	struct nfs41_session *ses = container_of(kref, struct nfs41_session, se_ref);
+
 	list_del(&ses->se_hash);
 	list_del(&ses->se_perclnt);
-	if (ses->se_slots)
-		kfree(ses->se_slots);
=> I don't get it, this was added in patch 0008 and removed here,
=> who frees se_slots then?
 	kfree(ses);
 }
 

William A. (Andy) Adamson wrote:
> hi
> 
> here are 9 patches that implement the server slot table management. there is
> not DRC yet, but this code has been developed with the DRC in mind. This
> code almost passes the connectathon tests on the 4.1-sessions branch with
> the client patches i just sent out.
> 
> it fails the general connectathon tests with an NFS4ERR_SEQ_MISORDERED. i
> note that the client mostly uses slot 0, and occasionally uses slot 1. the
> slot 1 sequence is out of order - i don't know if it is the client or
> server.
> 
> please review the patches...
> 
> thanks
> 
> -->Andy
> 
> 
> 
> ------------------------------------------------------------------------
> 
> _______________________________________________
> pNFS mailing list
> pNFS at linux-nfs.org
> http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs