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

From: Dave Chinner <david@fromorbit.com>
To: Josef Bacik <josef@toxicpanda.com>
Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>,
	minchan@kernel.org, linux-mm@kvack.org, hannes@cmpxchg.org,
	riel@redhat.com, akpm@linux-foundation.org, kernel-team@fb.com,
	Josef Bacik <jbacik@fb.com>
Subject: Re: [PATCH 1/2] mm: use sc->priority for slab shrink targets
Date: Fri, 25 Aug 2017 08:15:59 +1000	[thread overview]
Message-ID: <20170824221559.GF21024@dastard> (raw)
In-Reply-To: <20170824144924.w3inhdnmgfscso7l@destiny>

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?

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

--
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>