[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20180517041634.lgkym6gdctya3oq6@esperanza>
Date: Thu, 17 May 2018 07:16:34 +0300
From: Vladimir Davydov <vdavydov.dev@...il.com>
To: Kirill Tkhai <ktkhai@...tuozzo.com>
Cc: akpm@...ux-foundation.org, shakeelb@...gle.com,
viro@...iv.linux.org.uk, hannes@...xchg.org, mhocko@...nel.org,
tglx@...utronix.de, pombredanne@...b.com, stummala@...eaurora.org,
gregkh@...uxfoundation.org, sfr@...b.auug.org.au, guro@...com,
mka@...omium.org, penguin-kernel@...ove.SAKURA.ne.jp,
chris@...is-wilson.co.uk, longman@...hat.com, minchan@...nel.org,
ying.huang@...el.com, mgorman@...hsingularity.net, jbacik@...com,
linux@...ck-us.net, linux-kernel@...r.kernel.org,
linux-mm@...ck.org, willy@...radead.org, lirongqing@...du.com,
aryabinin@...tuozzo.com
Subject: Re: [PATCH v5 11/13] mm: Iterate only over charged shrinkers during
memcg shrink_slab()
On Tue, May 15, 2018 at 05:49:59PM +0300, Kirill Tkhai wrote:
> >> @@ -589,13 +647,7 @@ static unsigned long shrink_slab(gfp_t gfp_mask, int nid,
> >> .memcg = memcg,
> >> };
> >>
> >> - /*
> >> - * If kernel memory accounting is disabled, we ignore
> >> - * SHRINKER_MEMCG_AWARE flag and call all shrinkers
> >> - * passing NULL for memcg.
> >> - */
> >> - if (memcg_kmem_enabled() &&
> >> - !!memcg != !!(shrinker->flags & SHRINKER_MEMCG_AWARE))
> >> + if (!!memcg != !!(shrinker->flags & SHRINKER_MEMCG_AWARE))
> >> continue;
> >
> > I want this check gone. It's easy to achieve, actually - just remove the
> > following lines from shrink_node()
> >
> > if (global_reclaim(sc))
> > shrink_slab(sc->gfp_mask, pgdat->node_id, NULL,
> > sc->priority);
>
> This check is not related to the patchset.
Yes, it is. This patch modifies shrink_slab which is used only by
shrink_node. Simplifying shrink_node along the way looks right to me.
> Let's don't mix everything in the single series of patches, because
> after your last remarks it will grow at least up to 15 patches.
Most of which are trivial so I don't see any problem here.
> This patchset can't be responsible for everything.
I don't understand why you balk at simplifying the code a bit while you
are patching related functions anyway.
>
> >>
> >> if (!(shrinker->flags & SHRINKER_NUMA_AWARE))
> >>
Powered by blists - more mailing lists