[pnfs] [PATCH 9/9] pnfs client prevent race in sequence slot

[William A. (Andy) Adamson]

Here is what is happening on the failure of the Connectathon lock test #12.

The client produces a SEQUENCE:PUTFH:GETATTR (nfs4_proc_getattr) compond
followed by a SEQUENCE:PUTFH:LOCKU (nfs4_proc_unlck).

Wireshark shows the nfs4_proc_getattr as succeeding, and the nfs4_proc_unlck
as failing with BAD_SESSION.

The nfs4_proc_getattr rpc task catches a signal (from the test) and returns
-ERESTARTSYS, having not called decode, which results in the
nfs4_proc_getattr local variable nfs41_sequence_res.status being left unset
- so whatever garbage is in the un-initalized nfs41_sequence_res.status is
what is passed to nfs41_sequence_done. This happens to be non-zero, and is
interpreted by nfs41_proc_sequence_done() as an error, which means that the
sequence number for the slot is decremented, and the next rpc
(nfs4_proc_unlk) will send the same sequence number and get an error
(BAD_SESSION on our server).

The client does not know if the rpc succeeded, because it never decoded the
reply. But, in order to process the SEQUENCE operation on the
nfs4_proc_getattr and not get out of sync with the server, the client needs
to know the status of the SEQUENCE operation sent by the server.

Suggestions?

-->Andy


On 9/26/07, Trond Myklebust <trond.myklebust at fys.uio.no> wrote:
>
> On Wed, 2007-09-26 at 08:45 -0400, andros at umich.edu wrote:
> > From: Andy Adamson <andros at umich.edu>
> >
> > Move the lock to cover reading the sequence list in rpc_sequence
> > which allowed multiple rpc's to be in flight with the same slot
> > sequence id.
>
> Sorry. This patch is just wrong. There is no reason to take the spinlock
> when just testing for equality here.
>
> Trond
>
> > Signed-off by: Andy Adamson<andros at umich.edu>
> > ---
> >  fs/nfs/nfs4state.c |    4 ++--
> >  1 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
> > index 4963454..cbc08af 100644
> > --- a/fs/nfs/nfs4state.c
> > +++ b/fs/nfs/nfs4state.c
> > @@ -773,15 +773,15 @@ int nfs_wait_on_sequence(struct nfs_seqid *seqid,
> struct rpc_task *task)
> >       struct rpc_sequence *sequence = seqid->sequence->sequence;
> >       int status = 0;
> >
> > +     spin_lock(&sequence->lock);
> >       if (sequence->list.next == &seqid->list)
> >               goto out;
> > -     spin_lock(&sequence->lock);
> >       if (sequence->list.next != &seqid->list) {
> >               rpc_sleep_on(&sequence->wait, task, NULL, NULL);
> >               status = -EAGAIN;
> >       }
> > -     spin_unlock(&sequence->lock);
> >  out:
> > +     spin_unlock(&sequence->lock);
> >       return status;
> >  }
> >
>
> _______________________________________________
> pNFS mailing list
> pNFS at linux-nfs.org
> http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://linux-nfs.org/pipermail/pnfs/attachments/20070926/a10b5cb1/attachment.htm