lkml.org 
[lkml]   [2018]   [May]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH v5 11/13] mm: Iterate only over charged shrinkers during memcg shrink_slab()
From
Date
On 17.05.2018 07:16, Vladimir Davydov wrote:
> 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.

shrink_slab() is used not only in this place. I does not seem a trivial
change for 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.

Because this function is used in several places, and we have some particulars
on root_mem_cgroup initialization, and this function called from these places
with different states of root_mem_cgroup. It does not seem trivial fix for me.

Let's do it on top of the series later, what is the problem? It does not seem
critical problem.

Kirill

\
 
 \ /
  Last update: 2018-05-17 13:50    [W:0.118 / U:0.400 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site