On Tue, Apr 30, 2013 at 03:00:50PM -0700, Kent Overstreet wrote:
On Tue, Apr 30, 2013 at 10:53:55PM +0100, Mel Gorman wrote:
On Sat, Apr 27, 2013 at 03:19:13AM +0400, Glauber Costa wrote:
diff --git a/drivers/md/bcache/btree.c b/drivers/md/bcache/btree.c index 03e44c1..8b9c1a6 100644 --- a/drivers/md/bcache/btree.c +++ b/drivers/md/bcache/btree.c @@ -599,11 +599,12 @@ static int mca_reap(struct btree *b, struct closure *cl, unsigned min_order) return 0; }
-static int bch_mca_shrink(struct shrinker *shrink, struct shrink_control *sc) +static long bch_mca_scan(struct shrinker *shrink, struct shrink_control *sc) { struct cache_set *c = container_of(shrink, struct cache_set, shrink); struct btree *b, *t; unsigned long i, nr = sc->nr_to_scan;
long freed = 0;
if (c->shrinker_disabled) return 0;
-1 if shrinker disabled?
Otherwise if the shrinker is disabled we ultimately hit this loop in shrink_slab_one()
My memory is very hazy on this stuff, but I recall there being another loop that'd just spin if we always returned -1.
(It might've been /proc/sys/vm/drop_caches, or maybe that was another bug..)
It might be worth chasing down what that bug was and fixing it.
But 0 should certainly be safe - if we're always returning 0, then we're claiming we don't have anything to shrink.
It won't crash, but in Glauber's current code, it'll call you a few more times uselessly and the scanned statistics become misleading. I think Glauber/Dave's series is a big improvement over what we currently have and it would be nice to get it ironed out.