Re: [PATCH 1/2] NFS: Add sequence_priviliged_ops for nfs4_proc_sequence()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, 2012-11-12 at 15:22 -0500, Andy Adamson wrote:
> On Mon, Nov 12, 2012 at 1:35 PM,  <bjschuma@xxxxxxxxxx> wrote:
> > From: Bryan Schumaker <bjschuma@xxxxxxxxxx>
> >
> > During recovery the NFS4_SESSION_DRAINING flag might be set on the
> > client structure.  This can cause lease renewal to abort when

Not lease renewal. It is lease verification (i.e. checking that we have
a valid lease and session) that will deadlock.

> > nfs41_setup_sequence sees that we are doing recovery.  As a result, the
> > client never recovers and all activity with the NFS server halts.
> 
> 
> When does this happen? Say the session is draining, and an RPC returns
> from one of the compounds such as nfs_open, nfs_locku etc whose
> rpc_call_done routine calls renew_lease after freeing it's slot.  Like
> all calls on a draining session, the renew_lease sleeps on the session
> slot_tbl_waitq - is this what you mean by "causes lease renewal to
> abort"? How does this cause the client to never recover?
> 
> The only other call to renew_lease is from the state manager itself
> which runs one function at a time, and will not call renew_lease until
> the recovery is over....
> 
> What am I missing....?

nfs4_check_lease() is run exclusively by the state manager thread in
order to check that the clientid (and session id on NFSv4.1) are valid.
It will deadlock the state manager thread if NFS4_SESSION_DRAINING is
already set.

> -->Andy
> 
> 
> >
> > Signed-off-by: Bryan Schumaker <bjschuma@xxxxxxxxxx>
> > ---
> >  fs/nfs/nfs4proc.c | 21 +++++++++++++++++----
> >  1 file changed, 17 insertions(+), 4 deletions(-)
> >
> > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> > index 5eec442..537181c 100644
> > --- a/fs/nfs/nfs4proc.c
> > +++ b/fs/nfs/nfs4proc.c
> > @@ -6138,13 +6138,26 @@ static void nfs41_sequence_prepare(struct rpc_task *task, void *data)
> >         rpc_call_start(task);
> >  }
> >
> > +static void nfs41_sequence_prepare_privileged(struct rpc_task *task, void *data)
> > +{
> > +       rpc_task_set_priority(task, RPC_PRIORITY_PRIVILEGED);
> > +       nfs41_sequence_prepare(task, data);
> > +}
> > +
> >  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 struct rpc_task *_nfs41_proc_sequence(struct nfs_client *clp, struct rpc_cred *cred)
> > +static const struct rpc_call_ops nfs41_sequence_privileged_ops = {
> > +       .rpc_call_done = nfs41_sequence_call_done,
> > +       .rpc_call_prepare = nfs41_sequence_prepare_privileged,
> > +       .rpc_release = nfs41_sequence_release,
> > +};
> > +
> > +static struct rpc_task *_nfs41_proc_sequence(struct nfs_client *clp, struct rpc_cred *cred,
> > +                                            const struct rpc_call_ops *seq_ops)
> >  {
> >         struct nfs4_sequence_data *calldata;
> >         struct rpc_message msg = {
> > @@ -6154,7 +6167,7 @@ static struct rpc_task *_nfs41_proc_sequence(struct nfs_client *clp, struct rpc_
> >         struct rpc_task_setup task_setup_data = {
> >                 .rpc_client = clp->cl_rpcclient,
> >                 .rpc_message = &msg,
> > -               .callback_ops = &nfs41_sequence_ops,
> > +               .callback_ops = seq_ops,
> >                 .flags = RPC_TASK_ASYNC | RPC_TASK_SOFT,
> >         };
> >
> > @@ -6181,7 +6194,7 @@ static int nfs41_proc_async_sequence(struct nfs_client *clp, struct rpc_cred *cr
> >
> >         if ((renew_flags & NFS4_RENEW_TIMEOUT) == 0)
> >                 return 0;
> > -       task = _nfs41_proc_sequence(clp, cred);
> > +       task = _nfs41_proc_sequence(clp, cred, &nfs41_sequence_ops);
> >         if (IS_ERR(task))
> >                 ret = PTR_ERR(task);
> >         else
> > @@ -6195,7 +6208,7 @@ static int nfs4_proc_sequence(struct nfs_client *clp, struct rpc_cred *cred)
> >         struct rpc_task *task;
> >         int ret;
> >
> > -       task = _nfs41_proc_sequence(clp, cred);
> > +       task = _nfs41_proc_sequence(clp, cred, &nfs41_sequence_privileged_ops);
> >         if (IS_ERR(task)) {
> >                 ret = PTR_ERR(task);
> >                 goto out;
> > --
> > 1.8.0
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@xxxxxxxxxx
www.netapp.com
��.n��������+%������w��{.n�����{��w���jg��������ݢj����G�������j:+v���w�m������w�������h�����٥



[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux