Commit 7dc2993a authored by Robert Milkowski's avatar Robert Milkowski Committed by Anna Schumaker

NFSv4.0: nfs4_do_fsinfo() should not do implicit lease renewals

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 83ca7f5a ("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: 83ca7f5a ("NFS: Avoid PUTROOTFH when managing leases")
Signed-off-by: default avatarRobert Milkowski <rmilkowski@gmail.com>
Signed-off-by: default avatarAnna Schumaker <Anna.Schumaker@Netapp.com>
parent 924491f2
...@@ -447,9 +447,7 @@ extern void nfs4_schedule_state_renewal(struct nfs_client *); ...@@ -447,9 +447,7 @@ extern void nfs4_schedule_state_renewal(struct nfs_client *);
extern void nfs4_renewd_prepare_shutdown(struct nfs_server *); extern void nfs4_renewd_prepare_shutdown(struct nfs_server *);
extern void nfs4_kill_renewd(struct nfs_client *); extern void nfs4_kill_renewd(struct nfs_client *);
extern void nfs4_renew_state(struct work_struct *); extern void nfs4_renew_state(struct work_struct *);
extern void nfs4_set_lease_period(struct nfs_client *clp, extern void nfs4_set_lease_period(struct nfs_client *clp, unsigned long lease);
unsigned long lease,
unsigned long lastrenewed);
/* nfs4state.c */ /* nfs4state.c */
......
...@@ -5053,16 +5053,13 @@ static int nfs4_do_fsinfo(struct nfs_server *server, struct nfs_fh *fhandle, str ...@@ -5053,16 +5053,13 @@ static int nfs4_do_fsinfo(struct nfs_server *server, struct nfs_fh *fhandle, str
struct nfs4_exception exception = { struct nfs4_exception exception = {
.interruptible = true, .interruptible = true,
}; };
unsigned long now = jiffies;
int err; int err;
do { do {
err = _nfs4_do_fsinfo(server, fhandle, fsinfo); err = _nfs4_do_fsinfo(server, fhandle, fsinfo);
trace_nfs4_fsinfo(server, fhandle, fsinfo->fattr, err); trace_nfs4_fsinfo(server, fhandle, fsinfo->fattr, err);
if (err == 0) { if (err == 0) {
nfs4_set_lease_period(server->nfs_client, nfs4_set_lease_period(server->nfs_client, fsinfo->lease_time * HZ);
fsinfo->lease_time * HZ,
now);
break; break;
} }
err = nfs4_handle_exception(server, err, &exception); err = nfs4_handle_exception(server, err, &exception);
...@@ -6126,6 +6123,7 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program, ...@@ -6126,6 +6123,7 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program,
.callback_data = &setclientid, .callback_data = &setclientid,
.flags = RPC_TASK_TIMEOUT | RPC_TASK_NO_ROUND_ROBIN, .flags = RPC_TASK_TIMEOUT | RPC_TASK_NO_ROUND_ROBIN,
}; };
unsigned long now = jiffies;
int status; int status;
/* nfs_client_id4 */ /* nfs_client_id4 */
...@@ -6158,6 +6156,9 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program, ...@@ -6158,6 +6156,9 @@ int nfs4_proc_setclientid(struct nfs_client *clp, u32 program,
clp->cl_acceptor = rpcauth_stringify_acceptor(setclientid.sc_cred); clp->cl_acceptor = rpcauth_stringify_acceptor(setclientid.sc_cred);
put_rpccred(setclientid.sc_cred); put_rpccred(setclientid.sc_cred);
} }
if (status == 0)
do_renew_lease(clp, now);
out: out:
trace_nfs4_setclientid(clp, status); trace_nfs4_setclientid(clp, status);
dprintk("NFS reply setclientid: %d\n", status); dprintk("NFS reply setclientid: %d\n", status);
...@@ -8245,6 +8246,7 @@ static int _nfs4_proc_exchange_id(struct nfs_client *clp, const struct cred *cre ...@@ -8245,6 +8246,7 @@ static int _nfs4_proc_exchange_id(struct nfs_client *clp, const struct cred *cre
struct rpc_task *task; struct rpc_task *task;
struct nfs41_exchange_id_args *argp; struct nfs41_exchange_id_args *argp;
struct nfs41_exchange_id_res *resp; struct nfs41_exchange_id_res *resp;
unsigned long now = jiffies;
int status; int status;
task = nfs4_run_exchange_id(clp, cred, sp4_how, NULL); task = nfs4_run_exchange_id(clp, cred, sp4_how, NULL);
...@@ -8265,6 +8267,8 @@ static int _nfs4_proc_exchange_id(struct nfs_client *clp, const struct cred *cre ...@@ -8265,6 +8267,8 @@ static int _nfs4_proc_exchange_id(struct nfs_client *clp, const struct cred *cre
if (status != 0) if (status != 0)
goto out; goto out;
do_renew_lease(clp, now);
clp->cl_clientid = resp->clientid; clp->cl_clientid = resp->clientid;
clp->cl_exchange_flags = resp->flags; clp->cl_exchange_flags = resp->flags;
clp->cl_seqid = resp->seqid; clp->cl_seqid = resp->seqid;
......
...@@ -138,15 +138,12 @@ nfs4_kill_renewd(struct nfs_client *clp) ...@@ -138,15 +138,12 @@ nfs4_kill_renewd(struct nfs_client *clp)
* *
* @clp: pointer to nfs_client * @clp: pointer to nfs_client
* @lease: new value for lease period * @lease: new value for lease period
* @lastrenewed: time at which lease was last renewed
*/ */
void nfs4_set_lease_period(struct nfs_client *clp, void nfs4_set_lease_period(struct nfs_client *clp,
unsigned long lease, unsigned long lease)
unsigned long lastrenewed)
{ {
spin_lock(&clp->cl_lock); spin_lock(&clp->cl_lock);
clp->cl_lease_time = lease; clp->cl_lease_time = lease;
clp->cl_last_renewal = lastrenewed;
spin_unlock(&clp->cl_lock); spin_unlock(&clp->cl_lock);
/* Cap maximum reconnect timeout at 1/2 lease period */ /* Cap maximum reconnect timeout at 1/2 lease period */
......
...@@ -92,17 +92,15 @@ static int nfs4_setup_state_renewal(struct nfs_client *clp) ...@@ -92,17 +92,15 @@ static int nfs4_setup_state_renewal(struct nfs_client *clp)
{ {
int status; int status;
struct nfs_fsinfo fsinfo; struct nfs_fsinfo fsinfo;
unsigned long now;
if (!test_bit(NFS_CS_CHECK_LEASE_TIME, &clp->cl_res_state)) { if (!test_bit(NFS_CS_CHECK_LEASE_TIME, &clp->cl_res_state)) {
nfs4_schedule_state_renewal(clp); nfs4_schedule_state_renewal(clp);
return 0; return 0;
} }
now = jiffies;
status = nfs4_proc_get_lease_time(clp, &fsinfo); status = nfs4_proc_get_lease_time(clp, &fsinfo);
if (status == 0) { 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); nfs4_schedule_state_renewal(clp);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment