On 12/15/23 11:54 AM, Jeff Layton wrote:
On Fri, 2023-12-15 at 11:15 -0800, Dai Ngo wrote:
If the callback workqueue is stuck, nfsd4_deleg_getattr_conflict will
also stuck waiting for the callback request to be executed. This causes
the client to hang waiting for the reply of the GETATTR and also causes
the reboot of the NFS server to hang due to the pending NFS request.
Fix by replacing wait_on_bit with wait_on_bit_timeout with 20 seconds
time out.
Reported-by: Wolfgang Walter <linux-nfs@xxxxxxx>
Fixes: 6c41d9a9bd02 ("NFSD: handle GETATTR conflict with write delegation")
Signed-off-by: Dai Ngo <dai.ngo@xxxxxxxxxx>
---
fs/nfsd/nfs4state.c | 6 +++++-
fs/nfsd/state.h | 2 ++
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 175f3e9f5822..0cc7d4953807 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2948,6 +2948,9 @@ void nfs4_cb_getattr(struct nfs4_cb_fattr *ncf)
if (test_and_set_bit(CB_GETATTR_BUSY, &ncf->ncf_cb_flags))
return;
+ /* set to proper status when nfsd4_cb_getattr_done runs */
+ ncf->ncf_cb_status = NFS4ERR_IO;
+
refcount_inc(&dp->dl_stid.sc_count);
if (!nfsd4_run_cb(&ncf->ncf_getattr)) {
refcount_dec(&dp->dl_stid.sc_count);
@@ -8558,7 +8561,8 @@ nfsd4_deleg_getattr_conflict(struct svc_rqst *rqstp, struct inode *inode,
nfs4_cb_getattr(&dp->dl_cb_fattr);
spin_unlock(&ctx->flc_lock);
- wait_on_bit(&ncf->ncf_cb_flags, CB_GETATTR_BUSY, TASK_INTERRUPTIBLE);
+ wait_on_bit_timeout(&ncf->ncf_cb_flags, CB_GETATTR_BUSY,
+ TASK_INTERRUPTIBLE, NFSD_CB_GETATTR_TIMEOUT);
The RPC won't necessarily have timed out at this point, and it looks
like ncf_cb_status won't have been set to anything (and is probably
still 0?).
The timeout was added to handle the case where the callback request
did not get queued to the workqueue; nfsd4_run_cb fails. In this case
RPC is not involved and we don't want to hang here. Note that this patch
sets ncf_cb_status to NFS4ERR_IO before calling nfsd4_run_cb so we can
detect this error condition.
Don't you need to check whether the wait timed out or was successful?
ncf_cb_status is set to tk_status by nfsd4_cb_getattr_done. If the request
was successful then ncf_cb_status is 0.
What happens now when this times out?
Then we go through the normal logic of nfsd_open_break_lease which will
also get timed out but eventually the lease, delegation state, will be
removed by __break_lease after 45 secs (lease_break_time).
-Dai
if (ncf->ncf_cb_status) {
status = nfserrno(nfsd_open_break_lease(inode, NFSD_MAY_READ));
if (status != nfserr_jukebox ||
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index f96eaa8e9413..94563a6813a6 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -135,6 +135,8 @@ struct nfs4_cb_fattr {
/* bits for ncf_cb_flags */
#define CB_GETATTR_BUSY 0
+#define NFSD_CB_GETATTR_TIMEOUT msecs_to_jiffies(20000) /* 20 secs */
+
Why 20s?
RPC will time out after 9 secs if it does not receive a callback reply.
This time out value needs to be greater than 9 secs. I just be generous
here, we can reduce it to any value > 9 secs.
-Dai
/*
* Represents a delegation stateid. The nfs4_client holds references to these
* and they are put when it is being destroyed or when the delegation is
