[pnfs] A Few bug fixes

Iyer, Rahul Rahul.Iyer at netapp.com
Fri Jun 1 13:09:06 EDT 2007


Hi Andy,
Will do. I'll send it out by Tuesday or so.
Regards
Rahul
 

> -----Original Message-----
> From: William A. (Andy) Adamson [mailto:andros at citi.umich.edu] 
> Sent: Friday, June 01, 2007 6:22 AM
> To: Iyer, Rahul
> Cc: pnfs at linux-nfs.org
> Subject: Re: [pnfs] A Few bug fixes
> 
> Hi Rahul
> 
> Would you please re-submit these bug-fix patches?
> 
> Thanks
> 
> -->Andy
> 
> 
> On 5/31/07, William A. (Andy) Adamson <andros at citi.umich.edu> wrote:
> 
> 	comment in line...
> 	
> 	-->Andy
> 	
> 	
> 	On 5/31/07, Benny Halevy < bhalevy at panasas.com 
> <mailto:bhalevy at panasas.com> > wrote:
> 
> 		Rahul, some comments on patches 2-5:
> 		
> 		patch 2:
> 		diff --git a/net/sunrpc/xprtsock.c 
> b/net/sunrpc/xprtsock.c
> 		index 94cc6dd..8c24516 100644
> 		--- a/net/sunrpc/xprtsock.c
> 		+++ b/net/sunrpc/xprtsock.c
> 		@@ -691,7 +691,14 @@ static inline void 
> xs_tcp_read_request(struct rpc_xprt *xprt, skb_reader_t *desc 
> 		        calldir = ntohl(xprt->tcp_calldir);
> 		        if (calldir == RPC_CALL) {
> 		
> 		-               req = xprt_alloc_bc_request(xprt);
> 		+               if (xprt->bc_mempool)
> 		+                       req = 
> xprt_alloc_bc_request(xprt);
> 		+               else {
> 		+                       /*
> 		+                        * Received callback 
> when none is expected
> 		+                        */
> 		+                       req = NULL;
> 		+               }
> 		
> 		                if (!req) {
> 		                        printk(KERN_NOTICE 
> "Couldn't get rpc_rqst for the allback! Dropping callback...\n");
> 		
> 		Please print a different warning for the two 
> cases (in one bc_mempool is NULL
> 		the other is xprt_alloc_bc_request returning 
> NULL) and fix "allback" to "callback" 
> 		
> 		Eventually we can drop the callback safely in 
> the first case since it was sent
> 		when there's no back channel but in the latter 
> case we will need to handle the
> 		error differently as we discussed.
> 		
> 		patch 3:@@ -1245,18 +1245,24 @@ static int 
> nfs4_fill_super(struct super_block *sb, struct nfs4_mount_data *data, 
> 		                }
> 		        }
> 		
> 		+       /*
> 		+        * This is safe to do because all 
> nfs4_create_client uses is the
> 		+        * version number. The version number 
> for all minorversions is 4
> 		+        */
> 		
> 		I'd put this comment only in the commit message 
> rather than in the code since
> 		the code is straight forward.
> 		
> 		+       server->rpc_ops = 
> nfsv4_minorversion_clientops[NFSV4_MAX_MINORVERSION];
> 		+       server->client = 
> nfs4_create_client(server, &timeparms, data->proto, authflavour);
> 		+       if (IS_ERR(server->client)) {
> 		+               err = PTR_ERR(server->client);
> 		+                       dprintk("%s: cannot 
> create RPC client. Error = %d\n",
> 		+                                       
> __FUNCTION__, err);
> 		+                       goto out_fail;
> 		
> 		I see that this was cut and pasted from below 
> but there's an
> 		extra tab indent in the two statements above 
> which we can fix in process.
> 		
> 		+       }
> 		+
> 		
> 		@@ -1282,8 +1288,6 @@ static int 
> nfs4_fill_super(struct super_block *sb, struct nfs4_mount_data *data, 
> 		                                 * Shut down 
> the callback service
> 		                                 */
> 		                                
> nfs_callback_down(i, server->client->cl_xprt);
> 		-
> 		-                                
> rpc_shutdown_client(server->client);
> 		                         }
> 		                        else
> 		                                break;
> 		
> 		Who destroys the nfs4 client (and rpc client) 
> if setup session
> 		failed for all minor versions?
> 		
> 		patch 4:
> 		diff --git a/fs/nfs/nfs4renewd.c b/fs/nfs/nfs4renewd.c 
> 		index 14a918b..aff054b 100644
> 		--- a/fs/nfs/nfs4renewd.c
> 		+++ b/fs/nfs/nfs4renewd.c
> 		@@ -120,6 +120,7 @@ out:
> 		no_cred:
> 		        printk(KERN_EMERG "couldn't get renew 
> creds!\n");
> 		        set_bit(NFS4CLNT_LEASE_EXPIRED, 
> &clp->cl_state); 
> 		+       spin_unlock(&clp->cl_lock);
> 		        nfs_expire_all_delegations(clp);
> 		        goto out;
> 		}
> 		
> 		1. I'd move the printk call after the spin_unlock.
> 		
> 		2. I see the need for this unlock, good catch!. 
>  When looking for 
> 		   it I realized that the indentation in this 
> function is quite
> 		   confusing.  How about also fixing it:
> 		
> 		                        if (cred == NULL) {
> 		                                if 
> (IS_ERR(clp->cl_rpcclient))
> 		                                        goto no_cred;
> 		                                cred = 
> rpcauth_lookupcred(clp->cl_rpcclient->cl_auth, 0);
> 		                                if (IS_ERR(cred))
> 		                                        goto no_cred;
> 		-               }
> 		-               spin_unlock(&clp->cl_lock);
> 		+                       }
> 		+                       spin_unlock(&clp->cl_lock);
> 		                        if 
> (nfs4_proc_async_sequence(clp, cred))
> 		                                
> printk(KERN_NOTICE "Could not allocate memory \
> 		                                                
> for SEQUENCE call!\n");
> 		                }
> 		
> 		
> 		patch 5 looks good
> 		
> 		Benny
> 		
> 		Iyer, Rahul wrote:
> 		> Forgot the patches
> 		> Regards
> 		> Rahul
> 		>
> 		>
> 		>> -----Original Message----- 
> 		>> From: Iyer, Rahul
> 		>> Sent: Friday, May 25, 2007 5:54 PM
> 		>> To: pnfs at linux-nfs.org 
> 		>> Subject: [pnfs] A Few bug fixes
> 		>>
> 		>> Hi All, 
> 		>> Here are a few Bug fixes I have for the 
> client. Today, I got
> 		>> the latest tree and could mount the server. 
> These patches
> 		>> apply to the latest tree and do not break 
> anything. I could
> 		>> run connectathon on them multiple times. 
> 		>>
> 		>> Thanks
> 		>> Regards
> 		>> Rahul
> 		>>
> 		>>
> 		>> Summary
> 		>> -------
> 		>>  fs/nfs/nfs4renewd.c           |    1 +
> 		>>  fs/nfs/nfs4state.c            |    2 +-
> 		>>  fs/nfs/super.c                |   22 
> +++++++++++--------
> 		>>  include/linux/sunrpc/xprt.h   |    3 ++
> 		>>  net/sunrpc/backchannel_rqst.c |   45
> 		>> +++++++++++++++++++++++++++++++++++++++++
> 		>>  net/sunrpc/xprtsock.c         |    9 +++++++- 
> 		>>  6 files changed, 71 insertions(+), 11 deletions(-)
> 		>>
> 		>> Patch Description
> 		>> -----------------
> 		>>
> 		>> [PATCH 1/5] Synchronize callback request 
> allocation with disconnection 
> 		>>
> 		>> Currently, there is no synchronization 
> between disconnection
> 		>> of the callback channel and allocation of 
> callback requests.
> 		>> This patch ensures that:
> 		>> 1. If the channel is being disconnected, 
> requests are not 
> 		>> allocated 2. If requests are allocated, the 
> channel waits for
> 		>> the requests to be
> 		>>    deallocated before disconnecting
> 		>>
> 		>> ----
> 		>> [PATCH 2/5] Allocate a callback request only 
> if you have a mempool 
> 		>>
> 		>> This patch adds a check for xprt->bc_mempool 
> != NULL in
> 		>> xs_tcp_read_request.
> 		>> This is for cases when you may receive a 
> packet with the call
> 		>> header set. If you're not expecting 
> callbacks on the channel 
> 		>> (e.g NFSv4.0), the code will panic
> 		>>
> 		>> ----
> 		>> [PATCH 3/5] Avoid creating nfs4_client on 
> every minorversion
> 		>> iteration.
> 		>>
> 		>> This patch creates an nfs4_client structure 
> once and then 
> 		>> uses it for the subsequent minorversion 
> iterations until one is found.
> 		>>
> 		>> As a by product, this also fixes a bug in 
> the refcounting of
> 		>> the nfs4_client struct. Since 
> nfs4_create_client is called to 
> 		>> create the client and rpc_shutdown_client is 
> called to shut
> 		>> it down, there is a leak in the cl_users field of the
> 		>> nfs4_client struct. As a result if an 
> NFSv4.1 mount fails and
> 		>> it falls back to v4.0, on unmount, cl_users isn't
> 		>> decremented. As a result, the callback 
> server on the client
> 		>> isn't killed.
> 		>>
> 		>> ----
> 		>> [PATCH 4/5] Release cl_lock when unable to 
> find renew creds 
> 		>> in nfs4_renew_state
> 		>>
> 		>> If renew creds could not be obtained in the 
> NFSv4 case, the
> 		>> spin lock on
> 		>> clp->cl_lock wasn't dropped. This resuts in 
> a soft lockup.
> 		>>
> 		>> ----
> 		>> [PATCH 5/5] Move the call to 
> nfs_callback_down to nfs4_put_client.
> 		>>
> 		>> The callto nfs_callback_down is currently in
> 		>> nfs4_free_client. Move this to 
> nfs4_put_client because this 
> 		>> is where it "fits".
> 		>>
> 		>> ----
> 		>> _______________________________________________
> 		>> pNFS mailing list
> 		>> pNFS at linux-nfs.org <mailto:pNFS at linux-nfs.org> 
> 		>> http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs
> 		>>
> 		>> 
> 		>> 
> --------------------------------------------------------------
> ---------- 
> 		>>
> 		>> From 
> bc7afef1f3d5630884c24aa693df07d5e1c88c5a Mon Sep 17 00:00:00 2001
> 		>> Message-Id: < 
> bc7afef1f3d5630884c24aa693df07d5e1c88c5a.1180138977.git.iyer at n
> etapp.com 
> <mailto:bc7afef1f3d5630884c24aa693df07d5e1c88c5a.1180138977.gi
> t.iyer at netapp.com> >
> 		>> From: Rahul Iyer <iyer at netapp.com>
> 		>> Date: Fri, 25 May 2007 12:32:44 -0700
> 		>> Subject: [PATCH 1/5] Synchronize callback 
> request allocation with disconnection 
> 		>>
> 		>> Currently, there is no synchronization 
> between disconnection of the callback
> 		>> channel and allocation of callback requests. 
> This patch ensures that:
> 		>> 1. If the channel is being disconnected, 
> requests are not allocated 
> 		>> 2. If requests are allocated, the channel 
> waits for the requests to be
> 		>>    deallocated before disconnecting
> 		>>
> 		>> Signed-off-by: Rahul Iyer < iyer at netapp.com 
> <mailto:iyer at netapp.com> >
> 		>> ---
> 		>>  include/linux/sunrpc/xprt.h   |    3 ++
> 		>>  net/sunrpc/backchannel_rqst.c |   45 
> +++++++++++++++++++++++++++++++++++++++++
> 		>>  2 files changed, 48 insertions(+), 0 deletions(-) 
> 		>>
> 		>> diff --git a/include/linux/sunrpc/xprt.h 
> b/include/linux/sunrpc/xprt.h
> 		>> index dd8a5a8..361979f 100644
> 		>> --- a/include/linux/sunrpc/xprt.h
> 		>> +++ b/include/linux/sunrpc/xprt.h 
> 		>> @@ -196,6 +196,9 @@ struct rpc_xprt {
> 		>>      struct svc_serv *       serv;           
> /* The svc_serv to queue the callbacks onto */
> 		>>      kmem_cache_t *          bc_slab;        
> /* The slab for the callback request mempool */
> 		>>      mempool_t *             bc_mempool;     
> /* The mempool to allocate rpc_rqst structs from for the callbacks */
> 		>> +    unsigned char           destructing;    
> /* The callback service is being shut down. */
> 		>> +    unsigned int            bc_req_count;   
> /* Number of requests allocated */
> 		>> +    wait_queue_head_t       bc_shutdown_wq;
> 		>>  #endif
> 		>>
> 		>>      struct list_head        recv;
> 		>> diff --git a/net/sunrpc/backchannel_rqst.c 
> b/net/sunrpc/backchannel_rqst.c 
> 		>> index 9f740b1..68237f0 100644
> 		>> --- a/net/sunrpc/backchannel_rqst.c
> 		>> +++ b/net/sunrpc/backchannel_rqst.c
> 		>> @@ -58,6 +58,10 @@ int 
> xprt_setup_backchannel(struct rpc_xprt *xprt, unsigned int min_reqs) 
> 		>>              goto out_free;
> 		>>      }
> 		>>
> 		>> +    xprt->destructing = 0;
> 		>> +    xprt->bc_req_count = 0;
> 		>> +    init_waitqueue_head(&xprt->bc_shutdown_wq); 
> 		>> +
> 		>>      return 0;
> 		>>
> 		>>  out_free:
> 		>> @@ -70,12 +74,32 @@ 
> EXPORT_SYMBOL(xprt_setup_backchannel);
> 		>>
> 		>>  void xprt_destroy_backchannel(struct 
> rpc_xprt *xprt) 
> 		>>  {
> 		>> +    DEFINE_WAIT(wq);
> 		>> +
> 		>> +    prepare_to_wait(&xprt->bc_shutdown_wq, 
> &wq, TASK_UNINTERRUPTIBLE);
> 		>> +    spin_lock(&xprt->transport_lock);
> 		>> +    xprt->destructing = 1; 
> 		>> +
> 		>> +    if (xprt->bc_req_count) {
> 		>> +            spin_unlock(&xprt->transport_lock);
> 		>> +            schedule();
> 		>> +    }
> 		>> +    spin_unlock(&xprt->transport_lock); 
> 		>> +    finish_wait(&xprt->bc_shutdown_wq, &wq);
> 		>> +
> 		>>      mempool_destroy(xprt->bc_mempool);
> 		>> +    xprt->bc_mempool = NULL;
> 		>> +
> 		>> +    BUG_ON(!xprt->bc_slab); 
> 		>>      kmem_cache_destroy(xprt->bc_slab);
> 		>>  }
> 		>>
> 		>>  EXPORT_SYMBOL(xprt_destroy_backchannel);
> 		>>
> 		>> +/*
> 		>> + * Allocate a backchannel request. Must be 
> always called with the 
> 		>> + * xprt->transport_lock held
> 		>> + */
> 		>>  struct rpc_rqst 
> *xprt_alloc_bc_request(struct rpc_xprt *xprt)
> 		>>  {
> 		>>      struct rpc_rqst *req;
> 		>> @@ -84,11 +108,24 @@ struct rpc_rqst 
> *xprt_alloc_bc_request(struct rpc_xprt *xprt) 
> 		>>
> 		>>      BUG_ON(xprt->bc_mempool == NULL);
> 		>>
> 		>> +    if (xprt->destructing) {
> 
> 
> 
> 	Don't you need to take the xprt->transport_lock before 
> inspecting xprt->destructing?
> 	
> 	
> 	
> 	
> 
> 		>> +            /*
> 		>> +             * This callback channel is 
> being torn down. Do not honour this
> 		>> +             * request
> 		>> +             */
> 		>> +            goto out;
> 		>> +    }
> 		>> +
> 		>>      req = mempool_alloc(xprt->bc_mempool, 
> GFP_ATOMIC); 
> 		>>
> 		>>      if (!req)
> 		>>              goto out;
> 		>>
> 		>> +    /*
> 		>> +     * Increment the allocated request count
> 		>> +     */
> 		>> +    ++xprt->bc_req_count; 
> 
> 
> 
> 	also take the xprt->transport_lock before inc/dec bc_req_count?
> 	 
> 	
> 	
> 
> 		>> +
> 		>>      req->rq_xprt = xprt;
> 		>>      req->rq_xid = xprt->tcp_xid; 
> 		>>
> 		>> @@ -124,6 +161,7 @@ struct rpc_rqst 
> *xprt_alloc_bc_request(struct rpc_xprt *xprt)
> 		>>  out_free_page:
> 		>>      free_page((unsigned 
> long)xbufp->head[0].iov_base);
> 		>>  out_free: 
> 		>> +    --xprt->bc_req_count;
> 
> 
> 	nee to take the xprt->transport_lock to dec?
> 	 
> 	
> 	
> 
> 		>>      mempool_free(req, xprt->bc_mempool); 
> 		>>  out:
> 		>>      return NULL; 
> 		>> @@ -141,6 +179,13 @@ void 
> xprt_free_bc_request(struct rpc_rqst *req)
> 		>>      free_page((unsigned 
> long)xbufp->head[0].iov_base);
> 		>>
> 		>>      mempool_free(req, xprt->bc_mempool); 
> 		>> +
> 		>> +    BUG_ON(!xprt->bc_req_count);
> 		>> +
> 		>> +    spin_lock(&xprt->transport_lock);
> 		>> +    --xprt->bc_req_count;
> 		>> +    wake_up(&xprt->bc_shutdown_wq); 
> 		>> +    spin_unlock(&xprt->transport_lock);
> 		>>  }
> 		>>
> 		>>  #endif
> 		>>
> 		>> 
> --------------------------------------------------------------
> ----------
> 		>>
> 		>> From 
> e8f5e610582908efe478a32520e3b167469f64a2 Mon Sep 17 00:00:00 2001 
> 		>> Message-Id: 
> <e8f5e610582908efe478a32520e3b167469f64a2.1180138977.git.iyer@
> netapp.com >
> 		>> In-Reply-To: < 
> bc7afef1f3d5630884c24aa693df07d5e1c88c5a.1180138977.git.iyer at n
> etapp.com >
> 		>> References: < 
> bc7afef1f3d5630884c24aa693df07d5e1c88c5a.1180138977.git.iyer at n
> etapp.com 
> <mailto:bc7afef1f3d5630884c24aa693df07d5e1c88c5a.1180138977.gi
> t.iyer at netapp.com> >
> 		>> From: Rahul Iyer <iyer at netapp.com >
> 		>> Date: Fri, 25 May 2007 15:18:09 -0700
> 		>> Subject: [PATCH 2/5] Allocate a callback 
> request only if you have a mempool
> 		>>
> 		>> This patch adds a check for xprt->bc_mempool 
> != NULL in xs_tcp_read_request.
> 		>> This is for cases when you may receive a 
> packet with the call header set. If 
> 		>> you're not expecting callbacks on the 
> channel (e.g NFSv4.0), the code will
> 		>> panic
> 		>>
> 		>> Signed-off-by: Rahul Iyer < iyer at netapp.com 
> <mailto:iyer at netapp.com> >
> 		>> ---
> 		>>  net/sunrpc/xprtsock.c |    9 ++++++++-
> 		>>  1 files changed, 8 insertions(+), 1 deletions(-)
> 		>>
> 		>> diff --git a/net/sunrpc/xprtsock.c 
> b/net/sunrpc/xprtsock.c
> 		>> index 94cc6dd..8c24516 100644 
> 		>> --- a/net/sunrpc/xprtsock.c
> 		>> +++ b/net/sunrpc/xprtsock.c
> 		>> @@ -691,7 +691,14 @@ static inline void 
> xs_tcp_read_request(struct rpc_xprt *xprt, skb_reader_t *desc
> 		>>      calldir = ntohl(xprt->tcp_calldir); 
> 		>>      if (calldir == RPC_CALL) {
> 		>>
> 		>> -            req = xprt_alloc_bc_request(xprt);
> 		>> +            if (xprt->bc_mempool)
> 		>> +                    req = 
> xprt_alloc_bc_request(xprt);
> 		>> +            else {
> 		>> +                    /*
> 		>> +                     * Received callback 
> when none is expected
> 		>> +                     */
> 		>> +                    req = NULL;
> 		>> +            }
> 		>>
> 		>>              if (!req) {
> 		>>                      printk(KERN_NOTICE 
> "Couldn't get rpc_rqst for the allback! Dropping callback...\n");
> 		>>
> 		>> 
> --------------------------------------------------------------
> ----------
> 		>>
> 		>> From 
> 683c5ff48227c2a3f69abcf752ee08abb791808a Mon Sep 17 00:00:00 2001 
> 		>> Message-Id: 
> <683c5ff48227c2a3f69abcf752ee08abb791808a.1180138977.git.iyer@
> netapp.com >
> 		>> In-Reply-To: < 
> bc7afef1f3d5630884c24aa693df07d5e1c88c5a.1180138977.git.iyer at n
> etapp.com >
> 		>> References: < 
> bc7afef1f3d5630884c24aa693df07d5e1c88c5a.1180138977.git.iyer at n
> etapp.com 
> <mailto:bc7afef1f3d5630884c24aa693df07d5e1c88c5a.1180138977.gi
> t.iyer at netapp.com> >
> 		>> From: Rahul Iyer <iyer at netapp.com >
> 		>> Date: Fri, 25 May 2007 15:24:04 -0700
> 		>> Subject: [PATCH 3/5] Avoid creating 
> nfs4_client on every minorversion iteration.
> 		>>
> 		>> This patch creates an nfs4_client structure 
> once and then uses it for the
> 		>> subsequent minorversion iterations until one 
> is found. 
> 		>>
> 		>> As a by product, this also fixes a bug in 
> the refcounting of the nfs4_client
> 		>> struct. Since nfs4_create_client is called 
> to create the client and
> 		>> rpc_shutdown_client is called to shut it 
> down, there is a leak in the cl_users 
> 		>> field of the nfs4_client struct. As a result 
> if an NFSv4.1 mount fails and it
> 		>> falls back to v4.0, on unmount, cl_users 
> isn't decremented. As a result, the
> 		>> callback server on the client isn't killed. 
> 		>>
> 		>> Signed-off-by: Rahul Iyer <iyer at netapp.com>
> 		>> ---
> 		>>  fs/nfs/super.c |   22 +++++++++++++--------- 
> 		>>  1 files changed, 13 insertions(+), 9 deletions(-) 
> 		>>
> 		>> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> 		>> index fc18c8c..1721c01 100644
> 		>> --- a/fs/nfs/super.c
> 		>> +++ b/fs/nfs/super.c
> 		>> @@ -1245,18 +1245,24 @@ static int 
> nfs4_fill_super(struct super_block *sb, struct nfs4_mount_data *data, 
> 		>>              }
> 		>>      }
> 		>>
> 		>> +    /*
> 		>> +     * This is safe to do because all 
> nfs4_create_client uses is the
> 		>> +     * version number. The version number 
> for all minorversions is 4 
> 		>> +     */
> 		>> +    server->rpc_ops = 
> nfsv4_minorversion_clientops[NFSV4_MAX_MINORVERSION];
> 		>> +    server->client = 
> nfs4_create_client(server, &timeparms, data->proto, authflavour); 
> 		>> +    if (IS_ERR(server->client)) {
> 		>> +            err = PTR_ERR(server->client);
> 		>> +                    dprintk("%s: cannot 
> create RPC client. Error = %d\n",
> 		>> +                                    
> __FUNCTION__, err);
> 		>> +                    goto out_fail;
> 		>> +    }
> 		>> +
> 		>>          for (i = NFSV4_MAX_MINORVERSION; i 
> >= 0; --i) {
> 		>>                  nfs_version4 = 
> *nfs4_minorversions[i];
> 		>>                  nfs4_procedures = 
> nfs4_minorversion_procedures[i];
> 		>>                  server->rpc_ops = 
> nfsv4_minorversion_clientops[i];
> 		>>
> 		>> -                server->client = 
> nfs4_create_client(server, &timeparms, data->proto, authflavour);
> 		>> -                if (IS_ERR(server->client)) {
> 		>> -                        err = 
> PTR_ERR(server->client);
> 		>> -                                
> dprintk("%s: cannot create RPC client. Error = %d\n",
> 		>> -                                            
>     __FUNCTION__, err);
> 		>> -                                goto out_fail;
> 		>> -                }
> 		>>                  
> server->nfs4_state->cl_minorversion = i;
> 		>>
> 		>>              /*
> 		>> @@ -1282,8 +1288,6 @@ static int 
> nfs4_fill_super(struct super_block *sb, struct nfs4_mount_data *data, 
> 		>>                               * Shut down 
> the callback service
> 		>>                               */
> 		>>                              
> nfs_callback_down(i, server->client->cl_xprt);
> 		>> -
> 		>> -                                
> rpc_shutdown_client(server->client);
> 		>>                          }
> 		>>                      else
> 		>>                              break;
> 		>>
> 		>> 
> --------------------------------------------------------------
> ---------- 
> 		>>
> 		>> From 
> 8d73dafc0cb7cf428079803154420cb235ad25c7 Mon Sep 17 00:00:00 2001
> 		>> Message-Id: < 
> 8d73dafc0cb7cf428079803154420cb235ad25c7.1180138977.git.iyer at n
> etapp.com 
> <mailto:8d73dafc0cb7cf428079803154420cb235ad25c7.1180138977.gi
> t.iyer at netapp.com> >
> 		>> In-Reply-To: 
> <bc7afef1f3d5630884c24aa693df07d5e1c88c5a.1180138977.git.iyer@
> netapp.com >
> 		>> References: < 
> bc7afef1f3d5630884c24aa693df07d5e1c88c5a.1180138977.git.iyer at n
> etapp.com >
> 		>> From: Rahul Iyer < iyer at netapp.com 
> <mailto:iyer at netapp.com> >
> 		>> Date: Fri, 25 May 2007 15:27:19 -0700
> 		>> Subject: [PATCH 4/5] Release cl_lock when 
> unable to find renew creds in nfs4_renew_state
> 		>>
> 		>> If renew creds could not be obtained in the 
> NFSv4 case, the spin lock on 
> 		>> clp->cl_lock wasn't dropped. This resuts in 
> a soft lockup.
> 		>>
> 		>> Signed-off-by: Rahul Iyer < iyer at netapp.com 
> <mailto:iyer at netapp.com> >
> 		>> ---
> 		>>  fs/nfs/nfs4renewd.c |    1 + 
> 		>>  1 files changed, 1 insertions(+), 0 deletions(-)
> 		>>
> 		>> diff --git a/fs/nfs/nfs4renewd.c 
> b/fs/nfs/nfs4renewd.c
> 		>> index 14a918b..aff054b 100644
> 		>> --- a/fs/nfs/nfs4renewd.c 
> 		>> +++ b/fs/nfs/nfs4renewd.c
> 		>> @@ -120,6 +120,7 @@ out:
> 		>>  no_cred:
> 		>>      printk(KERN_EMERG "couldn't get renew 
> creds!\n");
> 
> 
> 
> 	don't like KERN_EMERG. just a printk would do ......
> 	 
> 	
> 	
> 
> 		>>      set_bit(NFS4CLNT_LEASE_EXPIRED, 
> &clp->cl_state); 
> 		>> +    spin_unlock(&clp->cl_lock); 
> 		>>      nfs_expire_all_delegations(clp);
> 		>>      goto out;
> 		>>  }
> 		>>
> 		>> 
> --------------------------------------------------------------
> ----------
> 		>>
> 		>> From 
> 5fe3a61025d7052a1a4e77f15413b0c3d1e81b6c Mon Sep 17 00:00:00 2001 
> 		>> Message-Id: 
> <5fe3a61025d7052a1a4e77f15413b0c3d1e81b6c.1180138977.git.iyer@
> netapp.com >
> 		>> In-Reply-To: < 
> bc7afef1f3d5630884c24aa693df07d5e1c88c5a.1180138977.git.iyer at n
> etapp.com >
> 		>> References: < 
> bc7afef1f3d5630884c24aa693df07d5e1c88c5a.1180138977.git.iyer at n
> etapp.com 
> <mailto:bc7afef1f3d5630884c24aa693df07d5e1c88c5a.1180138977.gi
> t.iyer at netapp.com> >
> 		>> From: Rahul Iyer <iyer at netapp.com >
> 		>> Date: Fri, 25 May 2007 16:45:11 -0700
> 		>> Subject: [PATCH 5/5] Move the call to 
> nfs_callback_down to nfs4_put_client.
> 		>>
> 		>> The callto nfs_callback_down is currently in 
> nfs4_free_client. Move this to
> 		>> nfs4_put_client because this is where it "fits". 
> 		>>
> 		>> Signed-off-by: Rahul Iyer <iyer at netapp.com>
> 		>> ---
> 		>>  fs/nfs/nfs4state.c |    2 +- 
> 		>>  1 files changed, 1 insertions(+), 1 deletions(-) 
> 		>>
> 		>> diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
> 		>> index 9c0d53a..f276779 100644
> 		>> --- a/fs/nfs/nfs4state.c
> 		>> +++ b/fs/nfs/nfs4state.c
> 		>> @@ -125,7 +125,6 @@ nfs4_free_client(struct 
> nfs4_client *clp) 
> 		>>      }
> 		>>      BUG_ON(!list_empty(&clp->cl_state_owners));
> 		>>      nfs_idmap_delete(clp);
> 		>> -    nfs_callback_down(minorversion, 
> clp->cl_rpcclient->cl_xprt);
> 		>>      if (!IS_ERR(clp->cl_rpcclient)) 
> 		>>              rpc_shutdown_client(clp->cl_rpcclient);
> 		>>      kfree(clp);
> 		>> @@ -189,6 +188,7 @@ nfs4_put_client(struct 
> nfs4_client *clp)
> 		>>      spin_unlock(&state_spinlock); 
> 		>>      BUG_ON(!list_empty(&clp->cl_superblocks));
> 		>>      rpc_wake_up(&clp->cl_rpcwaitq);
> 		>> +    nfs_callback_down(clp->cl_minorversion, 
> clp->cl_rpcclient->cl_xprt);
> 		>>      nfs4_kill_renewd(clp);
> 		>>      nfs4_free_client(clp);
> 		>>  }
> 		>>
> 		>> 
> --------------------------------------------------------------
> ----------
> 		>>
> 		>> _______________________________________________ 
> 		>> 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> 
> 		
> 		_______________________________________________
> 		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