| From 778a923f38d113cc09669042d01e98d9b8bbfb53 Mon Sep 17 00:00:00 2001 |
| From: Robert Milkowski <rmilkowski@gmail.com> |
| Date: Thu, 30 Jan 2020 09:43:25 +0000 |
| Subject: [PATCH] NFSv4.0: nfs4_do_fsinfo() should not do implicit lease |
| renewals |
| |
| commit 7dc2993a9e51dd2eee955944efec65bef90265b7 upstream. |
| |
| Currently, each time nfs4_do_fsinfo() is called it will do an implicit |
| NFS4 lease renewal, which is not compliant with the NFS4 specification. |
| This can result in a lease being expired by an NFS server. |
| |
| Commit 83ca7f5ab31f ("NFS: Avoid PUTROOTFH when managing leases") |
| introduced implicit client lease renewal in nfs4_do_fsinfo(), |
| which can result in the NFSv4.0 lease to expire on a server side, |
| and servers returning NFS4ERR_EXPIRED or NFS4ERR_STALE_CLIENTID. |
| |
| This can easily be reproduced by frequently unmounting a sub-mount, |
| then stat'ing it to get it mounted again, which will delay or even |
| completely prevent client from sending RENEW operations if no other |
| NFS operations are issued. Eventually nfs server will expire client's |
| lease and return an error on file access or next RENEW. |
| |
| This can also happen when a sub-mount is automatically unmounted |
| due to inactivity (after nfs_mountpoint_expiry_timeout), then it is |
| mounted again via stat(). This can result in a short window during |
| which client's lease will expire on a server but not on a client. |
| This specific case was observed on production systems. |
| |
| This patch removes the implicit lease renewal from nfs4_do_fsinfo(). |
| |
| Fixes: 83ca7f5ab31f ("NFS: Avoid PUTROOTFH when managing leases") |
| Signed-off-by: Robert Milkowski <rmilkowski@gmail.com> |
| Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com> |
| Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com> |
| |
| diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h |
| index 235919156edd..c9113808e68e 100644 |
| --- a/fs/nfs/nfs4_fs.h |
| +++ b/fs/nfs/nfs4_fs.h |
| @@ -439,9 +439,7 @@ extern void nfs4_schedule_state_renewal(struct nfs_client *); |
| extern void nfs4_renewd_prepare_shutdown(struct nfs_server *); |
| extern void nfs4_kill_renewd(struct nfs_client *); |
| extern void nfs4_renew_state(struct work_struct *); |
| -extern void nfs4_set_lease_period(struct nfs_client *clp, |
| - unsigned long lease, |
| - unsigned long lastrenewed); |
| +extern void nfs4_set_lease_period(struct nfs_client *clp, unsigned long lease); |
| |
| |
| /* nfs4state.c */ |
| diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c |
| index 11c0100d09e0..73729fa91026 100644 |
| --- a/fs/nfs/nfs4proc.c |
| +++ b/fs/nfs/nfs4proc.c |
| @@ -4942,16 +4942,13 @@ static int nfs4_do_fsinfo(struct nfs_server *server, struct nfs_fh *fhandle, str |
| struct nfs4_exception exception = { |
| .interruptible = true, |
| }; |
| - unsigned long now = jiffies; |
| int err; |
| |
| do { |
| err = _nfs4_do_fsinfo(server, fhandle, fsinfo); |
| trace_nfs4_fsinfo(server, fhandle, fsinfo->fattr, err); |
| if (err == 0) { |
| - nfs4_set_lease_period(server->nfs_client, |
| - fsinfo->lease_time * HZ, |
| - now); |
| + nfs4_set_lease_period(server->nfs_client, fsinfo->lease_time * HZ); |
| break; |
| } |
| err = nfs4_handle_exception(server, err, &exception); |
| @@ -6008,6 +6005,7 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program, |
| .callback_data = &setclientid, |
| .flags = RPC_TASK_TIMEOUT, |
| }; |
| + unsigned long now = jiffies; |
| int status; |
| |
| /* nfs_client_id4 */ |
| @@ -6045,6 +6043,9 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program, |
| put_rpccred(setclientid.sc_cred); |
| } |
| rpc_put_task(task); |
| + |
| + if (status == 0) |
| + do_renew_lease(clp, now); |
| out: |
| trace_nfs4_setclientid(clp, status); |
| dprintk("NFS reply setclientid: %d\n", status); |
| @@ -8094,6 +8095,7 @@ static int _nfs4_proc_exchange_id(struct nfs_client *clp, const struct cred *cre |
| struct rpc_task *task; |
| struct nfs41_exchange_id_args *argp; |
| struct nfs41_exchange_id_res *resp; |
| + unsigned long now = jiffies; |
| int status; |
| |
| task = nfs4_run_exchange_id(clp, cred, sp4_how, NULL); |
| @@ -8114,6 +8116,8 @@ static int _nfs4_proc_exchange_id(struct nfs_client *clp, const struct cred *cre |
| if (status != 0) |
| goto out; |
| |
| + do_renew_lease(clp, now); |
| + |
| clp->cl_clientid = resp->clientid; |
| clp->cl_exchange_flags = resp->flags; |
| clp->cl_seqid = resp->seqid; |
| diff --git a/fs/nfs/nfs4renewd.c b/fs/nfs/nfs4renewd.c |
| index 6ea431b067dd..ff876dda7f06 100644 |
| --- a/fs/nfs/nfs4renewd.c |
| +++ b/fs/nfs/nfs4renewd.c |
| @@ -138,15 +138,12 @@ nfs4_kill_renewd(struct nfs_client *clp) |
| * |
| * @clp: pointer to nfs_client |
| * @lease: new value for lease period |
| - * @lastrenewed: time at which lease was last renewed |
| */ |
| void nfs4_set_lease_period(struct nfs_client *clp, |
| - unsigned long lease, |
| - unsigned long lastrenewed) |
| + unsigned long lease) |
| { |
| spin_lock(&clp->cl_lock); |
| clp->cl_lease_time = lease; |
| - clp->cl_last_renewal = lastrenewed; |
| spin_unlock(&clp->cl_lock); |
| |
| /* Cap maximum reconnect timeout at 1/2 lease period */ |
| diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c |
| index 0e69cd846afb..ca70d60ad540 100644 |
| --- a/fs/nfs/nfs4state.c |
| +++ b/fs/nfs/nfs4state.c |
| @@ -290,17 +290,15 @@ static int nfs41_setup_state_renewal(struct nfs_client *clp) |
| { |
| int status; |
| struct nfs_fsinfo fsinfo; |
| - unsigned long now; |
| |
| if (!test_bit(NFS_CS_CHECK_LEASE_TIME, &clp->cl_res_state)) { |
| nfs4_schedule_state_renewal(clp); |
| return 0; |
| } |
| |
| - now = jiffies; |
| status = nfs4_proc_get_lease_time(clp, &fsinfo); |
| if (status == 0) { |
| - nfs4_set_lease_period(clp, fsinfo.lease_time * HZ, now); |
| + nfs4_set_lease_period(clp, fsinfo.lease_time * HZ); |
| nfs4_schedule_state_renewal(clp); |
| } |
| |
| -- |
| 2.7.4 |
| |