2014-09-23 06:56:28

by Christoph Hellwig

[permalink] [raw]
Subject: CB_RECALL fixes

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.



2014-09-24 08:22:07

by Christoph Hellwig

[permalink] [raw]
Subject: Re: CB_RECALL fixes

On Tue, Sep 23, 2014 at 03:33:02PM -0400, J. Bruce Fields wrote:
> On Tue, Sep 23, 2014 at 08:58:47AM +0200, Christoph Hellwig wrote:
> > 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: [email protected] ?)

Yes, and I'll send it to you with cc to linux-nfs next time, sorry.

2014-09-23 06:56:28

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 1/2] nfsd: fix nfsd4_cb_recall_done error handling

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 <[email protected]>
---
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


2014-09-23 06:56:30

by Christoph Hellwig

[permalink] [raw]
Subject: [PATCH 2/2] nfsd: do not clear rpc_resp in nfsd4_cb_done_sequence

From: Benny Halevy <[email protected]>

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 <[email protected]>
Signed-off-by: Christoph Hellwig <[email protected]>
---
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


2014-09-23 19:33:04

by J. Bruce Fields

[permalink] [raw]
Subject: Re: CB_RECALL fixes

On Tue, Sep 23, 2014 at 08:58:47AM +0200, Christoph Hellwig wrote:
> 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: [email protected] ?)

--b.