Re: The recent kref_put warning (was: [PATCH] sunrpc: remove unnecessary svc_xprt_put)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Mar 01, 2010 at 09:50:15AM -0500, J. Bruce Fields wrote:
> On Mon, Mar 01, 2010 at 04:51:14PM +1100, Neil Brown wrote:
> > On Sun, 28 Feb 2010 22:46:47 -0500
> > "J. Bruce Fields" <bfields@xxxxxxxxxxxx> wrote:
> > 
> > > On Mon, Mar 01, 2010 at 10:57:34AM +1100, Neil Brown wrote:
> > > > No, you are correct.  "return 0" is wrong, it should be "return -EAGAIN",
> > > > both in the XPT_CLOSE case and the XPT_LISTENER case.
> > > > 
> > > > I observed that in both those cases, 'len' remained at 0 and we didn't do
> > > > much else but 'return len', so I optimised.
> > > > I forgot to factor in:
> > > > 
> > > > 	if (len == 0 || len == -EAGAIN) {
> > > > 		rqstp->rq_res.len = 0;
> > > > 		svc_xprt_release(rqstp);
> > > > 		return -EAGAIN;
> > > > 	}
> > > > 
> > > > So the svc_xprt_release needs to be moved in there as well, I'm not sure
> > > > about the rq_res.len = 0.
> > > > Maybe that was a bad case of premature-optimisation :-)
> > > > 
> > > > We should probably leave that last else clause as it is and just have a
> > > > single return from the function.
> > > 
> > > OK, so the below is what I'm thinking of sending, after some testing;
> > > really just a split-up version of your patches (uh, so credits may be
> > > wrong) with the final cleanup removed:
> > 
> > Credits and code look OK the me, thanks.

And, by the way, this is all ready to submit--but I'd like to avoid
having to revert anything more, and as part of that I'd greatly
appreciate any testing results, however partial.

--b.
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Filesystem Development]     [Linux USB Development]     [Linux Media Development]     [Video for Linux]     [Linux NILFS]     [Linux Audio Users]     [Yosemite Info]     [Linux SCSI]

  Powered by Linux