diff mbox

[v1,09/19] NFS: Add a "struct nfs_server *" argument to nfs4_sequence_done()

Message ID 20130712163245.1444.48096.stgit@seurat.1015granger.net (mailing list archive)
State New, archived
Headers show

Commit Message

Chuck Lever III July 12, 2013, 4:32 p.m. UTC 
For NFSv4.1, a session slot table reference is passed to
nfs4_sequence_done().

For NFSv4.0, transport state is managed in the nfs_client that owns
this nfs_server.  The nfs_server struct must be passed to
nfs4_sequence_done() to make it available to the functions dealing
with transport blocking.

Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfs/nfs4proc.c |   46 ++++++++++++++++++++++++++--------------------
 1 file changed, 26 insertions(+), 20 deletions(-)


--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Trond Myklebust July 22, 2013, 7:27 p.m. UTC | #1 
On Fri, 2013-07-12 at 12:32 -0400, Chuck Lever wrote:
> For NFSv4.1, a session slot table reference is passed to

> nfs4_sequence_done().

> 

> For NFSv4.0, transport state is managed in the nfs_client that owns

> this nfs_server.  The nfs_server struct must be passed to

> nfs4_sequence_done() to make it available to the functions dealing

> with transport blocking.


Why? Can't 4.0 just reuse the existing v4.1 session slot table for this?

-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com
Chuck Lever III July 24, 2013, 10:04 p.m. UTC | #2 
On Jul 22, 2013, at 3:27 PM, "Myklebust, Trond" <Trond.Myklebust@netapp.com> wrote:

> On Fri, 2013-07-12 at 12:32 -0400, Chuck Lever wrote:
>> For NFSv4.1, a session slot table reference is passed to
>> nfs4_sequence_done().
>> 
>> For NFSv4.0, transport state is managed in the nfs_client that owns
>> this nfs_server.  The nfs_server struct must be passed to
>> nfs4_sequence_done() to make it available to the functions dealing
>> with transport blocking.
> 
> Why? Can't 4.0 just reuse the existing v4.1 session slot table for this?

Before commit 774d5f14 "NFSv4.1 Fix a pNFS session draining deadlock," Mon May 20 14:13:50 2013, the DRAINING bit was in nfs4_session, not in nfs4_slot_table.  Before that commit, I would have had to enable most of the NFSv4.1 sessions code to re-use it for NFSv4.0 transport blocking.

(And btw, thanks Andy!  774d5f14 is a very useful refactoring).

I spent the last couple of days re-implementing my NFSv4.0 transport blocking mechanism based on the slot table abstraction, rather than adding similar fields to the nfs_client.  At this point, it looks like a workable approach, but I won't get it finished and tested before I head to Berlin.  I may have some time to continue work while I'm there.

Since our town hall meeting is cancelled this week, let me report on progress with migration support.

I was hoping to publish my 3.11-rc port of the migration patches today for another round of review, but I'm not going to make it due to the complexity of rebasing the transport blocking mechanism directly on struct nfs4_slot_table.  It will be at the top of my list when I get back.

I was able to check off some of the test cases I mentioned in the cover letter for the v1 patch series:

Migration with no TSM:  identified and fixed a client bug, then was able to witness state recovery during a successful migration.  The server prototype currently reports NFS4ERR_EXPIRED, so state recovery in this case involves OPEN(CLAIM_NULL) - the server is not in a grace period, and it is possible for clients to lose their locks.

Migration during a lock-intensive workload:  the server prototype is still attempting to put off client requests without using NFS4ERR_DELAY, which causes the client workload to terminate; waiting for that to be addressed, then will try again.

Migration recovery failure:  identified the need for more infrastructure to communicate state manager failure back to forward NFS processes.  That infrastructure has been added, but not yet tested.

The idea is that all I/O on the mount should fail after a migration, but an admin should be able to unmount cleanly.  In other words, manual recovery by remounting the FSID on the destination server, which is what would happen in the pre-migration world.

Migration with Kerberos:  exploring KDC resources available for testing my client with the server prototypes that are located in our Austin lab.
diff mbox

