Re: [PATCH 1/7] xfs: use a normal shrinker for the dquot freelist

[Ben Myers <bpm@sgi.com>]

On Fri, Feb 10, 2012 at 12:49:47PM +1100, Dave Chinner wrote:
> On Thu, Feb 09, 2012 at 05:56:26PM -0500, Christoph Hellwig wrote:
> > On Thu, Feb 09, 2012 at 04:03:20PM -0600, Ben Myers wrote:
> > > > +	LIST_HEAD		(dispose_list);
> > > > +	struct xfs_dquot	*dqp;
> > > >  
> > > > -	if (nfree <= ndqused && nfree < ndquot)
> > > > +	if ((sc->gfp_mask & (__GFP_FS|__GFP_WAIT)) != (__GFP_FS|__GFP_WAIT))
> > > >  		return 0;
> > > > +	if (!nr_to_scan)
> > > > +		goto out;
> > > 
> > > I suggest something more like:
> > > 
> > > 	if (!nr_to_scan)
> > > 		goto out;
> > >         if ((sc->gfp_mask...
> > > 		return -1;
> > 
> > Why?  Counting the number of objects when we can't actually do anything
> > is just a waste of time, and -1 vs 0 for the sizing pass seem to be
> > treateds the same in the calling code.
> 
> .....
> 
> > >  * The callback must not return -1 if nr_to_scan is zero.
> > 
> > this is against your suggestion of using -1 for the estimation pass
> > above, btw.
> 
> Technically, if the shrinker cannot make progress or the gfp mask
> means it cannot enter the filesystem code, then it should return -1,
> not zero. Yes, the calc code treats 0 and -1 the same because it is
> defensive - for the calculation a shrinker can validly return 0 to
> mean "I have no work to do" rather than "I cannot do any work in
> this context", but both mean the same thing - don't try to run the
> shrinker here.
> 
> However, the later shrinker callout to do work (i.e. nr_to_scan !=
> 0) relies on this distinction to break out of the shrink loop early
> whenteh shrinker says "can't do any work". If you just keep
> returning zero there then it will just looping uselessly until the
> scan count runs out.
> 
> The interface is a piece of shit, and I need to get back to my patch
> series that fixes this all up by separating the calculation callback
> from the work callback...

Ok... so it'll be sorted out in a different patch.  ;)

-Ben

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs