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

J. Bruce Fields
2010-03-01T23:18:49Z

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@fieldses.org> 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.