* [PATCH 1/4] nfs: kill renewd before clearing the client session @ 2010-01-27 2:43 Alexandros Batsakis 2010-01-27 2:43 ` [PATCH 2/4] nfs4: prevent backlogging of renewd requests Alexandros Batsakis 0 siblings, 1 reply; 6+ messages in thread From: Alexandros Batsakis @ 2010-01-27 2:43 UTC (permalink / raw) To: linux-nfs; +Cc: trond, Alexandros Batsakis Signed-off-by: Alexandros Batsakis <batsakis@netapp.com> --- fs/nfs/client.c | 47 +++++++++++++++++++++++------------------------ 1 files changed, 23 insertions(+), 24 deletions(-) diff --git a/fs/nfs/client.c b/fs/nfs/client.c index d0b060a..07e4b56 100644 --- a/fs/nfs/client.c +++ b/fs/nfs/client.c @@ -164,30 +164,6 @@ error_0: return ERR_PTR(err); } -static void nfs4_shutdown_client(struct nfs_client *clp) -{ -#ifdef CONFIG_NFS_V4 - if (__test_and_clear_bit(NFS_CS_RENEWD, &clp->cl_res_state)) - nfs4_kill_renewd(clp); - BUG_ON(!RB_EMPTY_ROOT(&clp->cl_state_owners)); - if (__test_and_clear_bit(NFS_CS_IDMAP, &clp->cl_res_state)) - nfs_idmap_delete(clp); - - rpc_destroy_wait_queue(&clp->cl_rpcwaitq); -#endif -} - -/* - * Destroy the NFS4 callback service - */ -static void nfs4_destroy_callback(struct nfs_client *clp) -{ -#ifdef CONFIG_NFS_V4 - if (__test_and_clear_bit(NFS_CS_CALLBACK, &clp->cl_res_state)) - nfs_callback_down(clp->cl_minorversion); -#endif /* CONFIG_NFS_V4 */ -} - /* * Clears/puts all minor version specific parts from an nfs_client struct * reverting it to minorversion 0. @@ -202,9 +178,31 @@ static void nfs4_clear_client_minor_version(struct nfs_client *clp) clp->cl_call_sync = _nfs4_call_sync; #endif /* CONFIG_NFS_V4_1 */ +} + +/* + * Destroy the NFS4 callback service + */ +static void nfs4_destroy_callback(struct nfs_client *clp) +{ +#ifdef CONFIG_NFS_V4 + if (__test_and_clear_bit(NFS_CS_CALLBACK, &clp->cl_res_state)) + nfs_callback_down(clp->cl_minorversion); +#endif /* CONFIG_NFS_V4 */ +} +static void nfs4_shutdown_client(struct nfs_client *clp) +{ +#ifdef CONFIG_NFS_V4 + if (__test_and_clear_bit(NFS_CS_RENEWD, &clp->cl_res_state)) + nfs4_kill_renewd(clp); + nfs4_clear_client_minor_version(clp); nfs4_destroy_callback(clp); + if (__test_and_clear_bit(NFS_CS_IDMAP, &clp->cl_res_state)) + nfs_idmap_delete(clp); + + rpc_destroy_wait_queue(&clp->cl_rpcwaitq); +#endif } /* -- 1.6.2.5 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/4] nfs4: prevent backlogging of renewd requests 2010-01-27 2:43 [PATCH 1/4] nfs: kill renewd before clearing the client session Alexandros Batsakis @ 2010-01-27 2:43 ` Alexandros Batsakis 2010-01-27 2:43 ` [PATCH 3/4] nfs41: fix race between umount and renewd sequence operations Alexandros Batsakis 0 siblings, 1 reply; 6+ messages in thread From: Alexandros Batsakis @ 2010-01-27 2:43 UTC (permalink / raw) To: linux-nfs; +Cc: trond, Alexandros Batsakis Signed-off-by: Alexandros Batsakis <batsakis@netapp.com> --- fs/nfs/nfs4proc.c | 3 +++ fs/nfs/nfs4renewd.c | 13 ------------- 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 3644c35..9fc99e9 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -3162,6 +3162,7 @@ static void nfs4_renew_done(struct rpc_task *task, void *data) if (time_before(clp->cl_last_renewal,timestamp)) clp->cl_last_renewal = timestamp; spin_unlock(&clp->cl_lock); + nfs4_schedule_state_renewal(clp); } static const struct rpc_call_ops nfs4_renew_ops = { @@ -5049,6 +5050,8 @@ void nfs41_sequence_call_done(struct rpc_task *task, void *data) } dprintk("%s rpc_cred %p\n", __func__, task->tk_msg.rpc_cred); + nfs4_schedule_state_renewal(clp); + kfree(task->tk_msg.rpc_argp); kfree(task->tk_msg.rpc_resp); diff --git a/fs/nfs/nfs4renewd.c b/fs/nfs/nfs4renewd.c index 0156c01..f514902 100644 --- a/fs/nfs/nfs4renewd.c +++ b/fs/nfs/nfs4renewd.c @@ -36,11 +36,6 @@ * as an rpc_task, not a real kernel thread, so it always runs in rpciod's * context. There is one renewd per nfs_server. * - * TODO: If the send queue gets backlogged (e.g., if the server goes down), - * we will keep filling the queue with periodic RENEW requests. We need a - * mechanism for ensuring that if renewd successfully sends off a request, - * then it only wakes up when the request is finished. Maybe use the - * child task framework of the RPC layer? */ #include <linux/mm.h> @@ -92,17 +87,9 @@ nfs4_renew_state(struct work_struct *work) put_rpccred(cred); } timeout = (2 * lease) / 3; - spin_lock(&clp->cl_lock); } else dprintk("%s: failed to call renewd. Reason: lease not expired \n", __func__); - if (timeout < 5 * HZ) /* safeguard */ - timeout = 5 * HZ; - dprintk("%s: requeueing work. Lease period = %ld\n", - __func__, (timeout + HZ - 1) / HZ); - cancel_delayed_work(&clp->cl_renewd); - schedule_delayed_work(&clp->cl_renewd, timeout); - spin_unlock(&clp->cl_lock); nfs_expire_unreferenced_delegations(clp); out: dprintk("%s: done\n", __func__); -- 1.6.2.5 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/4] nfs41: fix race between umount and renewd sequence operations 2010-01-27 2:43 ` [PATCH 2/4] nfs4: prevent backlogging of renewd requests Alexandros Batsakis @ 2010-01-27 2:43 ` Alexandros Batsakis 2010-01-27 2:43 ` [PATCH 4/4] nfs4: fix race between umount and renew operations Alexandros Batsakis 2010-01-28 20:47 ` [PATCH 3/4] nfs41: fix race between umount and renewd sequence operations Trond Myklebust 0 siblings, 2 replies; 6+ messages in thread From: Alexandros Batsakis @ 2010-01-27 2:43 UTC (permalink / raw) To: linux-nfs; +Cc: trond, Alexandros Batsakis Signed-off-by: Alexandros Batsakis <batsakis@netapp.com> --- fs/nfs/nfs4proc.c | 18 ++++++++++++++++-- 1 files changed, 16 insertions(+), 2 deletions(-) diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 9fc99e9..cd523df 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -5041,6 +5041,8 @@ void nfs41_sequence_call_done(struct rpc_task *task, void *data) if (task->tk_status < 0) { dprintk("%s ERROR %d\n", __func__, task->tk_status); + if (atomic_read(&clp->cl_count) == 1) + return; if (_nfs4_async_handle_error(task, NULL, clp, NULL) == -EAGAIN) { @@ -5050,7 +5052,8 @@ void nfs41_sequence_call_done(struct rpc_task *task, void *data) } dprintk("%s rpc_cred %p\n", __func__, task->tk_msg.rpc_cred); - nfs4_schedule_state_renewal(clp); + if (atomic_read(&clp->cl_count) > 1) + nfs4_schedule_state_renewal(clp); kfree(task->tk_msg.rpc_argp); kfree(task->tk_msg.rpc_resp); @@ -5073,9 +5076,15 @@ static void nfs41_sequence_prepare(struct rpc_task *task, void *data) rpc_call_start(task); } +static void nfs41_sequence_release(void *calldata) +{ + nfs_put_client((struct nfs_client *) calldata); +} + static const struct rpc_call_ops nfs41_sequence_ops = { .rpc_call_done = nfs41_sequence_call_done, .rpc_call_prepare = nfs41_sequence_prepare, + .rpc_release = nfs41_sequence_release, }; static int nfs41_proc_async_sequence(struct nfs_client *clp, @@ -5088,11 +5097,16 @@ static int nfs41_proc_async_sequence(struct nfs_client *clp, .rpc_cred = cred, }; + if (!atomic_inc_not_zero(&clp->cl_count)) + return -EIO; args = kzalloc(sizeof(*args), GFP_KERNEL); - if (!args) + if (!args) { + nfs_put_client(clp); return -ENOMEM; + } res = kzalloc(sizeof(*res), GFP_KERNEL); if (!res) { + nfs_put_client(clp); kfree(args); return -ENOMEM; } -- 1.6.2.5 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 4/4] nfs4: fix race between umount and renew operations 2010-01-27 2:43 ` [PATCH 3/4] nfs41: fix race between umount and renewd sequence operations Alexandros Batsakis @ 2010-01-27 2:43 ` Alexandros Batsakis 2010-01-28 20:48 ` Trond Myklebust 2010-01-28 20:47 ` [PATCH 3/4] nfs41: fix race between umount and renewd sequence operations Trond Myklebust 1 sibling, 1 reply; 6+ messages in thread From: Alexandros Batsakis @ 2010-01-27 2:43 UTC (permalink / raw) To: linux-nfs; +Cc: trond, Alexandros Batsakis Signed-off-by: Alexandros Batsakis <batsakis@netapp.com> --- fs/nfs/nfs4proc.c | 12 +++++++++++- 1 files changed, 11 insertions(+), 1 deletions(-) diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index cd523df..cc8f059 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -3162,11 +3162,18 @@ static void nfs4_renew_done(struct rpc_task *task, void *data) if (time_before(clp->cl_last_renewal,timestamp)) clp->cl_last_renewal = timestamp; spin_unlock(&clp->cl_lock); - nfs4_schedule_state_renewal(clp); + if (atomic_read(clp->cl_count) > 1) + nfs4_schedule_state_renewal(clp); +} + +static void nfs4_renew_release(void *calldata) +{ + nfs_put_client((struct nfs_client *) calldata); } static const struct rpc_call_ops nfs4_renew_ops = { .rpc_call_done = nfs4_renew_done, + .rpc_release = nfs4_renew_release, }; int nfs4_proc_async_renew(struct nfs_client *clp, struct rpc_cred *cred) @@ -3177,6 +3184,9 @@ int nfs4_proc_async_renew(struct nfs_client *clp, struct rpc_cred *cred) .rpc_cred = cred, }; + if (!atomic_inc_not_zero(clp->cl_count)) + return -EIO; + return rpc_call_async(clp->cl_rpcclient, &msg, RPC_TASK_SOFT, &nfs4_renew_ops, (void *)jiffies); } -- 1.6.2.5 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 4/4] nfs4: fix race between umount and renew operations 2010-01-27 2:43 ` [PATCH 4/4] nfs4: fix race between umount and renew operations Alexandros Batsakis @ 2010-01-28 20:48 ` Trond Myklebust 0 siblings, 0 replies; 6+ messages in thread From: Trond Myklebust @ 2010-01-28 20:48 UTC (permalink / raw) To: Alexandros Batsakis; +Cc: linux-nfs, trond On Tue, 2010-01-26 at 18:43 -0800, Alexandros Batsakis wrote: > Signed-off-by: Alexandros Batsakis <batsakis@netapp.com> > --- > fs/nfs/nfs4proc.c | 12 +++++++++++- > 1 files changed, 11 insertions(+), 1 deletions(-) > > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c > index cd523df..cc8f059 100644 > --- a/fs/nfs/nfs4proc.c > +++ b/fs/nfs/nfs4proc.c > @@ -3162,11 +3162,18 @@ static void nfs4_renew_done(struct rpc_task *task, void *data) > if (time_before(clp->cl_last_renewal,timestamp)) > clp->cl_last_renewal = timestamp; > spin_unlock(&clp->cl_lock); > - nfs4_schedule_state_renewal(clp); > + if (atomic_read(clp->cl_count) > 1) > + nfs4_schedule_state_renewal(clp); > +} > + > +static void nfs4_renew_release(void *calldata) > +{ > + nfs_put_client((struct nfs_client *) calldata); > } > > static const struct rpc_call_ops nfs4_renew_ops = { > .rpc_call_done = nfs4_renew_done, > + .rpc_release = nfs4_renew_release, > }; > > int nfs4_proc_async_renew(struct nfs_client *clp, struct rpc_cred *cred) > @@ -3177,6 +3184,9 @@ int nfs4_proc_async_renew(struct nfs_client *clp, struct rpc_cred *cred) > .rpc_cred = cred, > }; > > + if (!atomic_inc_not_zero(clp->cl_count)) > + return -EIO; > + > return rpc_call_async(clp->cl_rpcclient, &msg, RPC_TASK_SOFT, > &nfs4_renew_ops, (void *)jiffies); > } Thanks! This looks good. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 3/4] nfs41: fix race between umount and renewd sequence operations 2010-01-27 2:43 ` [PATCH 3/4] nfs41: fix race between umount and renewd sequence operations Alexandros Batsakis 2010-01-27 2:43 ` [PATCH 4/4] nfs4: fix race between umount and renew operations Alexandros Batsakis @ 2010-01-28 20:47 ` Trond Myklebust 1 sibling, 0 replies; 6+ messages in thread From: Trond Myklebust @ 2010-01-28 20:47 UTC (permalink / raw) To: Alexandros Batsakis; +Cc: linux-nfs, trond On Tue, 2010-01-26 at 18:43 -0800, Alexandros Batsakis wrote: > Signed-off-by: Alexandros Batsakis <batsakis@netapp.com> > --- > fs/nfs/nfs4proc.c | 18 ++++++++++++++++-- > 1 files changed, 16 insertions(+), 2 deletions(-) > > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c > index 9fc99e9..cd523df 100644 > --- a/fs/nfs/nfs4proc.c > +++ b/fs/nfs/nfs4proc.c > @@ -5041,6 +5041,8 @@ void nfs41_sequence_call_done(struct rpc_task *task, void *data) > > if (task->tk_status < 0) { > dprintk("%s ERROR %d\n", __func__, task->tk_status); > + if (atomic_read(&clp->cl_count) == 1) > + return; > > if (_nfs4_async_handle_error(task, NULL, clp, NULL) > == -EAGAIN) { > @@ -5050,7 +5052,8 @@ void nfs41_sequence_call_done(struct rpc_task *task, void *data) > } > dprintk("%s rpc_cred %p\n", __func__, task->tk_msg.rpc_cred); > > - nfs4_schedule_state_renewal(clp); > + if (atomic_read(&clp->cl_count) > 1) Why not just have a single if (atomic_read(&clp->cl_count) == 1) return; above > + nfs4_schedule_state_renewal(clp); > > kfree(task->tk_msg.rpc_argp); > kfree(task->tk_msg.rpc_resp); These belong in the 'sequence_release' function regardless. Otherwise you have a memory leak if rpc_call_async() failed, and also if your test for clp->cl_count == 1 in the error case above. > @@ -5073,9 +5076,15 @@ static void nfs41_sequence_prepare(struct rpc_task *task, void *data) > rpc_call_start(task); > } > > +static void nfs41_sequence_release(void *calldata) > +{ > + nfs_put_client((struct nfs_client *) calldata); > +} > + > static const struct rpc_call_ops nfs41_sequence_ops = { > .rpc_call_done = nfs41_sequence_call_done, > .rpc_call_prepare = nfs41_sequence_prepare, > + .rpc_release = nfs41_sequence_release, > }; > > static int nfs41_proc_async_sequence(struct nfs_client *clp, > @@ -5088,11 +5097,16 @@ static int nfs41_proc_async_sequence(struct nfs_client *clp, > .rpc_cred = cred, > }; > > + if (!atomic_inc_not_zero(&clp->cl_count)) > + return -EIO; > args = kzalloc(sizeof(*args), GFP_KERNEL); > - if (!args) > + if (!args) { > + nfs_put_client(clp); > return -ENOMEM; > + } > res = kzalloc(sizeof(*res), GFP_KERNEL); > if (!res) { > + nfs_put_client(clp); > kfree(args); > return -ENOMEM; > } ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2010-01-28 20:48 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-01-27 2:43 [PATCH 1/4] nfs: kill renewd before clearing the client session Alexandros Batsakis 2010-01-27 2:43 ` [PATCH 2/4] nfs4: prevent backlogging of renewd requests Alexandros Batsakis 2010-01-27 2:43 ` [PATCH 3/4] nfs41: fix race between umount and renewd sequence operations Alexandros Batsakis 2010-01-27 2:43 ` [PATCH 4/4] nfs4: fix race between umount and renew operations Alexandros Batsakis 2010-01-28 20:48 ` Trond Myklebust 2010-01-28 20:47 ` [PATCH 3/4] nfs41: fix race between umount and renewd sequence operations Trond Myklebust
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox