Thread (9 messages) 9 messages, 4 authors, 2017-08-25

Re: [PATCH 1/2] mm: use sc->priority for slab shrink targets

From: Josef Bacik <josef@toxicpanda.com>
Date: 2017-08-24 22:45:49

On Fri, Aug 25, 2017 at 08:15:59AM +1000, Dave Chinner wrote:
On Thu, Aug 24, 2017 at 10:49:25AM -0400, Josef Bacik wrote:
quoted
On Thu, Aug 24, 2017 at 05:29:59PM +0300, Andrey Ryabinin wrote:
quoted

On 08/22/2017 10:35 PM, josef@toxicpanda.com wrote:
quoted
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -306,9 +306,7 @@ EXPORT_SYMBOL(unregister_shrinker);
 #define SHRINK_BATCH 128
 
 static unsigned long do_shrink_slab(struct shrink_control *shrinkctl,
-				    struct shrinker *shrinker,
-				    unsigned long nr_scanned,
-				    unsigned long nr_eligible)
+				    struct shrinker *shrinker, int priority)
 {
 	unsigned long freed = 0;
 	unsigned long long delta;
@@ -333,9 +331,8 @@ static unsigned long do_shrink_slab(struct shrink_control *shrinkctl,
 	nr = atomic_long_xchg(&shrinker->nr_deferred[nid], 0);
 
 	total_scan = nr;
-	delta = (4 * nr_scanned) / shrinker->seeks;
-	delta *= freeable;
-	do_div(delta, nr_eligible + 1);
+	delta = freeable >> priority;
+	delta = (4 * freeable) / shrinker->seeks;
Something is wrong. The first line does nothing.
Lol jesus, nice catch, I'll fix this up.  Thanks,
Josef, this bug has been in every patch you've sent. What does
fixing it do to the behaviour of the algorithm now? It's going to
change it, for sure, so can you run all your behavioural
characterisation tests and let us know what the difference between
the broken and fixed patches are?
The bug made it so we were way more agressive with slab reclaim than we should
be.  My second patch masked this with the inactive/slab diff stuff, but I've
dropped that patch since its controversial and I don't really care to argue
about it anymore.  This patch still fixes the issue of us not reclaiming enough
in slab mostly workloads, I ran my fs_mark test before I sent out the new
version to verify there still isn't the huge drop in performance once reclaim
kicks in.  Without any changes we reclaimed basically no slab, with the bug in
place (without my second patch) we reclaimed all of the slab in one go, and with
the fixed patch we reclaim a proportional amount each time we enter the
shrinker.  Thanks,

Josef

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help