Re: [PATCH 2/3] SUNRPC: Fix the TCP write space reservations for deferred requests

["J. Bruce Fields" <bfields@fieldses.org>]

On Sat, Jun 20, 2009 at 05:44:08PM -0400, Trond Myklebust wrote:
> On Fri, 2009-06-19 at 18:23 -0400, J. Bruce Fields wrote:
> > On Mon, May 18, 2009 at 05:47:56PM -0400, Trond Myklebust wrote:
> > > Ensure that deferred requests are accounted for correctly by the write
> > > space reservation mechanism. In order to avoid double counting, remove the
> > > reservation when we defer the request, and save any calculated value, so
> > > that we can restore it when the request is requeued.
> > 
> > I like that it does the addition to xpt_reserved in just one place
> > instead of two, and carrying over the reserved_space is nice, but I
> > don't understand the "double accounting" comment--where exactly is
> > something counted twice?
> 
> Correct me if I'm wrong, but as far as I can see, the current code bumps
> xpt_reserved every time you call svc_xprt_enqueue() irrespective of
> whether or not you are going to process a new RPC call, or whether you
> are processing a deferred call.
> 
> In the latter case, there doesn't appear to be any code that subtracts
> from xpt_reserved prior to the re-enqueue process, hence my belief that
> we are double counting those events...

Sorry for the slow response!  The original request is
dropped--cache_check() returns -EAGAIN on queueing up the deferred
request, the caller returns back to svc_process, and the reservation is
removed from xpt_reserved by svc_drop().

And a new reservation isn't made until the deferred request is
revisited, at which point it's treated as any new request received from
a socket would be.

Your patch would instead have the server keep that reservation while the
request is deferred.

I'm not really sure how to decide which is better.

--b.