[pnfs] Patch to fix ^C crash on mount
Iyer, Rahul
Rahul.Iyer at netapp.com
Thu Jul 26 17:46:59 EDT 2007
> -----Original Message-----
> From: William A. (Andy) Adamson [mailto:andros at citi.umich.edu]
> Sent: Thursday, July 26, 2007 1:55 PM
> To: Iyer, Rahul
> Cc: pnfs at linux-nfs.org
> Subject: Re: [pnfs] Patch to fix ^C crash on mount
>
> 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@
> 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
>
The setup_sequence pointer is always non NULL for v4.1. The above if
says that if it is non NULL, then encode 0, else encode the actual
value...
Regards
Rahul
>
>
> 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 <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
> <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 >
> > >
> > >
> > >
> > >
> > >
> >
> >
> >
> >
> >
>
>
>
>
>
More information about the pNFS
mailing list