Discussion:
[PATCH 1/2] nfsd: fix nfsd4_cb_recall_done error handling
Christoph Hellwig
2014-09-23 06:58:48 UTC
Permalink
For any error that is not EBADHANDLE or NFS4ERR_BAD_STATEID,
nfsd4_cb_recall_done first marks the connection down, then
retries until dl_retries hits zero, then marks the connection down
again and sets cb_done. This changes the code to only retry
for EBADHANDLE or NFS4ERR_BAD_STATEID, and factors setting
cb_done into a single point in the function.

Signed-off-by: Christoph Hellwig <hch-***@public.gmane.org>
---
fs/nfsd/nfs4callback.c | 17 +++++++----------
1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
index e0be57b..795e218 100644
--- a/fs/nfsd/nfs4callback.c
+++ b/fs/nfsd/nfs4callback.c
@@ -874,24 +874,21 @@ static void nfsd4_cb_recall_done(struct rpc_task *task, void *calldata)
return;
switch (task->tk_status) {
case 0:
- cb->cb_done = true;
- return;
+ break;
case -EBADHANDLE:
case -NFS4ERR_BAD_STATEID:
/* Race: client probably got cb_recall
* before open reply granting delegation */
- break;
+ if (dp->dl_retries--) {
+ rpc_delay(task, 2*HZ);
+ task->tk_status = 0;
+ rpc_restart_call_prepare(task);
+ return;
+ }
default:
/* Network partition? */
nfsd4_mark_cb_down(clp, task->tk_status);
}
- if (dp->dl_retries--) {
- rpc_delay(task, 2*HZ);
- task->tk_status = 0;
- rpc_restart_call_prepare(task);
- return;
- }
- nfsd4_mark_cb_down(clp, task->tk_status);
cb->cb_done = true;
}
--
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Christoph Hellwig
2014-09-23 06:58:49 UTC
Permalink
From: Benny Halevy <bhalevy-***@public.gmane.org>

This is incorrect when a callback is has to be restarted, in which case
the XDR decoding of the second iteration will see a NULL cb argument.

[hch: updated description]
Signed-off-by: Benny Halevy <bhalevy-***@public.gmane.org>
Signed-off-by: Christoph Hellwig <hch-***@public.gmane.org>
---
fs/nfsd/nfs4callback.c | 3 ---
1 file changed, 3 deletions(-)

diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
index 795e218..fc07084 100644
--- a/fs/nfsd/nfs4callback.c
+++ b/fs/nfsd/nfs4callback.c
@@ -847,9 +847,6 @@ static void nfsd4_cb_done(struct rpc_task *task, void *calldata)
rpc_wake_up_next(&clp->cl_cb_waitq);
dprintk("%s: freed slot, new seqid=%d\n", __func__,
clp->cl_cb_session->se_cb_seq_nr);
-
- /* We're done looking into the sequence information */
- task->tk_msg.rpc_resp = NULL;
}
}
--
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
J. Bruce Fields
2014-09-23 19:33:02 UTC
Permalink
First one fixes error handling in nfsd4_cb_recall_done, second is a fix from
Benny in the old pnfs tree which applies to restarted delegation recalls
as well.
Thanks, applying both for 3.18.

(Assuming that's what you intended. When you want nfsd patches applied,
could you do an explicit To: bfields-***@public.gmane.org ?)

--b.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Christoph Hellwig
2014-09-24 08:22:04 UTC
Permalink
Post by J. Bruce Fields
First one fixes error handling in nfsd4_cb_recall_done, second is a fix from
Benny in the old pnfs tree which applies to restarted delegation recalls
as well.
Thanks, applying both for 3.18.
(Assuming that's what you intended. When you want nfsd patches applied,
Yes, and I'll send it to you with cc to linux-nfs next time, sorry.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-***@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Loading...