Re: [PATCH 2.6.30] svcrdma: clean up error paths.

[Steve Wise <swise@opengridcomputing.com>]

Hey Bruce,

J. Bruce Fields wrote:
> On Wed, Apr 29, 2009 at 02:14:00PM -0500, Steve Wise wrote:
>   
>> These fixes resolved crashes due to resource leak BUG_ON checks. The
>> resource leaks were detected by introducing asynchronous transport errors.
>>     
>
> Thanks, applied for 2.6.30.  (And also appropriate for stable (2.6.29),
> I assume?)
>
> But, could someone take a closer look at the error paths here?  Questions:
>
> 	- svc_rdma_post_recv() does a svc_rdma_put_context() on error--
> 	  are you sure its caller needs to as well?
>   

The svc_rdma_put_context() call inside svc_rdma_post_recv() is for the 
recv context that was allocated inside that function.  The caller, in 
this case send_reply() also does a svc_rdma_put_context(), but that is 
for the send context.   So I think this is correct.


> 	- In send_reply, some of the cleanout is shared between the
> 	  first return -ENOTCONN and the final err: cleanup.  Could we
> 	  add another err: label and share some of that cleanup?
>   

The only common logic I see is the svc_rdma_put_context() call that 
could be shared.  But one case calls it with free_pages == 1 after the 
pages have been mapped, and the other with 0 since no pages are mapped 
at that point (when the call to svc_rdma_post_recv() fails).   So I'm 
not sure its worth doing?

 
Steve.