[PATCH 01/15] NFSv41: Fix a memory leak in nfs41_proc_async_sequence()

[PATCH 01/15] NFSv41: Fix a memory leak in nfs41_proc_async_sequence()

[Trond Myklebust]

If the call to rpc_call_async() fails, then the arguments will not be
freed, since there will be no call to nfs41_sequence_call_done

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---
 fs/nfs/nfs4proc.c |   38 ++++++++++++++++++++------------------
 1 files changed, 20 insertions(+), 18 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 70015dd..bb70ff7 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -5078,18 +5078,27 @@ static int nfs4_proc_sequence(struct nfs_client *clp, struct rpc_cred *cred)
 				       &res, args.sa_cache_this, 1);
 }
 
+struct nfs4_sequence_data {
+	struct nfs_client *clp;
+	struct nfs4_sequence_args args;
+	struct nfs4_sequence_res res;
+};
+
 static void nfs41_sequence_release(void *data)
 {
-	struct nfs_client *clp = (struct nfs_client *)data;
+	struct nfs4_sequence_data *calldata = data;
+	struct nfs_client *clp = calldata->clp;
 
 	if (atomic_read(&clp->cl_count) > 1)
 		nfs4_schedule_state_renewal(clp);
 	nfs_put_client(clp);
+	kfree(calldata);
 }
 
 static void nfs41_sequence_call_done(struct rpc_task *task, void *data)
 {
-	struct nfs_client *clp = (struct nfs_client *)data;
+	struct nfs4_sequence_data *calldata = data;
+	struct nfs_client *clp = calldata->clp;
 
 	nfs41_sequence_done(clp, task->tk_msg.rpc_resp, task->tk_status);
 
@@ -5106,19 +5115,16 @@ static void nfs41_sequence_call_done(struct rpc_task *task, void *data)
 	}
 	dprintk("%s rpc_cred %p\n", __func__, task->tk_msg.rpc_cred);
 out:
