[pnfs] Patch to fix ^C crash on mount

William A. (Andy) Adamson andros at citi.umich.edu
Thu Jul 26 16:54:36 EDT 2007


Hi Rahul

Don't you have it backwards in this patch? Comment below...

-->Andy

*0001-Add-code-to-encode-a-0-instead-of-the-sequence-numbe.patch.txt

*From b2b5de7379a051176943cd3f97ba5442d6bd7abf Mon Sep 17 00:00:00 2001
Message-Id: <
b2b5de7379a051176943cd3f97ba5442d6bd7abf.1185241829.git.iyer at netapp.com>
From: Rahul Iyer <iyer at netapp.com>
Date: Wed, 18 Jul 2007 14:50:01 -0400
Subject: [PATCH 1/2] Add code to encode a 0 instead of the sequence number
in the open op.

Check whether we're using v4.0 or v4.1 and encode 0 if it's 4.1, the
sequence number otherwise.

Signed-off-by: Rahul Iyer <iyer at netapp.com>
---
 fs/nfs/nfs4xdr.c |    7 ++++++-
 1 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index 2ea38df..2e5a8a7 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -1067,7 +1067,12 @@ static inline void encode_openhdr(struct xdr_stream
*xdr, const struct nfs_opena
  */
        RESERVE_SPACE(8);
        WRITE32(OP_OPEN);
-       WRITE32(arg->seqid->sequence->counter);
+       if (arg->server->rpc_ops->setup_sequence)
                                                ^^^^^^^^^^^^^^^
                                               doesn't this mean v4.1 and
therefore should encode seqid??
+               WRITE32(0);
+       else {
+               dprintk("encoding seqid!!\n");
+               WRITE32(arg->seqid->sequence->counter);
+       }
        encode_share_access(xdr, arg->open_flags);
        RESERVE_SPACE(16);
        WRITE64(arg->clientid);
--
1.5.2


On 7/23/07, Iyer, Rahul <Rahul.Iyer at netapp.com> wrote:
>
> Hi,
> The attached patches fix the latter two problems mentioned in my email
> below.
> Regards
> Rahul
>
>
> > -----Original Message-----
> > From: William A. (Andy) Adamson [mailto:andros at citi.umich.edu]
> > Sent: Wednesday, July 18, 2007 10:23 AM
> > To: Iyer, Rahul
> > Cc: pnfs at linux-nfs.org
> > Subject: Re: [pnfs] Patch to fix ^C crash on mount
> >
> > there are a bunch of exchange_id cases (similar to the
> > setclientid cases) that the server does not currently check.
> > these are enumerated in the pynfs exchange_id tests.
> >
> > we should seriously review the exchange_id and setclientid
> > cases, and share code where possible.
> >
> > -->Andy
> >
> >
> > On 7/18/07, Iyer, Rahul <Rahul.Iyer at netapp.com> wrote:
> >
> >       Hi guys,
> >       This patch seems to have the side effect of eliminating
> > the umount crash
> >       as well. I'm still investigating as to why it worked.
> > Either ways, the
> >       client is now more stable than before. There are still
> > a few issues:
> >       - The open sequence counter seems to be encoded as is.
> > So occasionally,
> >       it winds up with a seqid of 1 and the server rejects it with
> >       NFSERR_INVAL.
> >       - Read and write seem to update the lease value. This
> > is not true in
> >       NFSv4.1 IIRC. The current code does this and has the
> > unfortunate effect
> >       that if pNFS reads or writes run long (> lease time),
> > then the client
> >       would think the lease is up to date and not send
> > SEQUENCE ops to the
> >       MDS, resulting in NFSERR_BADSESSION/NFSERR_STALE_CLIENTID.
> >       - The server code has a bug in EXCHANGE_ID which
> > results in the long
> >       strings of NFSERR_CLID_INUSE errors. The current code does:
> >
> >               conf = find_confirmed_client_by_str(dname, strhashval);
> >               if (conf) {
> >                       if (!cmp_creds(&conf->cl_cred,
> > &rqstp->rq_cred) ||
> >       (ip_addr != conf->cl_addr)) {
> >                               /* Client collision: send
> > nfserr_clid_inuse */
> >                               goto out;
> >                       }
> >
> >                       if (cmp_verf(&verf, &conf->cl_verifier)) {
> >
> > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> >                               /* Client reboot: destroy old state */
> >                               expire_client(conf);
> >                               goto out_new;
> >                       }
> >                       /* router replay */
> >                       goto out;
> >               }
> >
> >
> >       In the highlighted line, it should be !cmp_verf because
> > cmp_verf returns
> >       true if the verifiers are the same.
> >
> >       I'll work on these and send out the patches.
> >       Regards
> >       Rahul
> >
> >
> >
> >
> >       > -----Original Message-----
> >       > From: William A. (Andy) Adamson
> > [mailto:andros at citi.umich.edu ]
> >       > Sent: Wednesday, July 18, 2007 9:47 AM
> >       > To: Iyer, Rahul
> >       > Cc: pnfs at linux-nfs.org
> >       > Subject: Re: [pnfs] Patch to fix ^C crash on mount
> >       >
> >       > ok. applied to 4.1-sessions branch and merged with master
> >       >
> >       > -->Andy
> >       >
> >       >
> >       > On 7/17/07, Iyer, Rahul <Rahul.Iyer at netapp.com> wrote:
> >       >
> >       >       Hi,
> >       >       If the mount hangs for some reason, and you hit ^C, the
> >       > client crashes as it tries to destroy a mempool and a  slab
> >       > that could be NULL. This patch check for the values before
> >       > destroying them.
> >       >       Regards
> >       >       Rahul
> >       >
> >       >
> >       >       _______________________________________________
> >       >       pNFS mailing list
> >       >       pNFS at linux-nfs.org
> >       >       http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs
> >       > <http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs >
> >       >
> >       >
> >       >
> >       >
> >       >
> >
> >
> >
> >
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://linux-nfs.org/pipermail/pnfs/attachments/20070726/5e007cd8/attachment.htm 


More information about the pNFS mailing list