From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qk0-f199.google.com (mail-qk0-f199.google.com [209.85.220.199]) by kanga.kvack.org (Postfix) with ESMTP id 00A17440882 for ; Thu, 24 Aug 2017 18:45:49 -0400 (EDT) Received: by mail-qk0-f199.google.com with SMTP id o184so4205480qkc.6 for ; Thu, 24 Aug 2017 15:45:48 -0700 (PDT) Received: from mail-qt0-x242.google.com (mail-qt0-x242.google.com. [2607:f8b0:400d:c0d::242]) by mx.google.com with ESMTPS id g11si3257275qtc.127.2017.08.24.15.45.47 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 24 Aug 2017 15:45:47 -0700 (PDT) Received: by mail-qt0-x242.google.com with SMTP id e2so393769qta.3 for ; Thu, 24 Aug 2017 15:45:47 -0700 (PDT) Date: Thu, 24 Aug 2017 18:45:46 -0400 From: Josef Bacik Subject: Re: [PATCH 1/2] mm: use sc->priority for slab shrink targets Message-ID: <20170824224544.3yrsl36u536fgkhc@destiny> References: <1503430539-2878-1-git-send-email-jbacik@fb.com> <20170824144924.w3inhdnmgfscso7l@destiny> <20170824221559.GF21024@dastard> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170824221559.GF21024@dastard> Sender: owner-linux-mm@kvack.org List-ID: To: Dave Chinner Cc: Josef Bacik , Andrey Ryabinin , minchan@kernel.org, linux-mm@kvack.org, hannes@cmpxchg.org, riel@redhat.com, akpm@linux-foundation.org, kernel-team@fb.com, Josef Bacik 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: > > On Thu, Aug 24, 2017 at 05:29:59PM +0300, Andrey Ryabinin wrote: > > > > > > > > > On 08/22/2017 10:35 PM, josef@toxicpanda.com wrote: > > > > --- 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: email@kvack.org