* [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 an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.