-	kfree(task->tk_msg.rpc_argp);
-	kfree(task->tk_msg.rpc_resp);
-
 	dprintk("<-- %s\n", __func__);
 }
 
 static void nfs41_sequence_prepare(struct rpc_task *task, void *data)
 {
-	struct nfs_client *clp;
+	struct nfs4_sequence_data *calldata = data;
+	struct nfs_client *clp = calldata->clp;
 	struct nfs4_sequence_args *args;
 	struct nfs4_sequence_res *res;
 
-	clp = (struct nfs_client *)data;
 	args = task->tk_msg.rpc_argp;
 	res = task->tk_msg.rpc_resp;
 
@@ -5136,8 +5142,7 @@ static const struct rpc_call_ops nfs41_sequence_ops = {
 static int nfs41_proc_async_sequence(struct nfs_client *clp,
 				     struct rpc_cred *cred)
 {
-	struct nfs4_sequence_args *args;
-	struct nfs4_sequence_res *res;
+	struct nfs4_sequence_data *calldata;
 	struct rpc_message msg = {
 		.rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_SEQUENCE],
 		.rpc_cred = cred,
@@ -5145,20 +5150,17 @@ static int nfs41_proc_async_sequence(struct nfs_client *clp,
 
 	if (!atomic_inc_not_zero(&clp->cl_count))
 		return -EIO;
-	args = kzalloc(sizeof(*args), GFP_NOFS);
-	res = kzalloc(sizeof(*res), GFP_NOFS);
-	if (!args || !res) {
-		kfree(args);
-		kfree(res);
+	calldata = kmalloc(sizeof(*calldata), GFP_NOFS);
+	if (calldata == NULL) {
 		nfs_put_client(clp);
 		return -ENOMEM;
 	}
-	res->sr_slotid = NFS4_MAX_SLOT_TABLE;
-	msg.rpc_argp = args;
-	msg.rpc_resp = res;
+	msg.rpc_argp = &calldata->args;
+	msg.rpc_resp = &calldata->res;
+	calldata->clp = clp;
 
 	return rpc_call_async(clp->cl_rpcclient, &msg, RPC_TASK_SOFT,
-			      &nfs41_sequence_ops, (void *)clp);
+			      &nfs41_sequence_ops, calldata);
 }
 
 struct nfs4_reclaim_complete_data {
-- 
1.7.0.1

[PATCH 02/15] NFSv4.1: Clean up nfs4_setup_sequence

[Trond Myklebust]

Firstly, there is little point in first zeroing out the entire struct
nfs4_sequence_res, and then initialising all fields save one. Just
initialise the last field to zero...

Secondly, nfs41_setup_sequence() has only 2 possible return values: 0, or
-EAGAIN, so there is no 'terminate rpc task' case.

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---
 fs/nfs/nfs4proc.c |    7 +------
 1 files changed, 1 insertions(+), 6 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index bb70ff7..a7a2032 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -480,7 +480,6 @@ static int nfs41_setup_sequence(struct nfs4_session *session,
 	if (res->sr_slotid != NFS4_MAX_SLOT_TABLE)
 		return 0;
 
-	memset(res, 0, sizeof(*res));
 	res->sr_slotid = NFS4_MAX_SLOT_TABLE;
 	tbl = &session->fc_slot_table;
 
@@ -525,6 +524,7 @@ static int nfs41_setup_sequence(struct nfs4_session *session,
 	res->sr_session = session;
 	res->sr_slotid = slotid;
 	res->sr_renewal_time = jiffies;
+	res->sr_status_flags = 0;
 	/*
 	 * sr_status is only set in decode_sequence, and so will remain
 	 * set to 1 if an rpc level failure occurs.
@@ -548,11 +548,6 @@ int nfs4_setup_sequence(struct nfs_client *clp,
 		goto out;
 	ret = nfs41_setup_sequence(clp->cl_session, args, res, cache_reply,
 				   task);
-	if (ret && ret != -EAGAIN) {
-		/* terminate rpc task */
-		task->tk_status = ret;
-		task->tk_action = NULL;
-	}
 out:
 	dprintk("<-- %s status=%d\n", __func__, ret);
 	return ret;
-- 
1.7.0.1

[PATCH 03/15] NFSv4.1: Simplify nfs41_sequence_done()

[Trond Myklebust]

Nobody uses the rpc_status parameter.

It is not obvious why we need the struct nfs_client argument either, when
we already have that information in the session.

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---
 fs/nfs/nfs4proc.c |   26 ++++++++++++--------------
 1 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index a7a2032..67b2359 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -370,12 +370,11 @@ static void nfs41_check_drain_session_complete(struct nfs4_session *ses)
 	complete(&ses->complete);
 }
 
-static void nfs41_sequence_free_slot(const struct nfs_client *clp,
-			      struct nfs4_sequence_res *res)
+static void nfs41_sequence_free_slot(struct nfs4_sequence_res *res)
 {
 	struct nfs4_slot_table *tbl;
 
-	tbl = &clp->cl_session->fc_slot_table;
+	tbl = &res->sr_session->fc_slot_table;
 	if (res->sr_slotid == NFS4_MAX_SLOT_TABLE) {
 		/* just wake up the next guy waiting since
 		 * we may have not consumed a slot after all */
@@ -385,14 +384,12 @@ static void nfs41_sequence_free_slot(const struct nfs_client *clp,
 
 	spin_lock(&tbl->slot_tbl_lock);
 	nfs4_free_slot(tbl, res->sr_slotid);
-	nfs41_check_drain_session_complete(clp->cl_session);
+	nfs41_check_drain_session_complete(res->sr_session);
 	spin_unlock(&tbl->slot_tbl_lock);
 	res->sr_slotid = NFS4_MAX_SLOT_TABLE;
 }
 
-static void nfs41_sequence_done(struct nfs_client *clp,
-				struct nfs4_sequence_res *res,
-				int rpc_status)
+static void nfs41_sequence_done(struct nfs4_sequence_res *res)
 {
 	unsigned long timestamp;
 	struct nfs4_slot_table *tbl;
@@ -413,7 +410,8 @@ static void nfs41_sequence_done(struct nfs_client *clp,
 
 	/* Check the SEQUENCE operation status */
 	if (res->sr_status == 0) {
-		tbl = &clp->cl_session->fc_slot_table;
+		struct nfs_client *clp = res->sr_session->clp;
+		tbl = &res->sr_session->fc_slot_table;
 		slot = tbl->slots + res->sr_slotid;
 		/* Update the slot's sequence and clientid lease timer */
 		++slot->seq_nr;
@@ -429,7 +427,7 @@ static void nfs41_sequence_done(struct nfs_client *clp,
 out:
 	/* The session may be reset by one of the error handlers. */
 	dprintk("%s: Error %d free the slot \n", __func__, res->sr_status);
-	nfs41_sequence_free_slot(clp, res);
+	nfs41_sequence_free_slot(res);
 }
 
 /*
@@ -582,7 +580,7 @@ static void nfs41_call_sync_done(struct rpc_task *task, void *calldata)
 {
 	struct nfs41_call_sync_data *data = calldata;
 
-	nfs41_sequence_done(data->clp, data->seq_res, task->tk_status);
+	nfs41_sequence_done(data->seq_res);
 }
 
 struct rpc_call_ops nfs41_call_sync_ops = {
@@ -662,7 +660,7 @@ static void nfs4_sequence_done(const struct nfs_server *server,
 {
 #ifdef CONFIG_NFS_V4_1
 	if (nfs4_has_session(server->nfs_client))
-		nfs41_sequence_done(server->nfs_client, res, rpc_status);
+		nfs41_sequence_done(res);
 #endif /* CONFIG_NFS_V4_1 */
 }
 
@@ -4606,7 +4604,7 @@ static void nfs4_get_lease_time_done(struct rpc_task *task, void *calldata)
 			(struct nfs4_get_lease_time_data *)calldata;
 
 	dprintk("--> %s\n", __func__);
-	nfs41_sequence_done(data->clp, &data->res->lr_seq_res, task->tk_status);
+	nfs41_sequence_done(&data->res->lr_seq_res);
 	switch (task->tk_status) {
 	case -NFS4ERR_DELAY:
 	case -NFS4ERR_GRACE:
@@ -5095,7 +5093,7 @@ static void nfs41_sequence_call_done(struct rpc_task *task, void *data)
 	struct nfs4_sequence_data *calldata = data;
 	struct nfs_client *clp = calldata->clp;
 
-	nfs41_sequence_done(clp, task->tk_msg.rpc_resp, task->tk_status);
+	nfs41_sequence_done(task->tk_msg.rpc_resp);
 
 	if (task->tk_status < 0) {
 		dprintk("%s ERROR %d\n", __func__, task->tk_status);
@@ -5183,7 +5181,7 @@ static void nfs4_reclaim_complete_done(struct rpc_task *task, void *data)
 	struct nfs4_sequence_res *res = &calldata->res.seq_res;
 
 	dprintk("--> %s\n", __func__);
-	nfs41_sequence_done(clp, res, task->tk_status);
+	nfs41_sequence_done(res);
 	switch (task->tk_status) {
 	case 0:
 	case -NFS4ERR_COMPLETE_ALREADY:
-- 
1.7.0.1

[PATCH 04/15] NFSv4: Kill nfs4_async_handle_error() abuses by NFSv4.1

[Trond Myklebust]

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---
 fs/nfs/nfs4proc.c |   78 ++++++++++++++++++++++++++++-------------------------
 1 files changed, 41 insertions(+), 37 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 67b2359..7034188 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -3457,9 +3457,11 @@ static int nfs4_proc_set_acl(struct inode *inode, const void *buf, size_t buflen
 }
 
 static int
-_nfs4_async_handle_error(struct rpc_task *task, const struct nfs_server *server, struct nfs_client *clp, struct nfs4_state *state)
+nfs4_async_handle_error(struct rpc_task *task, const struct nfs_server *server, struct nfs4_state *state)
 {
-	if (!clp || task->tk_status >= 0)
+	struct nfs_client *clp = server->nfs_client;
+
+	if (task->tk_status >= 0)
 		return 0;
 	switch(task->tk_status) {
 		case -NFS4ERR_ADMIN_REVOKED:
@@ -3491,8 +3493,7 @@ _nfs4_async_handle_error(struct rpc_task *task, const struct nfs_server *server,
 			return -EAGAIN;
 #endif /* CONFIG_NFS_V4_1 */
 		case -NFS4ERR_DELAY:
-			if (server)
-				nfs_inc_server_stats(server, NFSIOS_DELAY);
+			nfs_inc_server_stats(server, NFSIOS_DELAY);
 		case -NFS4ERR_GRACE:
 		case -EKEYEXPIRED:
 			rpc_delay(task, NFS4_POLL_RETRY_MAX);
@@ -3513,12 +3514,6 @@ do_state_recovery:
 	return -EAGAIN;
 }
 
-static int
-nfs4_async_handle_error(struct rpc_task *task, const struct nfs_server *server, struct nfs4_state *state)
-{
-	return _nfs4_async_handle_error(task, server, server->nfs_client, state);
-}
-
 int nfs4_proc_setclientid(struct nfs_client *clp, u32 program,
 		unsigned short port, struct rpc_cred *cred,
 		struct nfs4_setclientid_res *res)
@@ -5088,6 +5083,19 @@ static void nfs41_sequence_release(void *data)
 	kfree(calldata);
 }
 
+static int nfs41_sequence_handle_errors(struct rpc_task *task, struct nfs_client *clp)
+{
+	switch(task->tk_status) {
+	case -NFS4ERR_DELAY:
+	case -EKEYEXPIRED:
+		rpc_delay(task, NFS4_POLL_RETRY_MAX);
+		return -EAGAIN;
+	default:
+		nfs4_schedule_state_recovery(clp);
+	}
+	return 0;
+}
+
 static void nfs41_sequence_call_done(struct rpc_task *task, void *data)
 {
 	struct nfs4_sequence_data *calldata = data;
@@ -5100,9 +5108,8 @@ static void nfs41_sequence_call_done(struct rpc_task *task, void *data)
 		if (atomic_read(&clp->cl_count) == 1)
 			goto out;
 
-		if (_nfs4_async_handle_error(task, NULL, clp, NULL)
-								== -EAGAIN) {
-			nfs_restart_rpc(task, clp);
+		if (nfs41_sequence_handle_errors(task, clp) == -EAGAIN) {
+			rpc_restart_call_prepare(task);
 			return;
 		}
 	}
@@ -5174,6 +5181,23 @@ static void nfs4_reclaim_complete_prepare(struct rpc_task *task, void *data)
 	rpc_call_start(task);
 }
 
+static int nfs41_reclaim_complete_handle_errors(struct rpc_task *task, struct nfs_client *clp)
+{
+	switch(task->tk_status) {
+	case 0:
+	case -NFS4ERR_COMPLETE_ALREADY:
+	case -NFS4ERR_WRONG_CRED: /* What to do here? */
+		break;
+	case -NFS4ERR_DELAY:
+	case -EKEYEXPIRED:
+		rpc_delay(task, NFS4_POLL_RETRY_MAX);
+		return -EAGAIN;
+	default:
+		nfs4_schedule_state_recovery(clp);
+	}
+	return 0;
+}
+
 static void nfs4_reclaim_complete_done(struct rpc_task *task, void *data)
 {
 	struct nfs4_reclaim_complete_data *calldata = data;
@@ -5182,31 +5206,11 @@ static void nfs4_reclaim_complete_done(struct rpc_task *task, void *data)
 
 	dprintk("--> %s\n", __func__);
 	nfs41_sequence_done(res);
-	switch (task->tk_status) {
-	case 0:
-	case -NFS4ERR_COMPLETE_ALREADY:
-		break;
-	case -NFS4ERR_BADSESSION:
-	case -NFS4ERR_DEADSESSION:
-		/*
-		 * Handle the session error, but do not retry the operation, as
-		 * we have no way of telling whether the clientid had to be
-		 * reset before we got our reply.  If reset, a new wave of
-		 * reclaim operations will follow, containing their own reclaim
-		 * complete.  We don't want our retry to get on the way of
-		 * recovery by incorrectly indicating to the server that we're
-		 * done reclaiming state since the process had to be restarted.
-		 */
-		_nfs4_async_handle_error(task, NULL, clp, NULL);
-		break;
-	default:
-		if (_nfs4_async_handle_error(
-				task, NULL, clp, NULL) == -EAGAIN) {
-			rpc_restart_call_prepare(task);
-			return;
-		}
-	}
 
+	if (nfs41_reclaim_complete_handle_errors(task, clp) == -EAGAIN) {
+		rpc_restart_call_prepare(task);
+		return;
+	}
 	dprintk("<-- %s\n", __func__);
 }
 
-- 
1.7.0.1

[PATCH 05/15] NFSv4.1: Merge the nfs41_proc_async_sequence() and nfs4_proc_sequence()

[Trond Myklebust]

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---
 fs/nfs/nfs4proc.c |   68 ++++++++++++++++++++++++++++++++++------------------
 1 files changed, 44 insertions(+), 24 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 7034188..ae2651e 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -5048,24 +5048,6 @@ int nfs4_init_session(struct nfs_server *server)
 /*
  * Renew the cl_session lease.
  */
-static int nfs4_proc_sequence(struct nfs_client *clp, struct rpc_cred *cred)
-{
-	struct nfs4_sequence_args args;
-	struct nfs4_sequence_res res;
-
-	struct rpc_message msg = {
-		.rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_SEQUENCE],
-		.rpc_argp = &args,
-		.rpc_resp = &res,
-		.rpc_cred = cred,
-	};
-
-	args.sa_cache_this = 0;
-
-	return nfs4_call_sync_sequence(clp, clp->cl_rpcclient, &msg, &args,
-				       &res, args.sa_cache_this, 1);
-}
-
 struct nfs4_sequence_data {
 	struct nfs_client *clp;
 	struct nfs4_sequence_args args;
@@ -5139,28 +5121,66 @@ static const struct rpc_call_ops nfs41_sequence_ops = {
 	.rpc_release = nfs41_sequence_release,
 };
 
-static int nfs41_proc_async_sequence(struct nfs_client *clp,
-				     struct rpc_cred *cred)
+static struct rpc_task *_nfs41_proc_sequence(struct nfs_client *clp, struct rpc_cred *cred)
 {
 	struct nfs4_sequence_data *calldata;
 	struct rpc_message msg = {
 		.rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_SEQUENCE],
 		.rpc_cred = cred,
 	};
+	struct rpc_task_setup task_setup_data = {
+		.rpc_client = clp->cl_rpcclient,
+		.rpc_message = &msg,
+		.callback_ops = &nfs41_sequence_ops,
+		.flags = RPC_TASK_ASYNC | RPC_TASK_SOFT,
+	};
 
 	if (!atomic_inc_not_zero(&clp->cl_count))
-		return -EIO;
+		return ERR_PTR(-EIO);
 	calldata = kmalloc(sizeof(*calldata), GFP_NOFS);
 	if (calldata == NULL) {
 		nfs_put_client(clp);
-		return -ENOMEM;
+		return ERR_PTR(-ENOMEM);
 	}
 	msg.rpc_argp = &calldata->args;
 	msg.rpc_resp = &calldata->res;
 	calldata->clp = clp;
+	task_setup_data.callback_data = calldata;
 
-	return rpc_call_async(clp->cl_rpcclient, &msg, RPC_TASK_SOFT,
-			      &nfs41_sequence_ops, calldata);
+	return rpc_run_task(&task_setup_data);
+}
+
+static int nfs41_proc_async_sequence(struct nfs_client *clp, struct rpc_cred *cred)
+{
+	struct rpc_task *task;
+	int ret = 0;
+
+	task = _nfs41_proc_sequence(clp, cred);
+	if (IS_ERR(task))
+		ret = PTR_ERR(task);
+	else
+		rpc_put_task(task);
+	dprintk("<-- %s status=%d\n", __func__, ret);
+	return ret;
+}
+
+static int nfs4_proc_sequence(struct nfs_client *clp, struct rpc_cred *cred)
+{
+	struct rpc_task *task;
+	int ret;
+
+	task = _nfs41_proc_sequence(clp, cred);
+	if (IS_ERR(task)) {
+		ret = PTR_ERR(task);
+		goto out;
+	}
+	ret = rpc_wait_for_completion_task(task);
+	if (!ret)
+		ret = task->tk_status;
+	rpc_put_task(task);
+out:
+	dprintk("<-- %s status=%d\n", __func__, ret);
+	return ret;
 }
 
 struct nfs4_reclaim_complete_data {
-- 
1.7.0.1

[PATCH 06/15] NFSv4.1: Make nfs4_setup_sequence take a nfs_server argument

[Trond Myklebust]

In anticipation of the day when we have per-filesystem sessions, and also
in order to allow the session to change in the event of a filesystem
migration event.

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---
 fs/nfs/nfs4_fs.h  |   14 ++++++++++++--
 fs/nfs/nfs4proc.c |   49 +++++++++++++++++++++++++++----------------------
 fs/nfs/read.c     |    2 +-
 fs/nfs/unlink.c   |    2 +-
 fs/nfs/write.c    |    4 ++--
 5 files changed, 43 insertions(+), 28 deletions(-)

diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index c538c61..9c0aa81 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -223,7 +223,12 @@ extern int nfs4_proc_fs_locations(struct inode *dir, const struct qstr *name,
 extern struct nfs4_state_recovery_ops *nfs4_reboot_recovery_ops[];
 extern struct nfs4_state_recovery_ops *nfs4_nograce_recovery_ops[];
 #if defined(CONFIG_NFS_V4_1)
-extern int nfs4_setup_sequence(struct nfs_client *clp,
+static inline struct nfs4_session *nfs4_get_session(const struct nfs_server *server)
+{
+	return server->nfs_client->cl_session;
+}
+
+extern int nfs4_setup_sequence(const struct nfs_server *server,
 		struct nfs4_sequence_args *args, struct nfs4_sequence_res *res,
 		int cache_reply, struct rpc_task *task);
 extern void nfs4_destroy_session(struct nfs4_session *session);
@@ -234,7 +239,12 @@ extern int nfs4_init_session(struct nfs_server *server);
 extern int nfs4_proc_get_lease_time(struct nfs_client *clp,
 		struct nfs_fsinfo *fsinfo);
 #else /* CONFIG_NFS_v4_1 */
-static inline int nfs4_setup_sequence(struct nfs_client *clp,
+static inline struct nfs4_session *nfs4_get_session(const struct nfs_server *server)
+{
+	return NULL;
+}
+
+static inline int nfs4_setup_sequence(const struct nfs_server *server,
 		struct nfs4_sequence_args *args, struct nfs4_sequence_res *res,
 		int cache_reply, struct rpc_task *task)
 {
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index ae2651e..94377a5 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -531,20 +531,25 @@ static int nfs41_setup_sequence(struct nfs4_session *session,
 	return 0;
 }
 
-int nfs4_setup_sequence(struct nfs_client *clp,
+int nfs4_setup_sequence(const struct nfs_server *server,
 			struct nfs4_sequence_args *args,
 			struct nfs4_sequence_res *res,
 			int cache_reply,
 			struct rpc_task *task)
 {
+	struct nfs4_session *session = nfs4_get_session(server);
 	int ret = 0;
 
+	if (session == NULL) {
+		args->sa_session = NULL;
+		res->sr_session = NULL;
+		goto out;
+	}
+
 	dprintk("--> %s clp %p session %p sr_slotid %d\n",
-		__func__, clp, clp->cl_session, res->sr_slotid);
+		__func__, session->clp, session, res->sr_slotid);
 
-	if (!nfs4_has_session(clp))
-		goto out;
-	ret = nfs41_setup_sequence(clp->cl_session, args, res, cache_reply,
+	ret = nfs41_setup_sequence(session, args, res, cache_reply,
 				   task);
 out:
 	dprintk("<-- %s status=%d\n", __func__, ret);
@@ -552,7 +557,7 @@ out:
 }
 
 struct nfs41_call_sync_data {
-	struct nfs_client *clp;
+	const struct nfs_server *seq_server;
 	struct nfs4_sequence_args *seq_args;
 	struct nfs4_sequence_res *seq_res;
 	int cache_reply;
@@ -562,9 +567,9 @@ static void nfs41_call_sync_prepare(struct rpc_task *task, void *calldata)
 {
 	struct nfs41_call_sync_data *data = calldata;
 
-	dprintk("--> %s data->clp->cl_session %p\n", __func__,
-		data->clp->cl_session);
-	if (nfs4_setup_sequence(data->clp, data->seq_args,
+	dprintk("--> %s data->seq_server %p\n", __func__, data->seq_server);
+
+	if (nfs4_setup_sequence(data->seq_server, data->seq_args,
 				data->seq_res, data->cache_reply, task))
 		return;
 	rpc_call_start(task);
@@ -593,8 +598,7 @@ struct rpc_call_ops nfs41_call_priv_sync_ops = {
 	.rpc_call_done = nfs41_call_sync_done,
 };
 
-static int nfs4_call_sync_sequence(struct nfs_client *clp,
-				   struct rpc_clnt *clnt,
+static int nfs4_call_sync_sequence(struct nfs_server *server,
 				   struct rpc_message *msg,
 				   struct nfs4_sequence_args *args,
 				   struct nfs4_sequence_res *res,
@@ -604,13 +608,13 @@ static int nfs4_call_sync_sequence(struct nfs_client *clp,
 	int ret;
 	struct rpc_task *task;
 	struct nfs41_call_sync_data data = {
-		.clp = clp,
+		.seq_server = server,
 		.seq_args = args,
 		.seq_res = res,
 		.cache_reply = cache_reply,
 	};
 	struct rpc_task_setup task_setup = {
-		.rpc_client = clnt,
+		.rpc_client = server->client,
 		.rpc_message = msg,
 		.callback_ops = &nfs41_call_sync_ops,
 		.callback_data = &data
@@ -635,8 +639,7 @@ int _nfs4_call_sync_session(struct nfs_server *server,
 			    struct nfs4_sequence_res *res,
 			    int cache_reply)
 {
-	return nfs4_call_sync_sequence(server->nfs_client, server->client,
-				       msg, args, res, cache_reply, 0);
+	return nfs4_call_sync_sequence(server, msg, args, res, cache_reply, 0);
 }
 
 #endif /* CONFIG_NFS_V4_1 */
@@ -1355,7 +1358,7 @@ static void nfs4_open_prepare(struct rpc_task *task, void *calldata)
 		nfs_copy_fh(&data->o_res.fh, data->o_arg.fh);
 	}
 	data->timestamp = jiffies;
-	if (nfs4_setup_sequence(data->o_arg.server->nfs_client,
+	if (nfs4_setup_sequence(data->o_arg.server,
 				&data->o_arg.seq_args,
 				&data->o_res.seq_res, 1, task))
 		return;
@@ -1896,7 +1899,7 @@ static void nfs4_close_prepare(struct rpc_task *task, void *data)
 
 	nfs_fattr_init(calldata->res.fattr);
 	calldata->timestamp = jiffies;
-	if (nfs4_setup_sequence((NFS_SERVER(calldata->inode))->nfs_client,
+	if (nfs4_setup_sequence(NFS_SERVER(calldata->inode),
 				&calldata->arg.seq_args, &calldata->res.seq_res,
 				1, task))
 		return;
@@ -3660,7 +3663,7 @@ static void nfs4_delegreturn_prepare(struct rpc_task *task, void *data)
 
 	d_data = (struct nfs4_delegreturndata *)data;
 
-	if (nfs4_setup_sequence(d_data->res.server->nfs_client,
+	if (nfs4_setup_sequence(d_data->res.server,
 				&d_data->args.seq_args,
 				&d_data->res.seq_res, 1, task))
 		return;
@@ -3915,7 +3918,7 @@ static void nfs4_locku_prepare(struct rpc_task *task, void *data)
 		return;
 	}
 	calldata->timestamp = jiffies;
-	if (nfs4_setup_sequence(calldata->server->nfs_client,
+	if (nfs4_setup_sequence(calldata->server,
 				&calldata->arg.seq_args,
 				&calldata->res.seq_res, 1, task))
 		return;
@@ -4070,7 +4073,8 @@ static void nfs4_lock_prepare(struct rpc_task *task, void *calldata)
 	} else
 		data->arg.new_lock_owner = 0;
 	data->timestamp = jiffies;
-	if (nfs4_setup_sequence(data->server->nfs_client, &data->arg.seq_args,
+	if (nfs4_setup_sequence(data->server,
+				&data->arg.seq_args,
 				&data->res.seq_res, 1, task))
 		return;
 	rpc_call_start(task);
@@ -5110,7 +5114,7 @@ static void nfs41_sequence_prepare(struct rpc_task *task, void *data)
 	args = task->tk_msg.rpc_argp;
 	res = task->tk_msg.rpc_resp;
 
-	if (nfs4_setup_sequence(clp, args, res, 0, task))
+	if (nfs41_setup_sequence(clp->cl_session, args, res, 0, task))
 		return;
 	rpc_call_start(task);
 }
@@ -5194,7 +5198,8 @@ static void nfs4_reclaim_complete_prepare(struct rpc_task *task, void *data)
 	struct nfs4_reclaim_complete_data *calldata = data;
 
 	rpc_task_set_priority(task, RPC_PRIORITY_PRIVILEGED);
-	if (nfs4_setup_sequence(calldata->clp, &calldata->arg.seq_args,
+	if (nfs41_setup_sequence(calldata->clp->cl_session,
+				&calldata->arg.seq_args,
 				&calldata->res.seq_res, 0, task))
 		return;
 
diff --git a/fs/nfs/read.c b/fs/nfs/read.c
index 6e2b06e..5a33a92 100644
--- a/fs/nfs/read.c
+++ b/fs/nfs/read.c
@@ -410,7 +410,7 @@ void nfs_read_prepare(struct rpc_task *task, void *calldata)
 {
 	struct nfs_read_data *data = calldata;
 
-	if (nfs4_setup_sequence(NFS_SERVER(data->inode)->nfs_client,
+	if (nfs4_setup_sequence(NFS_SERVER(data->inode),
 				&data->args.seq_args, &data->res.seq_res,
 				0, task))
 		return;
diff --git a/fs/nfs/unlink.c b/fs/nfs/unlink.c
index a2242af..2f84ada 100644
--- a/fs/nfs/unlink.c
+++ b/fs/nfs/unlink.c
@@ -110,7 +110,7 @@ void nfs_unlink_prepare(struct rpc_task *task, void *calldata)
 	struct nfs_unlinkdata *data = calldata;
 	struct nfs_server *server = NFS_SERVER(data->dir);
 
-	if (nfs4_setup_sequence(server->nfs_client, &data->args.seq_args,
+	if (nfs4_setup_sequence(server, &data->args.seq_args,
 				&data->res.seq_res, 1, task))
 		return;
 	rpc_call_start(task);
diff --git a/fs/nfs/write.c b/fs/nfs/write.c
index 91679e2..03df228 100644
--- a/fs/nfs/write.c
+++ b/fs/nfs/write.c
@@ -1036,9 +1036,9 @@ out:
 void nfs_write_prepare(struct rpc_task *task, void *calldata)
 {
 	struct nfs_write_data *data = calldata;
-	struct nfs_client *clp = (NFS_SERVER(data->inode))->nfs_client;
 
-	if (nfs4_setup_sequence(clp, &data->args.seq_args,
+	if (nfs4_setup_sequence(NFS_SERVER(data->inode),
+				&data->args.seq_args,
 				&data->res.seq_res, 1, task))
 		return;
 	rpc_call_start(task);
-- 
1.7.0.1

[PATCH 07/15] NFSv41: Further cleanup for nfs4_sequence_done

[Trond Myklebust]

Instead of testing if the nfs_client has a session, we should be testing if
the struct nfs4_sequence_res was set up with one.

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---
 fs/nfs/nfs4proc.c |   21 ++++++++++++---------
 1 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 94377a5..60ea066 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -430,6 +430,13 @@ out:
 	nfs41_sequence_free_slot(res);
 }
 
+static void nfs4_sequence_done(const struct nfs_server *server,
+			       struct nfs4_sequence_res *res, int rpc_status)
+{
+	if (res->sr_session != NULL)
+		nfs41_sequence_done(res);
+}
+
 /*
  * nfs4_find_slot - efficiently look for a free slot
  *
@@ -642,6 +649,11 @@ int _nfs4_call_sync_session(struct nfs_server *server,
 	return nfs4_call_sync_sequence(server, msg, args, res, cache_reply, 0);
 }
 
+#else
+static void nfs4_sequence_done(const struct nfs_server *server,
+			       struct nfs4_sequence_res *res, int rpc_status)
+{
+}
 #endif /* CONFIG_NFS_V4_1 */
 
 int _nfs4_call_sync(struct nfs_server *server,
@@ -658,15 +670,6 @@ int _nfs4_call_sync(struct nfs_server *server,
 	(server)->nfs_client->cl_call_sync((server), (msg), &(args)->seq_args, \
 			&(res)->seq_res, (cache_reply))
 
-static void nfs4_sequence_done(const struct nfs_server *server,
-			       struct nfs4_sequence_res *res, int rpc_status)
-{
-#ifdef CONFIG_NFS_V4_1
-	if (nfs4_has_session(server->nfs_client))
-		nfs41_sequence_done(res);
-#endif /* CONFIG_NFS_V4_1 */
-}
-
 static void update_changeattr(struct inode *dir, struct nfs4_change_info *cinfo)
 {
 	struct nfs_inode *nfsi = NFS_I(dir);
-- 
1.7.0.1

[PATCH 08/15] NFSv41: Don't store session state in the nfs_client->cl_state

[Trond Myklebust]

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---
 fs/nfs/nfs4_fs.h   |    5 ++++-
 fs/nfs/nfs4proc.c  |    4 ++--
 fs/nfs/nfs4state.c |    6 ++++--
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index 9c0aa81..bb1e955 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -45,10 +45,13 @@ enum nfs4_client_state {
 	NFS4CLNT_RECLAIM_NOGRACE,
 	NFS4CLNT_DELEGRETURN,
 	NFS4CLNT_SESSION_RESET,
-	NFS4CLNT_SESSION_DRAINING,
 	NFS4CLNT_RECALL_SLOT,
 };
 
+enum nfs4_session_state {
+	NFS4_SESSION_DRAINING,
+};
+
 /*
  * struct rpc_sequence ensures that RPC calls are sent in the exact
  * order that they appear on the list.
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 60ea066..7e0483c 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -356,7 +356,7 @@ static void nfs41_check_drain_session_complete(struct nfs4_session *ses)
 {
 	struct rpc_task *task;
 
-	if (!test_bit(NFS4CLNT_SESSION_DRAINING, &ses->clp->cl_state)) {
+	if (!test_bit(NFS4_SESSION_DRAINING, &ses->session_state)) {
 		task = rpc_wake_up_next(&ses->fc_slot_table.slot_tbl_waitq);
 		if (task)
 			rpc_task_set_priority(task, RPC_PRIORITY_PRIVILEGED);
@@ -489,7 +489,7 @@ static int nfs41_setup_sequence(struct nfs4_session *session,
 	tbl = &session->fc_slot_table;
 
 	spin_lock(&tbl->slot_tbl_lock);
-	if (test_bit(NFS4CLNT_SESSION_DRAINING, &session->clp->cl_state) &&
+	if (test_bit(NFS4_SESSION_DRAINING, &session->session_state) &&
 	    !rpc_task_has_priority(task, RPC_PRIORITY_PRIVILEGED)) {
 		/*
 		 * The state manager will wait until the slot table is empty.
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 34acf59..4538c56 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -145,7 +145,9 @@ static void nfs4_end_drain_session(struct nfs_client *clp)
 	struct nfs4_session *ses = clp->cl_session;
 	int max_slots;
 
-	if (test_and_clear_bit(NFS4CLNT_SESSION_DRAINING, &clp->cl_state)) {
+	if (ses == NULL)
+		return;
+	if (test_and_clear_bit(NFS4_SESSION_DRAINING, &ses->session_state)) {
 		spin_lock(&ses->fc_slot_table.slot_tbl_lock);
 		max_slots = ses->fc_slot_table.max_slots;
 		while (max_slots--) {
@@ -167,7 +169,7 @@ static int nfs4_begin_drain_session(struct nfs_client *clp)
 	struct nfs4_slot_table *tbl = &ses->fc_slot_table;
 
 	spin_lock(&tbl->slot_tbl_lock);
-	set_bit(NFS4CLNT_SESSION_DRAINING, &clp->cl_state);
+	set_bit(NFS4_SESSION_DRAINING, &ses->session_state);
 	if (tbl->highest_used_slotid != -1) {
 		INIT_COMPLETION(ses->complete);
 		spin_unlock(&tbl->slot_tbl_lock);
-- 
1.7.0.1

[PATCH 09/15] NFSv41: Clean up the NFSv4.1 minor version specific operations

[Trond Myklebust]

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---
 fs/nfs/client.c           |   14 ++++++--------
 fs/nfs/nfs4_fs.h          |   11 +++++++++++
 fs/nfs/nfs4proc.c         |   21 ++++++++++++++++++++-
 include/linux/nfs_fs_sb.h |    7 ++-----
 4 files changed, 39 insertions(+), 14 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 7ec9b34..123fcbd 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -150,6 +150,7 @@ static struct nfs_client *nfs_alloc_client(const struct nfs_client_initdata *cl_
 	clp->cl_boot_time = CURRENT_TIME;
 	clp->cl_state = 1 << NFS4CLNT_LEASE_EXPIRED;
 	clp->cl_minorversion = cl_init->minorversion;
+	clp->cl_mvops = nfs_v4_minor_ops[cl_init->minorversion];
 #endif
 	cred = rpc_lookup_machine_cred();
 	if (!IS_ERR(cred))
@@ -178,7 +179,7 @@ static void nfs4_clear_client_minor_version(struct nfs_client *clp)
 		clp->cl_session = NULL;
 	}
 
-	clp->cl_call_sync = _nfs4_call_sync;
+	clp->cl_mvops = nfs_v4_minor_ops[0];
 #endif /* CONFIG_NFS_V4_1 */
 }
 
@@ -188,7 +189,7 @@ static void nfs4_clear_client_minor_version(struct nfs_client *clp)
 static void nfs4_destroy_callback(struct nfs_client *clp)
 {
 	if (__test_and_clear_bit(NFS_CS_CALLBACK, &clp->cl_res_state))
-		nfs_callback_down(clp->cl_minorversion);
+		nfs_callback_down(clp->cl_mvops->minor_version);
 }
 
 static void nfs4_shutdown_client(struct nfs_client *clp)
@@ -1126,7 +1127,7 @@ static int nfs4_init_callback(struct nfs_client *clp)
 				return error;
 		}
 
-		error = nfs_callback_up(clp->cl_minorversion,
+		error = nfs_callback_up(clp->cl_mvops->minor_version,
 					clp->cl_rpcclient->cl_xprt);
 		if (error < 0) {
 			dprintk("%s: failed to start callback. Error = %d\n",
@@ -1143,10 +1144,8 @@ static int nfs4_init_callback(struct nfs_client *clp)
  */
 static int nfs4_init_client_minor_version(struct nfs_client *clp)
 {
-	clp->cl_call_sync = _nfs4_call_sync;
-
 #if defined(CONFIG_NFS_V4_1)
-	if (clp->cl_minorversion) {
+	if (clp->cl_mvops->minor_version) {
 		struct nfs4_session *session = NULL;
 		/*
 		 * Create the session and mark it expired.
@@ -1158,7 +1157,6 @@ static int nfs4_init_client_minor_version(struct nfs_client *clp)
 			return -ENOMEM;
 
 		clp->cl_session = session;
-		clp->cl_call_sync = _nfs4_call_sync_session;
 	}
 #endif /* CONFIG_NFS_V4_1 */
 
@@ -1448,7 +1446,7 @@ struct nfs_server *nfs4_create_referral_server(struct nfs_clone_mount *data,
 				data->authflavor,
 				parent_server->client->cl_xprt->prot,
 				parent_server->client->cl_timeout,
-				parent_client->cl_minorversion);
+				parent_client->cl_mvops->minor_version);
 	if (error < 0)
 		goto error;
 
diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index bb1e955..5b01705 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -52,6 +52,16 @@ enum nfs4_session_state {
 	NFS4_SESSION_DRAINING,
 };
 
+struct nfs4_minor_version_ops {
+	u32	minor_version;
+
+	int	(*call_sync)(struct nfs_server *server,
+			struct rpc_message *msg,
+			struct nfs4_sequence_args *args,
+			struct nfs4_sequence_res *res,
+			int cache_reply);
+};
+
 /*
  * struct rpc_sequence ensures that RPC calls are sent in the exact
  * order that they appear on the list.
@@ -260,6 +270,7 @@ static inline int nfs4_init_session(struct nfs_server *server)
 }
 #endif /* CONFIG_NFS_V4_1 */
 
+extern const struct nfs4_minor_version_ops *nfs_v4_minor_ops[];
 extern struct nfs4_state_maintenance_ops *nfs4_state_renewal_ops[];
 
 extern const u32 nfs4_fattr_bitmap[2];
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 7e0483c..8530782 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -667,7 +667,7 @@ int _nfs4_call_sync(struct nfs_server *server,
 }
 
 #define nfs4_call_sync(server, msg, args, res, cache_reply) \
-	(server)->nfs_client->cl_call_sync((server), (msg), &(args)->seq_args, \
+	(server)->nfs_client->cl_mvops->call_sync((server), (msg), &(args)->seq_args, \
 			&(res)->seq_res, (cache_reply))
 
 static void update_changeattr(struct inode *dir, struct nfs4_change_info *cinfo)
@@ -5352,6 +5352,18 @@ struct nfs4_state_maintenance_ops nfs41_state_renewal_ops = {
 };
 #endif
 
+static const struct nfs4_minor_version_ops nfs_v4_0_minor_ops = {
+	.minor_version = 0,
+	.call_sync = _nfs4_call_sync,
+};
+
+#if defined(CONFIG_NFS_V4_1)
+static const struct nfs4_minor_version_ops nfs_v4_1_minor_ops = {
+	.minor_version = 1,
+	.call_sync = _nfs4_call_sync_session,
+};
+#endif
+
 /*
  * Per minor version reboot and network partition recovery ops
  */
@@ -5377,6 +5389,13 @@ struct nfs4_state_maintenance_ops *nfs4_state_renewal_ops[] = {
 #endif
 };
 
+const struct nfs4_minor_version_ops *nfs_v4_minor_ops[] = {
+	[0] = &nfs_v4_0_minor_ops,
+#if defined(CONFIG_NFS_V4_1)
+	[1] = &nfs_v4_1_minor_ops,
+#endif
+};
+
 static const struct inode_operations nfs4_file_inode_operations = {
 	.permission	= nfs_permission,
 	.getattr	= nfs_getattr,
diff --git a/include/linux/nfs_fs_sb.h b/include/linux/nfs_fs_sb.h
index d6e10a4..c82ee7c 100644
--- a/include/linux/nfs_fs_sb.h
+++ b/include/linux/nfs_fs_sb.h
@@ -15,6 +15,7 @@ struct nlm_host;
 struct nfs4_sequence_args;
 struct nfs4_sequence_res;
 struct nfs_server;
+struct nfs4_minor_version_ops;
 
 /*
  * The nfs_client identifies our client state to the server.
@@ -70,11 +71,7 @@ struct nfs_client {
 	 */
 	char			cl_ipaddr[48];
 	unsigned char		cl_id_uniquifier;
-	int		     (* cl_call_sync)(struct nfs_server *server,
-					      struct rpc_message *msg,
-					      struct nfs4_sequence_args *args,
-					      struct nfs4_sequence_res *res,
-					      int cache_reply);
+	const struct nfs4_minor_version_ops *cl_mvops;
 #endif /* CONFIG_NFS_V4 */
 
 #ifdef CONFIG_NFS_V4_1
-- 
1.7.0.1

[PATCH 10/15] NFSv41: Convert the various reboot recovery ops etc to minor version ops

[Trond Myklebust]

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---
 fs/nfs/nfs4_fs.h    |    6 +++---
 fs/nfs/nfs4proc.c   |   31 ++++++-------------------------
 fs/nfs/nfs4renewd.c |    4 ++--
 fs/nfs/nfs4state.c  |   15 +++++++--------
 4 files changed, 18 insertions(+), 38 deletions(-)

diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index 5b01705..1ff0ea2 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -60,6 +60,9 @@ struct nfs4_minor_version_ops {
 			struct nfs4_sequence_args *args,
 			struct nfs4_sequence_res *res,
 			int cache_reply);
+	const struct nfs4_state_recovery_ops *reboot_recovery_ops;
+	const struct nfs4_state_recovery_ops *nograce_recovery_ops;
+	const struct nfs4_state_maintenance_ops *state_renewal_ops;
 };
 
 /*
@@ -233,8 +236,6 @@ extern int nfs4_server_capabilities(struct nfs_server *server, struct nfs_fh *fh
 extern int nfs4_proc_fs_locations(struct inode *dir, const struct qstr *name,
 		struct nfs4_fs_locations *fs_locations, struct page *page);
 
-extern struct nfs4_state_recovery_ops *nfs4_reboot_recovery_ops[];
-extern struct nfs4_state_recovery_ops *nfs4_nograce_recovery_ops[];
 #if defined(CONFIG_NFS_V4_1)
 static inline struct nfs4_session *nfs4_get_session(const struct nfs_server *server)
 {
@@ -271,7 +272,6 @@ static inline int nfs4_init_session(struct nfs_server *server)
 #endif /* CONFIG_NFS_V4_1 */
 
 extern const struct nfs4_minor_version_ops *nfs_v4_minor_ops[];
-extern struct nfs4_state_maintenance_ops *nfs4_state_renewal_ops[];
 
 extern const u32 nfs4_fattr_bitmap[2];
 extern const u32 nfs4_statfs_bitmap[2];
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 8530782..e4ec092 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -5355,40 +5355,21 @@ struct nfs4_state_maintenance_ops nfs41_state_renewal_ops = {
 static const struct nfs4_minor_version_ops nfs_v4_0_minor_ops = {
 	.minor_version = 0,
 	.call_sync = _nfs4_call_sync,
+	.reboot_recovery_ops = &nfs40_reboot_recovery_ops,
+	.nograce_recovery_ops = &nfs40_nograce_recovery_ops,
+	.state_renewal_ops = &nfs40_state_renewal_ops,
 };
 
 #if defined(CONFIG_NFS_V4_1)
 static const struct nfs4_minor_version_ops nfs_v4_1_minor_ops = {
 	.minor_version = 1,
 	.call_sync = _nfs4_call_sync_session,
+	.reboot_recovery_ops = &nfs41_reboot_recovery_ops,
+	.nograce_recovery_ops = &nfs41_nograce_recovery_ops,
+	.state_renewal_ops = &nfs41_state_renewal_ops,
 };
 #endif
 
-/*
- * Per minor version reboot and network partition recovery ops
- */
-
-struct nfs4_state_recovery_ops *nfs4_reboot_recovery_ops[] = {
-	&nfs40_reboot_recovery_ops,
-#if defined(CONFIG_NFS_V4_1)
-	&nfs41_reboot_recovery_ops,
-#endif
-};
-
-struct nfs4_state_recovery_ops *nfs4_nograce_recovery_ops[] = {
-	&nfs40_nograce_recovery_ops,
-#if defined(CONFIG_NFS_V4_1)
-	&nfs41_nograce_recovery_ops,
-#endif
-};
-
-struct nfs4_state_maintenance_ops *nfs4_state_renewal_ops[] = {
-	&nfs40_state_renewal_ops,
-#if defined(CONFIG_NFS_V4_1)
-	&nfs41_state_renewal_ops,
-#endif
-};
-
 const struct nfs4_minor_version_ops *nfs_v4_minor_ops[] = {
 	[0] = &nfs_v4_0_minor_ops,
 #if defined(CONFIG_NFS_V4_1)
diff --git a/fs/nfs/nfs4renewd.c b/fs/nfs/nfs4renewd.c
index d87f103..72b6c58 100644
--- a/fs/nfs/nfs4renewd.c
+++ b/fs/nfs/nfs4renewd.c
@@ -54,14 +54,14 @@
 void
 nfs4_renew_state(struct work_struct *work)
 {
-	struct nfs4_state_maintenance_ops *ops;
+	const struct nfs4_state_maintenance_ops *ops;
 	struct nfs_client *clp =
 		container_of(work, struct nfs_client, cl_renewd.work);
 	struct rpc_cred *cred;
 	long lease;
 	unsigned long last, now;
 
-	ops = nfs4_state_renewal_ops[clp->cl_minorversion];
+	ops = clp->cl_mvops->state_renewal_ops;
 	dprintk("%s: start\n", __func__);
 	/* Are there any active superblocks? */
 	if (list_empty(&clp->cl_superblocks))
diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
index 4538c56..c3c6b8a 100644
--- a/fs/nfs/nfs4state.c
+++ b/fs/nfs/nfs4state.c
@@ -1122,8 +1122,7 @@ static void nfs4_state_end_reclaim_reboot(struct nfs_client *clp)
 	if (!test_and_clear_bit(NFS4CLNT_RECLAIM_REBOOT, &clp->cl_state))
 		return;
 
-	nfs4_reclaim_complete(clp,
-		nfs4_reboot_recovery_ops[clp->cl_minorversion]);
+	nfs4_reclaim_complete(clp, clp->cl_mvops->reboot_recovery_ops);
 
 	for (pos = rb_first(&clp->cl_state_owners); pos != NULL; pos = rb_next(pos)) {
 		sp = rb_entry(pos, struct nfs4_state_owner, so_client_node);
@@ -1213,8 +1212,8 @@ restart:
 static int nfs4_check_lease(struct nfs_client *clp)
 {
 	struct rpc_cred *cred;
-	struct nfs4_state_maintenance_ops *ops =
-		nfs4_state_renewal_ops[clp->cl_minorversion];
+	const struct nfs4_state_maintenance_ops *ops =
+		clp->cl_mvops->state_renewal_ops;
 	int status = -NFS4ERR_EXPIRED;
 
 	/* Is the client already known to have an expired lease? */
@@ -1237,8 +1236,8 @@ out:
 static int nfs4_reclaim_lease(struct nfs_client *clp)
 {
 	struct rpc_cred *cred;
-	struct nfs4_state_recovery_ops *ops =
-		nfs4_reboot_recovery_ops[clp->cl_minorversion];
+	const struct nfs4_state_recovery_ops *ops =
+		clp->cl_mvops->reboot_recovery_ops;
 	int status = -ENOENT;
 
 	cred = ops->get_clid_cred(clp);
@@ -1446,7 +1445,7 @@ static void nfs4_state_manager(struct nfs_client *clp)
 		/* First recover reboot state... */
 		if (test_bit(NFS4CLNT_RECLAIM_REBOOT, &clp->cl_state)) {
 			status = nfs4_do_reclaim(clp,
-				nfs4_reboot_recovery_ops[clp->cl_minorversion]);
+				clp->cl_mvops->reboot_recovery_ops);
 			if (test_bit(NFS4CLNT_LEASE_EXPIRED, &clp->cl_state) ||
 			    test_bit(NFS4CLNT_SESSION_RESET, &clp->cl_state))
 				continue;
@@ -1460,7 +1459,7 @@ static void nfs4_state_manager(struct nfs_client *clp)
 		/* Now recover expired state... */
 		if (test_and_clear_bit(NFS4CLNT_RECLAIM_NOGRACE, &clp->cl_state)) {
 			status = nfs4_do_reclaim(clp,
-				nfs4_nograce_recovery_ops[clp->cl_minorversion]);
+				clp->cl_mvops->nograce_recovery_ops);
 			if (test_bit(NFS4CLNT_LEASE_EXPIRED, &clp->cl_state) ||
 			    test_bit(NFS4CLNT_SESSION_RESET, &clp->cl_state) ||
 			    test_bit(NFS4CLNT_RECLAIM_REBOOT, &clp->cl_state))
-- 
1.7.0.1

[PATCH 11/15] NFSv41: Fix nfs_async_inode_return_delegation() ugliness

[Trond Myklebust]

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---
 fs/nfs/callback_proc.c |   13 +------------
 fs/nfs/delegation.c    |    6 ++----
 fs/nfs/delegation.h    |    4 +---
 fs/nfs/nfs4_fs.h       |    2 ++
 fs/nfs/nfs4proc.c      |    2 ++
 5 files changed, 8 insertions(+), 19 deletions(-)

diff --git a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c
index a08770a..7445dd0 100644
--- a/fs/nfs/callback_proc.c
+++ b/fs/nfs/callback_proc.c
@@ -62,16 +62,6 @@ out:
 	return res->status;
 }
 
-static int (*nfs_validate_delegation_stateid(struct nfs_client *clp))(struct nfs_delegation *, const nfs4_stateid *)
-{
-#if defined(CONFIG_NFS_V4_1)
-	if (clp->cl_minorversion > 0)
-		return nfs41_validate_delegation_stateid;
-#endif
-	return nfs4_validate_delegation_stateid;
-}
-
-
 __be32 nfs4_callback_recall(struct cb_recallargs *args, void *dummy)
 {
 	struct nfs_client *clp;
@@ -92,8 +82,7 @@ __be32 nfs4_callback_recall(struct cb_recallargs *args, void *dummy)
 		inode = nfs_delegation_find_inode(clp, &args->fh);
 		if (inode != NULL) {
 			/* Set up a helper thread to actually return the delegation */
-			switch (nfs_async_inode_return_delegation(inode, &args->stateid,
-								  nfs_validate_delegation_stateid(clp))) {
+			switch (nfs_async_inode_return_delegation(inode, &args->stateid)) {
 				case 0:
 					res = 0;
 					break;
diff --git a/fs/nfs/delegation.c b/fs/nfs/delegation.c
index 3016345..f34f4ac 100644
--- a/fs/nfs/delegation.c
+++ b/fs/nfs/delegation.c
@@ -471,9 +471,7 @@ void nfs_expire_unreferenced_delegations(struct nfs_client *clp)
 /*
  * Asynchronous delegation recall!
  */
-int nfs_async_inode_return_delegation(struct inode *inode, const nfs4_stateid *stateid,
-				      int (*validate_stateid)(struct nfs_delegation *delegation,
-							      const nfs4_stateid *stateid))
+int nfs_async_inode_return_delegation(struct inode *inode, const nfs4_stateid *stateid)
 {
 	struct nfs_client *clp = NFS_SERVER(inode)->nfs_client;
 	struct nfs_delegation *delegation;
@@ -481,7 +479,7 @@ int nfs_async_inode_return_delegation(struct inode *inode, const nfs4_stateid *s
 	rcu_read_lock();
 	delegation = rcu_dereference(NFS_I(inode)->delegation);
 
-	if (!validate_stateid(delegation, stateid)) {
+	if (!clp->cl_mvops->validate_stateid(delegation, stateid)) {
 		rcu_read_unlock();
 		return -ENOENT;
 	}
diff --git a/fs/nfs/delegation.h b/fs/nfs/delegation.h
index 69e7b81..2026304 100644
--- a/fs/nfs/delegation.h
+++ b/fs/nfs/delegation.h
@@ -34,9 +34,7 @@ enum {
 int nfs_inode_set_delegation(struct inode *inode, struct rpc_cred *cred, struct nfs_openres *res);
 void nfs_inode_reclaim_delegation(struct inode *inode, struct rpc_cred *cred, struct nfs_openres *res);
 int nfs_inode_return_delegation(struct inode *inode);
-int nfs_async_inode_return_delegation(struct inode *inode, const nfs4_stateid *stateid,
-				      int (*validate_stateid)(struct nfs_delegation *delegation,
-							      const nfs4_stateid *stateid));
+int nfs_async_inode_return_delegation(struct inode *inode, const nfs4_stateid *stateid);
 void nfs_inode_return_delegation_noreclaim(struct inode *inode);
 
 struct inode *nfs_delegation_find_inode(struct nfs_client *clp, const struct nfs_fh *fhandle);
diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index 1ff0ea2..04eff16 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -60,6 +60,8 @@ struct nfs4_minor_version_ops {
 			struct nfs4_sequence_args *args,
 			struct nfs4_sequence_res *res,
 			int cache_reply);
+	int	(*validate_stateid)(struct nfs_delegation *,
+			const nfs4_stateid *);
 	const struct nfs4_state_recovery_ops *reboot_recovery_ops;
 	const struct nfs4_state_recovery_ops *nograce_recovery_ops;
 	const struct nfs4_state_maintenance_ops *state_renewal_ops;
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index e4ec092..ef613ea 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -5355,6 +5355,7 @@ struct nfs4_state_maintenance_ops nfs41_state_renewal_ops = {
 static const struct nfs4_minor_version_ops nfs_v4_0_minor_ops = {
 	.minor_version = 0,
 	.call_sync = _nfs4_call_sync,
+	.validate_stateid = nfs4_validate_delegation_stateid,
 	.reboot_recovery_ops = &nfs40_reboot_recovery_ops,
 	.nograce_recovery_ops = &nfs40_nograce_recovery_ops,
 	.state_renewal_ops = &nfs40_state_renewal_ops,
@@ -5364,6 +5365,7 @@ static const struct nfs4_minor_version_ops nfs_v4_0_minor_ops = {
 static const struct nfs4_minor_version_ops nfs_v4_1_minor_ops = {
 	.minor_version = 1,
 	.call_sync = _nfs4_call_sync_session,
+	.validate_stateid = nfs41_validate_delegation_stateid,
 	.reboot_recovery_ops = &nfs41_reboot_recovery_ops,
 	.nograce_recovery_ops = &nfs41_nograce_recovery_ops,
 	.state_renewal_ops = &nfs41_state_renewal_ops,
-- 
1.7.0.1

[PATCH 12/15] NFSv41: Deprecate nfs_client->cl_minorversion

[Trond Myklebust]

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---
 fs/nfs/nfs4xdr.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index 6bdef28..756a59e 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -1172,7 +1172,7 @@ static inline void encode_createmode(struct xdr_stream *xdr, const struct nfs_op
 		break;
 	default:
 		clp = arg->server->nfs_client;
-		if (clp->cl_minorversion > 0) {
+		if (clp->cl_mvops->minor_version > 0) {
 			if (nfs4_has_persistent_session(clp)) {
 				*p = cpu_to_be32(NFS4_CREATE_GUARDED);
 				encode_attrs(xdr, arg->u.attrs, arg->server);
@@ -1704,7 +1704,7 @@ static u32 nfs4_xdr_minorversion(const struct nfs4_sequence_args *args)
 {
 #if defined(CONFIG_NFS_V4_1)
 	if (args->sa_session)
-		return args->sa_session->clp->cl_minorversion;
+		return args->sa_session->clp->cl_mvops->minor_version;
 #endif /* CONFIG_NFS_V4_1 */
 	return 0;
 }
@@ -2395,7 +2395,7 @@ static int nfs4_xdr_enc_exchange_id(struct rpc_rqst *req, uint32_t *p,
 {
 	struct xdr_stream xdr;
 	struct compound_hdr hdr = {
-		.minorversion = args->client->cl_minorversion,
+		.minorversion = args->client->cl_mvops->minor_version,
 	};
 
 	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
@@ -2413,7 +2413,7 @@ static int nfs4_xdr_enc_create_session(struct rpc_rqst *req, uint32_t *p,
 {
 	struct xdr_stream xdr;
 	struct compound_hdr hdr = {
-		.minorversion = args->client->cl_minorversion,
+		.minorversion = args->client->cl_mvops->minor_version,
 	};
 
 	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
@@ -2431,7 +2431,7 @@ static int nfs4_xdr_enc_destroy_session(struct rpc_rqst *req, uint32_t *p,
 {
 	struct xdr_stream xdr;
 	struct compound_hdr hdr = {
-		.minorversion = session->clp->cl_minorversion,
+		.minorversion = session->clp->cl_mvops->minor_version,
 	};
 
 	xdr_init_encode(&xdr, &req->rq_snd_buf, p);
-- 
1.7.0.1

[PATCH 13/15] NFSv41: Clean up exclusive create

[Trond Myklebust]

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---
 fs/nfs/nfs4proc.c       |   17 ++++++-----------
 include/linux/nfs_xdr.h |    6 ++++--
 2 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index ef613ea..e9913de 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -744,19 +744,14 @@ static struct nfs4_opendata *nfs4_opendata_alloc(struct path *path,
 	p->o_arg.server = server;
 	p->o_arg.bitmask = server->attr_bitmask;
 	p->o_arg.claim = NFS4_OPEN_CLAIM_NULL;
-	if (flags & O_EXCL) {
-		if (nfs4_has_persistent_session(server->nfs_client)) {
-			/* GUARDED */
-			p->o_arg.u.attrs = &p->attrs;
-			memcpy(&p->attrs, attrs, sizeof(p->attrs));
-		} else { /* EXCLUSIVE4_1 */
-			u32 *s = (u32 *) p->o_arg.u.verifier.data;
-			s[0] = jiffies;
-			s[1] = current->pid;
-		}
-	} else if (flags & O_CREAT) {
+	if (flags & O_CREAT) {
+		u32 *s;
+
 		p->o_arg.u.attrs = &p->attrs;
 		memcpy(&p->attrs, attrs, sizeof(p->attrs));
+		s = (u32 *) p->o_arg.u.verifier.data;
+		s[0] = jiffies;
+		s[1] = current->pid;
 	}
 	p->c_arg.fh = &p->o_res.fh;
 	p->c_arg.stateid = &p->o_res.stateid;
diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
index 51914d7..a319cb9 100644
--- a/include/linux/nfs_xdr.h
+++ b/include/linux/nfs_xdr.h
@@ -196,8 +196,10 @@ struct nfs_openargs {
 	__u64                   clientid;
 	__u64                   id;
 	union {
-		struct iattr *  attrs;    /* UNCHECKED, GUARDED */
-		nfs4_verifier   verifier; /* EXCLUSIVE */
+		struct {
+			struct iattr *  attrs;    /* UNCHECKED, GUARDED */
+			nfs4_verifier   verifier; /* EXCLUSIVE */
+		};
 		nfs4_stateid	delegation;		/* CLAIM_DELEGATE_CUR */
 		fmode_t		delegation_type;	/* CLAIM_PREVIOUS */
 	} u;
-- 
1.7.0.1

[PATCH 14/15] NFSv41: Cleanup for nfs4_alloc_session.

[Trond Myklebust]

There is no reason to change the nfs_client state every time we allocate a
new session. Move that line into nfs4_init_client_minor_version.

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---
 fs/nfs/client.c   |    7 +++++++
 fs/nfs/nfs4proc.c |    7 -------
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/fs/nfs/client.c b/fs/nfs/client.c
index 123fcbd..544810f 100644
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -1157,6 +1157,13 @@ static int nfs4_init_client_minor_version(struct nfs_client *clp)
 			return -ENOMEM;
 
 		clp->cl_session = session;
+		/*
+		 * The create session reply races with the server back
+		 * channel probe. Mark the client NFS_CS_SESSION_INITING
+		 * so that the client back channel can find the
+		 * nfs_client struct
+		 */
+		clp->cl_cons_state = NFS_CS_SESSION_INITING;
 	}
 #endif /* CONFIG_NFS_V4_1 */
 
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index e9913de..1245939 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -4795,13 +4795,6 @@ struct nfs4_session *nfs4_alloc_session(struct nfs_client *clp)
 	if (!session)
 		return NULL;
 
-	/*
-	 * The create session reply races with the server back
-	 * channel probe. Mark the client NFS_CS_SESSION_INITING
-	 * so that the client back channel can find the
-	 * nfs_client struct
-	 */
-	clp->cl_cons_state = NFS_CS_SESSION_INITING;
 	init_completion(&session->complete);
 
 	tbl = &session->fc_slot_table;
-- 
1.7.0.1

[PATCH 15/15] NFSv4.1: There is no need to init the session more than once...

[Trond Myklebust]

Set up a flag to ensure that is indeed the case.

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---
 fs/nfs/nfs4_fs.h  |    1 +
 fs/nfs/nfs4proc.c |    7 ++++++-
 2 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index 04eff16..a986f13 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -49,6 +49,7 @@ enum nfs4_client_state {
 };
 
 enum nfs4_session_state {
+	NFS4_SESSION_INITING,
 	NFS4_SESSION_DRAINING,
 };
 
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 1245939..04eff86 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -4807,6 +4807,8 @@ struct nfs4_session *nfs4_alloc_session(struct nfs_client *clp)
 	spin_lock_init(&tbl->slot_tbl_lock);
 	rpc_init_wait_queue(&tbl->slot_tbl_waitq, "BackChannel Slot table");
 
+	session->session_state = 1<<NFS4_SESSION_INITING;
+
 	session->clp = clp;
 	return session;
 }
@@ -5023,6 +5025,10 @@ int nfs4_init_session(struct nfs_server *server)
 	if (!nfs4_has_session(clp))
 		return 0;
 
+	session = clp->cl_session;
+	if (!test_and_clear_bit(NFS4_SESSION_INITING, &session->session_state))
+		return 0;
+
 	rsize = server->rsize;
 	if (rsize == 0)
 		rsize = NFS_MAX_FILE_IO_SIZE;
@@ -5030,7 +5036,6 @@ int nfs4_init_session(struct nfs_server *server)
 	if (wsize == 0)
 		wsize = NFS_MAX_FILE_IO_SIZE;
 
-	session = clp->cl_session;
 	session->fc_attrs.max_rqst_sz = wsize + nfs41_maxwrite_overhead;
 	session->fc_attrs.max_resp_sz = rsize + nfs41_maxread_overhead;
 
-- 
1.7.0.1

Re: [PATCH 13/15] NFSv41: Clean up exclusive create

[Benny Halevy]

On Jun. 14, 2010, 17:51 -0400, Trond Myklebust <Trond.Myklebust@netapp.com> wrote:
> Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
> ---
>  fs/nfs/nfs4proc.c       |   17 ++++++-----------
>  include/linux/nfs_xdr.h |    6 ++++--
>  2 files changed, 10 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index ef613ea..e9913de 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -744,19 +744,14 @@ static struct nfs4_opendata *nfs4_opendata_alloc(struct path *path,
>  	p->o_arg.server = server;
>  	p->o_arg.bitmask = server->attr_bitmask;
>  	p->o_arg.claim = NFS4_OPEN_CLAIM_NULL;
> -	if (flags & O_EXCL) {
> -		if (nfs4_has_persistent_session(server->nfs_client)) {
> -			/* GUARDED */
> -			p->o_arg.u.attrs = &p->attrs;
> -			memcpy(&p->attrs, attrs, sizeof(p->attrs));

What about this case?
Aren't we losing the attrs?

Benny

> -		} else { /* EXCLUSIVE4_1 */
> -			u32 *s = (u32 *) p->o_arg.u.verifier.data;
> -			s[0] = jiffies;
> -			s[1] = current->pid;
> -		}
> -	} else if (flags & O_CREAT) {
> +	if (flags & O_CREAT) {
> +		u32 *s;
> +
>  		p->o_arg.u.attrs = &p->attrs;
>  		memcpy(&p->attrs, attrs, sizeof(p->attrs));
> +		s = (u32 *) p->o_arg.u.verifier.data;
> +		s[0] = jiffies;
> +		s[1] = current->pid;
>  	}
>  	p->c_arg.fh = &p->o_res.fh;
>  	p->c_arg.stateid = &p->o_res.stateid;
> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> index 51914d7..a319cb9 100644
> --- a/include/linux/nfs_xdr.h
> +++ b/include/linux/nfs_xdr.h
> @@ -196,8 +196,10 @@ struct nfs_openargs {
>  	__u64                   clientid;
>  	__u64                   id;
>  	union {
> -		struct iattr *  attrs;    /* UNCHECKED, GUARDED */
> -		nfs4_verifier   verifier; /* EXCLUSIVE */
> +		struct {
> +			struct iattr *  attrs;    /* UNCHECKED, GUARDED */
> +			nfs4_verifier   verifier; /* EXCLUSIVE */
> +		};
>  		nfs4_stateid	delegation;		/* CLAIM_DELEGATE_CUR */
>  		fmode_t		delegation_type;	/* CLAIM_PREVIOUS */
>  	} u;

Re: [PATCH 13/15] NFSv41: Clean up exclusive create

[Trond Myklebust]

On Mon, 2010-06-14 at 19:13 -0400, Benny Halevy wrote:
> On Jun. 14, 2010, 17:51 -0400, Trond Myklebust <Trond.Myklebust@netapp.com> wrote:
> > Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
> > ---
> >  fs/nfs/nfs4proc.c       |   17 ++++++-----------
> >  include/linux/nfs_xdr.h |    6 ++++--
> >  2 files changed, 10 insertions(+), 13 deletions(-)
> > 
> > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> > index ef613ea..e9913de 100644
> > --- a/fs/nfs/nfs4proc.c
> > +++ b/fs/nfs/nfs4proc.c
> > @@ -744,19 +744,14 @@ static struct nfs4_opendata *nfs4_opendata_alloc(struct path *path,
> >  	p->o_arg.server = server;
> >  	p->o_arg.bitmask = server->attr_bitmask;
> >  	p->o_arg.claim = NFS4_OPEN_CLAIM_NULL;
> > -	if (flags & O_EXCL) {
> > -		if (nfs4_has_persistent_session(server->nfs_client)) {
> > -			/* GUARDED */
> > -			p->o_arg.u.attrs = &p->attrs;
> > -			memcpy(&p->attrs, attrs, sizeof(p->attrs));
> 
> What about this case?
> Aren't we losing the attrs?

I'm not sure what you mean. Are you asking what will happen if someone
sets O_EXCL without setting O_CREAT? We don't call encode_createmode at
all in that case. Otherwise, see below.

Cheers
  Trond

> 
> > -		} else { /* EXCLUSIVE4_1 */
> > -			u32 *s = (u32 *) p->o_arg.u.verifier.data;
> > -			s[0] = jiffies;
> > -			s[1] = current->pid;
> > -		}
> > -	} else if (flags & O_CREAT) {
> > +	if (flags & O_CREAT) {
> > +		u32 *s;
> > +
> >  		p->o_arg.u.attrs = &p->attrs;
> >  		memcpy(&p->attrs, attrs, sizeof(p->attrs));
> > +		s = (u32 *) p->o_arg.u.verifier.data;
> > +		s[0] = jiffies;
> > +		s[1] = current->pid;
> >  	}
> >  	p->c_arg.fh = &p->o_res.fh;
> >  	p->c_arg.stateid = &p->o_res.stateid;
> > diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> > index 51914d7..a319cb9 100644
> > --- a/include/linux/nfs_xdr.h
> > +++ b/include/linux/nfs_xdr.h
> > @@ -196,8 +196,10 @@ struct nfs_openargs {
> >  	__u64                   clientid;
> >  	__u64                   id;
> >  	union {
> > -		struct iattr *  attrs;    /* UNCHECKED, GUARDED */
> > -		nfs4_verifier   verifier; /* EXCLUSIVE */
> > +		struct {
> > +			struct iattr *  attrs;    /* UNCHECKED, GUARDED */
> > +			nfs4_verifier   verifier; /* EXCLUSIVE */
> > +		};
> >  		nfs4_stateid	delegation;		/* CLAIM_DELEGATE_CUR */
> >  		fmode_t		delegation_type;	/* CLAIM_PREVIOUS */
> >  	} u;
> 

Re: [PATCH 13/15] NFSv41: Clean up exclusive create

[Benny Halevy]

On Jun. 15, 2010, 11:50 -0400, Trond Myklebust <Trond.Myklebust@netapp.com> wrote:
> On Mon, 2010-06-14 at 19:13 -0400, Benny Halevy wrote:
>> On Jun. 14, 2010, 17:51 -0400, Trond Myklebust <Trond.Myklebust@netapp.com> wrote:
>>> Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
>>> ---
>>>  fs/nfs/nfs4proc.c       |   17 ++++++-----------
>>>  include/linux/nfs_xdr.h |    6 ++++--
>>>  2 files changed, 10 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>>> index ef613ea..e9913de 100644
>>> --- a/fs/nfs/nfs4proc.c
>>> +++ b/fs/nfs/nfs4proc.c
>>> @@ -744,19 +744,14 @@ static struct nfs4_opendata *nfs4_opendata_alloc(struct path *path,
>>>  	p->o_arg.server = server;
>>>  	p->o_arg.bitmask = server->attr_bitmask;
>>>  	p->o_arg.claim = NFS4_OPEN_CLAIM_NULL;
>>> -	if (flags & O_EXCL) {
>>> -		if (nfs4_has_persistent_session(server->nfs_client)) {
>>> -			/* GUARDED */
>>> -			p->o_arg.u.attrs = &p->attrs;
>>> -			memcpy(&p->attrs, attrs, sizeof(p->attrs));
>>
>> What about this case?
>> Aren't we losing the attrs?
> 
> I'm not sure what you mean. Are you asking what will happen if someone
> sets O_EXCL without setting O_CREAT? We don't call encode_createmode at
> all in that case. Otherwise, see below.
> 

It looks like in the O_CREAT|O_EXCL case with persistent session
the attrs passed in to nfs4_opendata_alloc are not copied to p->attrs
and p->o_arg.u.attrs is not set.
Then later on, in encode_createmode, in the respective path, we still use it:
			if (nfs4_has_persistent_session(clp)) {
				*p = cpu_to_be32(NFS4_CREATE_GUARDED);
				encode_attrs(xdr, arg->u.attrs, arg->server);
			} else {

Benny

> Cheers
>   Trond
> 
>>
>>> -		} else { /* EXCLUSIVE4_1 */
>>> -			u32 *s = (u32 *) p->o_arg.u.verifier.data;
>>> -			s[0] = jiffies;
>>> -			s[1] = current->pid;
>>> -		}
>>> -	} else if (flags & O_CREAT) {
>>> +	if (flags & O_CREAT) {
>>> +		u32 *s;
>>> +
>>>  		p->o_arg.u.attrs = &p->attrs;
>>>  		memcpy(&p->attrs, attrs, sizeof(p->attrs));
>>> +		s = (u32 *) p->o_arg.u.verifier.data;
>>> +		s[0] = jiffies;
>>> +		s[1] = current->pid;
>>>  	}
>>>  	p->c_arg.fh = &p->o_res.fh;
>>>  	p->c_arg.stateid = &p->o_res.stateid;
>>> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
>>> index 51914d7..a319cb9 100644
>>> --- a/include/linux/nfs_xdr.h
>>> +++ b/include/linux/nfs_xdr.h
>>> @@ -196,8 +196,10 @@ struct nfs_openargs {
>>>  	__u64                   clientid;
>>>  	__u64                   id;
>>>  	union {
>>> -		struct iattr *  attrs;    /* UNCHECKED, GUARDED */
>>> -		nfs4_verifier   verifier; /* EXCLUSIVE */
>>> +		struct {
>>> +			struct iattr *  attrs;    /* UNCHECKED, GUARDED */
>>> +			nfs4_verifier   verifier; /* EXCLUSIVE */
>>> +		};
>>>  		nfs4_stateid	delegation;		/* CLAIM_DELEGATE_CUR */
>>>  		fmode_t		delegation_type;	/* CLAIM_PREVIOUS */
>>>  	} u;
>>
> 
> 
> --
> 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

Re: [PATCH 13/15] NFSv41: Clean up exclusive create

[Trond Myklebust]

On Tue, 2010-06-15 at 12:08 -0400, Benny Halevy wrote:
> On Jun. 15, 2010, 11:50 -0400, Trond Myklebust <Trond.Myklebust@netapp.com> wrote:
> > On Mon, 2010-06-14 at 19:13 -0400, Benny Halevy wrote:
> >> On Jun. 14, 2010, 17:51 -0400, Trond Myklebust <Trond.Myklebust@netapp.com> wrote:
> >>> Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
> >>> ---
> >>>  fs/nfs/nfs4proc.c       |   17 ++++++-----------
> >>>  include/linux/nfs_xdr.h |    6 ++++--
> >>>  2 files changed, 10 insertions(+), 13 deletions(-)
> >>>
> >>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> >>> index ef613ea..e9913de 100644
> >>> --- a/fs/nfs/nfs4proc.c
> >>> +++ b/fs/nfs/nfs4proc.c
> >>> @@ -744,19 +744,14 @@ static struct nfs4_opendata *nfs4_opendata_alloc(struct path *path,
> >>>  	p->o_arg.server = server;
> >>>  	p->o_arg.bitmask = server->attr_bitmask;
> >>>  	p->o_arg.claim = NFS4_OPEN_CLAIM_NULL;
> >>> -	if (flags & O_EXCL) {
> >>> -		if (nfs4_has_persistent_session(server->nfs_client)) {
> >>> -			/* GUARDED */
> >>> -			p->o_arg.u.attrs = &p->attrs;
> >>> -			memcpy(&p->attrs, attrs, sizeof(p->attrs));
> >>
> >> What about this case?
> >> Aren't we losing the attrs?
> > 
> > I'm not sure what you mean. Are you asking what will happen if someone
> > sets O_EXCL without setting O_CREAT? We don't call encode_createmode at
> > all in that case. Otherwise, see below.
> > 
> 
> It looks like in the O_CREAT|O_EXCL case with persistent session
> the attrs passed in to nfs4_opendata_alloc are not copied to p->attrs
> and p->o_arg.u.attrs is not set.
> Then later on, in encode_createmode, in the respective path, we still use it:
> 			if (nfs4_has_persistent_session(clp)) {
> 				*p = cpu_to_be32(NFS4_CREATE_GUARDED);
> 				encode_attrs(xdr, arg->u.attrs, arg->server);
> 			} else {
> 
> Benny
> 
> > Cheers
> >   Trond
> > 
> >>
> >>> -		} else { /* EXCLUSIVE4_1 */
> >>> -			u32 *s = (u32 *) p->o_arg.u.verifier.data;
> >>> -			s[0] = jiffies;
> >>> -			s[1] = current->pid;
> >>> -		}
> >>> -	} else if (flags & O_CREAT) {
> >>> +	if (flags & O_CREAT) {
> >>> +		u32 *s;
> >>> +
> >>>  		p->o_arg.u.attrs = &p->attrs;
> >>>  		memcpy(&p->attrs, attrs, sizeof(p->attrs));
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Why isn't this sufficient?

> >>> +		s = (u32 *) p->o_arg.u.verifier.data;
> >>> +		s[0] = jiffies;
> >>> +		s[1] = current->pid;
> >>>  	}
> >>>  	p->c_arg.fh = &p->o_res.fh;
> >>>  	p->c_arg.stateid = &p->o_res.stateid;
> >>> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> >>> index 51914d7..a319cb9 100644
> >>> --- a/include/linux/nfs_xdr.h
> >>> +++ b/include/linux/nfs_xdr.h
> >>> @@ -196,8 +196,10 @@ struct nfs_openargs {
> >>>  	__u64                   clientid;
> >>>  	__u64                   id;
> >>>  	union {
> >>> -		struct iattr *  attrs;    /* UNCHECKED, GUARDED */
> >>> -		nfs4_verifier   verifier; /* EXCLUSIVE */
> >>> +		struct {
> >>> +			struct iattr *  attrs;    /* UNCHECKED, GUARDED */
> >>> +			nfs4_verifier   verifier; /* EXCLUSIVE */
> >>> +		};
> >>>  		nfs4_stateid	delegation;		/* CLAIM_DELEGATE_CUR */
> >>>  		fmode_t		delegation_type;	/* CLAIM_PREVIOUS */
> >>>  	} u;
> >>
> > 
> > 
> > --
> > 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

Re: [PATCH 13/15] NFSv41: Clean up exclusive create

[Benny Halevy]

On Jun. 15, 2010, 12:19 -0400, Trond Myklebust <Trond.Myklebust@netapp.com> wrote:
> On Tue, 2010-06-15 at 12:08 -0400, Benny Halevy wrote:
>> On Jun. 15, 2010, 11:50 -0400, Trond Myklebust <Trond.Myklebust@netapp.com> wrote:
>>> On Mon, 2010-06-14 at 19:13 -0400, Benny Halevy wrote:
>>>> On Jun. 14, 2010, 17:51 -0400, Trond Myklebust <Trond.Myklebust@netapp.com> wrote:
>>>>> Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
>>>>> ---
>>>>>  fs/nfs/nfs4proc.c       |   17 ++++++-----------
>>>>>  include/linux/nfs_xdr.h |    6 ++++--
>>>>>  2 files changed, 10 insertions(+), 13 deletions(-)
>>>>>
>>>>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>>>>> index ef613ea..e9913de 100644
>>>>> --- a/fs/nfs/nfs4proc.c
>>>>> +++ b/fs/nfs/nfs4proc.c
>>>>> @@ -744,19 +744,14 @@ static struct nfs4_opendata *nfs4_opendata_alloc(struct path *path,
>>>>>  	p->o_arg.server = server;
>>>>>  	p->o_arg.bitmask = server->attr_bitmask;
>>>>>  	p->o_arg.claim = NFS4_OPEN_CLAIM_NULL;
>>>>> -	if (flags & O_EXCL) {
>>>>> -		if (nfs4_has_persistent_session(server->nfs_client)) {
>>>>> -			/* GUARDED */
>>>>> -			p->o_arg.u.attrs = &p->attrs;
>>>>> -			memcpy(&p->attrs, attrs, sizeof(p->attrs));
>>>>
>>>> What about this case?
>>>> Aren't we losing the attrs?
>>>
>>> I'm not sure what you mean. Are you asking what will happen if someone
>>> sets O_EXCL without setting O_CREAT? We don't call encode_createmode at
>>> all in that case. Otherwise, see below.
>>>
>>
>> It looks like in the O_CREAT|O_EXCL case with persistent session
>> the attrs passed in to nfs4_opendata_alloc are not copied to p->attrs
>> and p->o_arg.u.attrs is not set.
>> Then later on, in encode_createmode, in the respective path, we still use it:
>> 			if (nfs4_has_persistent_session(clp)) {
>> 				*p = cpu_to_be32(NFS4_CREATE_GUARDED);
>> 				encode_attrs(xdr, arg->u.attrs, arg->server);
>> 			} else {
>>
>> Benny
>>
>>> Cheers
>>>   Trond
>>>
>>>>
>>>>> -		} else { /* EXCLUSIVE4_1 */
>>>>> -			u32 *s = (u32 *) p->o_arg.u.verifier.data;
>>>>> -			s[0] = jiffies;
>>>>> -			s[1] = current->pid;
>>>>> -		}
>>>>> -	} else if (flags & O_CREAT) {
>>>>> +	if (flags & O_CREAT) {
>>>>> +		u32 *s;
>>>>> +
>>>>>  		p->o_arg.u.attrs = &p->attrs;
>>>>>  		memcpy(&p->attrs, attrs, sizeof(p->attrs));
>                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> Why isn't this sufficient?
> 

Duh. Sorry, I missed that.

Benny

>>>>> +		s = (u32 *) p->o_arg.u.verifier.data;
>>>>> +		s[0] = jiffies;
>>>>> +		s[1] = current->pid;
>>>>>  	}
>>>>>  	p->c_arg.fh = &p->o_res.fh;
>>>>>  	p->c_arg.stateid = &p->o_res.stateid;
>>>>> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
>>>>> index 51914d7..a319cb9 100644
>>>>> --- a/include/linux/nfs_xdr.h
>>>>> +++ b/include/linux/nfs_xdr.h
>>>>> @@ -196,8 +196,10 @@ struct nfs_openargs {
>>>>>  	__u64                   clientid;
>>>>>  	__u64                   id;
>>>>>  	union {
>>>>> -		struct iattr *  attrs;    /* UNCHECKED, GUARDED */
>>>>> -		nfs4_verifier   verifier; /* EXCLUSIVE */
>>>>> +		struct {
>>>>> +			struct iattr *  attrs;    /* UNCHECKED, GUARDED */
>>>>> +			nfs4_verifier   verifier; /* EXCLUSIVE */
>>>>> +		};
>>>>>  		nfs4_stateid	delegation;		/* CLAIM_DELEGATE_CUR */
>>>>>  		fmode_t		delegation_type;	/* CLAIM_PREVIOUS */
>>>>>  	} u;
>>>>
>>>
>>>
>>> --
>>> 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
> 
> 
> --
> 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

RE: [PATCH 02/15] NFSv4.1: Clean up nfs4_setup_sequence

[Gilliam, PaulX J]

> -----Original Message-----
> From: linux-nfs-owner@vger.kernel.org [mailto:linux-nfs-owner@vger.kernel.org] On Behalf Of Trond Myklebust
> Sent: Monday, June 14, 2010 2:51 PM
> To: linux-nfs@vger.kernel.org
> Subject: [PATCH 02/15] NFSv4.1: Clean up nfs4_setup_sequence
> 
> Firstly, there is little point in first zeroing out the entire struct
> nfs4_sequence_res, and then initialising all fields save one. Just
> initialise the last field to zero...

The reason one may want to zero out the entire struct is that in the future, someone may add elements to the struct.  In that case, 
	memset(res, 0, sizeof(*res));
would not have to be changed, and any new elements will "automatically" be initialized to a known value.

Just a thought.

-=# Paul Gilliam #=-

RE: [PATCH 02/15] NFSv4.1: Clean up nfs4_setup_sequence

[Trond Myklebust]

On Tue, 2010-06-15 at 09:50 -0700, Gilliam, PaulX J wrote:
> > -----Original Message-----
> > From: linux-nfs-owner@vger.kernel.org [mailto:linux-nfs-owner@vger.kernel.org] On Behalf Of Trond Myklebust
> > Sent: Monday, June 14, 2010 2:51 PM
> > To: linux-nfs@vger.kernel.org
> > Subject: [PATCH 02/15] NFSv4.1: Clean up nfs4_setup_sequence
> > 
> > Firstly, there is little point in first zeroing out the entire struct
> > nfs4_sequence_res, and then initialising all fields save one. Just
> > initialise the last field to zero...
> 
> The reason one may want to zero out the entire struct is that in the future, someone may add elements to the struct.  In that case, 
> 	memset(res, 0, sizeof(*res));
> would not have to be changed, and any new elements will "automatically" be initialized to a known value.
> 
> Just a thought.

That assumes this is a structure that is likely to change and/or be
extended in the future, which is unlikely since the NFSv4.1 protocol
specification is complete and the SEQUENCE results are fully contained
in the current set of fields.

It also assumes that '0' would be a desirable default value for these
new fields.

Trond

Re: [PATCH 02/15] NFSv4.1: Clean up nfs4_setup_sequence

[J. Bruce Fields]

On Tue, Jun 15, 2010 at 01:32:21PM -0400, Trond Myklebust wrote:
> On Tue, 2010-06-15 at 09:50 -0700, Gilliam, PaulX J wrote:
> > > -----Original Message-----
> > > From: linux-nfs-owner@vger.kernel.org [mailto:linux-nfs-owner@vger.kernel.org] On Behalf Of Trond Myklebust
> > > Sent: Monday, June 14, 2010 2:51 PM
> > > To: linux-nfs@vger.kernel.org
> > > Subject: [PATCH 02/15] NFSv4.1: Clean up nfs4_setup_sequence
> > > 
> > > Firstly, there is little point in first zeroing out the entire struct
> > > nfs4_sequence_res, and then initialising all fields save one. Just
> > > initialise the last field to zero...
> > 
> > The reason one may want to zero out the entire struct is that in the future, someone may add elements to the struct.  In that case, 
> > 	memset(res, 0, sizeof(*res));
> > would not have to be changed, and any new elements will "automatically" be initialized to a known value.
> > 
> > Just a thought.
> 
> That assumes this is a structure that is likely to change and/or be
> extended in the future, which is unlikely since the NFSv4.1 protocol
> specification is complete and the SEQUENCE results are fully contained
> in the current set of fields.
> 
> It also assumes that '0' would be a desirable default value for these
> new fields.

Also, the memset's/kzalloc's mean that "git grep <structure field name>"
doesn't turn up the initialization of the field.  Not a huge deal, but
it trips me up sometimes.

--b.

RE: [PATCH 02/15] NFSv4.1: Clean up nfs4_setup_sequence

[Gilliam, PaulX J]

>-----Original Message-----
>From: Trond Myklebust [mailto:trond.myklebust@fys.uio.no]
>Sent: Tuesday, June 15, 2010 10:32 AM
>To: Gilliam, PaulX J
>Cc: linux-nfs@vger.kernel.org
>Subject: RE: [PATCH 02/15] NFSv4.1: Clean up nfs4_setup_sequence
>
>On Tue, 2010-06-15 at 09:50 -0700, Gilliam, PaulX J wrote:
>> > -----Original Message-----
>> > From: linux-nfs-owner@vger.kernel.org [mailto:linux-nfs-
>owner@vger.kernel.org] On Behalf Of Trond Myklebust
>> > Sent: Monday, June 14, 2010 2:51 PM
>> > To: linux-nfs@vger.kernel.org
>> > Subject: [PATCH 02/15] NFSv4.1: Clean up nfs4_setup_sequence
>> >
>> > Firstly, there is little point in first zeroing out the entire struct
>> > nfs4_sequence_res, and then initialising all fields save one. Just
>> > initialise the last field to zero...
>>
>> The reason one may want to zero out the entire struct is that in the
>future, someone may add elements to the struct.  In that case,
>> 	memset(res, 0, sizeof(*res));
>> would not have to be changed, and any new elements will "automatically"
>be initialized to a known value.
>>
>> Just a thought.
>
>That assumes this is a structure that is likely to change and/or be
>extended in the future, which is unlikely since the NFSv4.1 protocol
>specification is complete and the SEQUENCE results are fully contained
>in the current set of fields.

Good point.


>
>It also assumes that '0' would be a desirable default value for these
>new fields.
>
>Trond

I think '0' would be a desirable default only in that it could lead to detecting errors earlier.


Back in olden times, Windows NT came in two flavors: "checked" and "unchecked".  The "checked" version was much slower and took more resources, but had a ton of "extra" code to check for bad things.  It was expected that the "unchecked" version would actually be the one for daily use.

If Linux had a similar thing (maybe it does?) one could use something like this:

#ifdef(CHECKED)
	memset(res, 0, sizeof(*res));
#endif


Maybe not in this specific case, but in general.

I'll shut up now and go back to lurking.

-=# Paul Gilliam #=-