On Thu, 2009-06-04 at 19:17 -0700, Labiaga, Ricardo wrote:
> > > +/*
> > > + * Helper routines that track the number of preallocation elements
> > > + * on the transport.
> > > + */
> > > +static inline int xprt_need_to_requeue(struct rpc_xprt *xprt)
> > > +{
> > > + return xprt->bc_alloc_count > 0;
> > > +}
> >
> > Shouldn't this be 'xprt->bc_alloc_count < min_reqs' or something like
> > that?
>
> xprt->bc_alloc_count is correct because 'bc_alloc_count' tracks the
> number of backchannels that have been setup. In other words, return the
> preallocated structure to the queue if there are backchannels that will
> eventually consume the 'struct rpc_xprt'. Otherwise it gets freed.
> 'bc_alloc_count' does not track the number of 'struct rpc_xprt'.
OK... I think I now understand what this is doing.
Basically, you are keeping a count of the number of requests that you
want to keep preallocated. When you destroy the back channel, then you
set it to zero, and destroy all those requests that are on the 'unused'
list, then you destroy all remaining requests in the xprt_release()
callback instead of requeuing them in the 'unused' list.
Hmm... Wouldn't just a flag have been sufficient here? Why do you need
to make bc_alloc_count a counter?
> > Why are we allocating full pages here? 99.9999999% of all callbacks
> are
> > going to be small. The only exception is CB_NOTIFY, and we're not yet
> > sure we're ever going to want those...
>
> CB_NOTIFY_DEVICEID can also get long if the server uses multiple
> deviceids. It can be an unbound list, but the client does have the
> ability of telling the server the maximum size of the callback requests.
> Each deviceID is 16 bytes, so it doesn't take many to fill a page. Is
> it an overkill just for this case? I could change it if you really want
> us to.
All right, let's keep it for the moment then, but please add a comment
explaining how you pulled this particular number out of your hat.
Trond
--
Trond Myklebust
Linux NFS client maintainer
NetApp
Trond.Myklebust@xxxxxxxxxx
www.netapp.com
--
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
