[pnfs] Patch to fix ^C crash on mount

[Iyer, Rahul]

Hi Andy,
You mean check in the renew_lease function instead of the individual
functions?
Thanks
Regards
Rahul
 

> -----Original Message-----
> From: William A. (Andy) Adamson [mailto:andros at citi.umich.edu] 
> Sent: Thursday, July 26, 2007 2:08 PM
> To: Iyer, Rahul
> Cc: pnfs at linux-nfs.org
> Subject: Re: [pnfs] Patch to fix ^C crash on mount
> 
> Hi Rahul
> 
> You should add the check inside of renew_lease() in the 
> following patch.
> 
> 0002-Renew-lease-only-on-sequence.patch.txt
> 
> static void renew_lease(const struct nfs_server *server, 
> unsigned long timestamp) {
>         struct nfs4_client *clp = server->nfs4_state;
> 
>         if (server->rpc_ops->setup_sequence)
>                      return
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^new code
> 
>         spin_lock(&clp->cl_lock);
>         if (time_before(clp->cl_last_renewal,timestamp))
>                 clp->cl_last_renewal = timestamp;
>         spin_unlock(&clp->cl_lock);
> }
> 
> -->Andy
> 
> 
> 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 >
> 	>       >
> 	>       >
> 	>       >
> 	>       >
> 	>       >
> 	>
> 	>
> 	>
> 	>
> 	>
> 	
> 	
> 
> 
>