Re: [PATCH 4/7] sunrpc/cache: centralise handling of size limit on deferred list.

[Neil Brown <neilb@suse.de>]

On Wed, 22 Sep 2010 14:31:54 -0400
"J. Bruce Fields" <bfields@fieldses.org> wrote:

> On Wed, Sep 22, 2010 at 12:55:07PM +1000, NeilBrown wrote:
> > We limit the number of 'defer' requests to DFR_MAX.
> > 
> > The imposition of this limit is spread about a bit - sometime we don't
> > add new things to the list, sometimes we remove old things.
> > 
> > Also it is currently applied to requests which we are 'waiting' for
> > rather than 'deferring'.  This doesn't seem ideal as 'waiting'
> > requests are naturally limited by the number of threads.
> > 
> > So gather the DFR_MAX handling code to one place and only apply it to
> > requests that are actually being deferred.
> > 
> > This means that not all 'cache_deferred_req' structures go on the
> > 'cache_defer_list, so we need to be careful when removing things.
> 
> The idea sounds OK.
> 
> > +static void cache_limit_defers(struct cache_deferred_req *dreq)
> > +{
> > +	/* Add 'dreq' to the list of deferred requests and make sure
> > +	 * we don't exceed the limit of allowed deferred requests.
> > +	 */
> > +	struct cache_deferred_req *discard = NULL;
> > +
> > +	spin_lock(&cache_defer_lock);
> > +	if (!list_empty(&dreq->recent) ||
> > +	    hlist_unhashed(&dreq->hash)) {
> > +		/* Must have lost a race, maybe cache_revisit_request is
> > +		 * already processing this.  In any case, there is nothing for
> > +		 * us to do.
> > +		 */
> > +		spin_unlock(&cache_defer_lock);
> 
> Did you mean to return here?
> 

Yes :-(

Thanks,
NeilBrown