Patch

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 0c1fa13..20768e8 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -553,8 +553,9 @@  out_retry:
 	return 0;
 }
 
-static int nfs4_sequence_done(struct rpc_task *task,
-			       struct nfs4_sequence_res *res)
+static int nfs4_sequence_done(const struct nfs_server *server,
+			      struct rpc_task *task,
+			      struct nfs4_sequence_res *res)
 {
 	if (res->sr_slot == NULL)
 		return 1;
@@ -726,8 +727,9 @@  static int nfs4_setup_sequence(const struct nfs_server *server,
 	return 0;
 }
 
-static int nfs4_sequence_done(struct rpc_task *task,
-			       struct nfs4_sequence_res *res)
+static int nfs4_sequence_done(const struct nfs_server *server,
+			      struct rpc_task *task,
+			      struct nfs4_sequence_res *res)
 {
 	return 1;
 }
@@ -745,7 +747,7 @@  static void nfs40_call_sync_done(struct rpc_task *task, void *calldata)
 {
 	struct nfs4_call_sync_data *data = calldata;
 
-	nfs4_sequence_done(task, data->seq_res);
+	nfs4_sequence_done(data->seq_server, task, data->seq_res);
 }
 
 static const struct rpc_call_ops nfs40_call_sync_ops = {
@@ -1622,7 +1624,7 @@  unlock_no_action:
 out_no_action:
 	task->tk_action = NULL;
 out_wait:
-	nfs4_sequence_done(task, &data->o_res.seq_res);
+	nfs4_sequence_done(data->o_arg.server, task, &data->o_res.seq_res);
 }
 
 static void nfs4_open_done(struct rpc_task *task, void *calldata)
@@ -1631,7 +1633,7 @@  static void nfs4_open_done(struct rpc_task *task, void *calldata)
 
 	data->rpc_status = task->tk_status;
 
-	if (!nfs4_sequence_done(task, &data->o_res.seq_res))
+	if (!nfs4_sequence_done(data->o_arg.server, task, &data->o_res.seq_res))
 		return;
 
 	if (task->tk_status == 0) {
@@ -2294,7 +2296,7 @@  static void nfs4_close_done(struct rpc_task *task, void *data)
 	struct nfs_server *server = NFS_SERVER(calldata->inode);
 
 	dprintk("%s: begin!\n", __func__);
-	if (!nfs4_sequence_done(task, &calldata->res.seq_res))
+	if (!nfs4_sequence_done(server, task, &calldata->res.seq_res))
 		return;
         /* hmm. we are done with the inode, and in the process of freeing
 	 * the state_owner. we keep this around to process errors
@@ -2381,7 +2383,7 @@  static void nfs4_close_prepare(struct rpc_task *task, void *data)
 out_no_action:
 	task->tk_action = NULL;
 out_wait:
-	nfs4_sequence_done(task, &calldata->res.seq_res);
+	nfs4_sequence_done(NFS_SERVER(inode), task, &calldata->res.seq_res);
 }
 
 static const struct rpc_call_ops nfs4_close_ops = {
@@ -3149,7 +3151,7 @@  static int nfs4_proc_unlink_done(struct rpc_task *task, struct inode *dir)
 {
 	struct nfs_removeres *res = task->tk_msg.rpc_resp;
 
-	if (!nfs4_sequence_done(task, &res->seq_res))
+	if (!nfs4_sequence_done(res->server, task, &res->seq_res))
 		return 0;
 	if (nfs4_async_handle_error(task, res->server, NULL) == -EAGAIN)
 		return 0;
@@ -3181,7 +3183,7 @@  static int nfs4_proc_rename_done(struct rpc_task *task, struct inode *old_dir,
 {
 	struct nfs_renameres *res = task->tk_msg.rpc_resp;
 
-	if (!nfs4_sequence_done(task, &res->seq_res))
+	if (!nfs4_sequence_done(res->server, task, &res->seq_res))
 		return 0;
 	if (nfs4_async_handle_error(task, res->server, NULL) == -EAGAIN)
 		return 0;
@@ -3705,10 +3707,11 @@  static bool nfs4_read_stateid_changed(struct rpc_task *task,
 
 static int nfs4_read_done(struct rpc_task *task, struct nfs_read_data *data)
 {
+	struct nfs_server *server = NFS_SERVER(data->header->inode);
 
 	dprintk("--> %s\n", __func__);
 
-	if (!nfs4_sequence_done(task, &data->res.seq_res))
+	if (!nfs4_sequence_done(server, task, &data->res.seq_res))
 		return -EAGAIN;
 	if (nfs4_read_stateid_changed(task, &data->args))
 		return -EAGAIN;
@@ -3753,7 +3756,6 @@  static int nfs4_write_done_cb(struct rpc_task *task, struct nfs_write_data *data
 static bool nfs4_write_stateid_changed(struct rpc_task *task,
 		struct nfs_writeargs *args)
 {
-
 	if (!nfs4_error_stateid_expired(task->tk_status) ||
 		nfs4_stateid_is_current(&args->stateid,
 				args->context,
@@ -3766,7 +3768,9 @@  static bool nfs4_write_stateid_changed(struct rpc_task *task,
 
 static int nfs4_write_done(struct rpc_task *task, struct nfs_write_data *data)
 {
-	if (!nfs4_sequence_done(task, &data->res.seq_res))
+	struct nfs_server *server = NFS_SERVER(data->header->inode);
+
+	if (!nfs4_sequence_done(server, task, &data->res.seq_res))
 		return -EAGAIN;
 	if (nfs4_write_stateid_changed(task, &data->args))
 		return -EAGAIN;
@@ -3839,7 +3843,9 @@  static int nfs4_commit_done_cb(struct rpc_task *task, struct nfs_commit_data *da
 
 static int nfs4_commit_done(struct rpc_task *task, struct nfs_commit_data *data)
 {
-	if (!nfs4_sequence_done(task, &data->res.seq_res))
+	struct nfs_server *server = NFS_SERVER(data->inode);
+
+	if (!nfs4_sequence_done(server, task, &data->res.seq_res))
 		return -EAGAIN;
 	return data->commit_done_cb(task, data);
 }
@@ -4439,7 +4445,7 @@  static void nfs4_delegreturn_done(struct rpc_task *task, void *calldata)
 {
 	struct nfs4_delegreturndata *data = calldata;
 
-	if (!nfs4_sequence_done(task, &data->res.seq_res))
+	if (!nfs4_sequence_done(data->res.server, task, &data->res.seq_res))
 		return;
 
 	switch (task->tk_status) {
@@ -4690,7 +4696,7 @@  static void nfs4_locku_done(struct rpc_task *task, void *data)
 {
 	struct nfs4_unlockdata *calldata = data;
 
-	if (!nfs4_sequence_done(task, &calldata->res.seq_res))
+	if (!nfs4_sequence_done(calldata->server, task, &calldata->res.seq_res))
 		return;
 	switch (task->tk_status) {
 		case 0:
@@ -4730,7 +4736,7 @@  static void nfs4_locku_prepare(struct rpc_task *task, void *data)
 out_no_action:
 	task->tk_action = NULL;
 out_wait:
-	nfs4_sequence_done(task, &calldata->res.seq_res);
+	nfs4_sequence_done(calldata->server, task, &calldata->res.seq_res);
 }
 
 static const struct rpc_call_ops nfs4_locku_ops = {
@@ -4905,7 +4911,7 @@  out_release_open_seqid:
 out_release_lock_seqid:
 	nfs_release_seqid(data->arg.lock_seqid);
 out_wait:
-	nfs4_sequence_done(task, &data->res.seq_res);
+	nfs4_sequence_done(data->server, task, &data->res.seq_res);
 	dprintk("%s: done!, ret = %d\n", __func__, data->rpc_status);
 }
 
@@ -4915,7 +4921,7 @@  static void nfs4_lock_done(struct rpc_task *task, void *calldata)
 
 	dprintk("%s: begin!\n", __func__);
 
-	if (!nfs4_sequence_done(task, &data->res.seq_res))
+	if (!nfs4_sequence_done(data->server, task, &data->res.seq_res))
 		return;
 
 	data->rpc_status = task->tk_status;