Re: [PATCH v7 15/17] mm: Generalize shrink_slab() calls in shrink_node()

From: Kirill Tkhai
Date: Sat Jun 09 2018 - 04:47:11 EST


Hi, Shakeel.

On 08.06.2018 22:21, Shakeel Butt wrote:
> On Tue, May 22, 2018 at 3:09 AM Kirill Tkhai <ktkhai@xxxxxxxxxxxxx> wrote:
>>
>> From: Vladimir Davydov <vdavydov.dev@xxxxxxxxx>
>>
>> The patch makes shrink_slab() be called for root_mem_cgroup
>> in the same way as it's called for the rest of cgroups.
>> This simplifies the logic and improves the readability.
>>
>> Signed-off-by: Vladimir Davydov <vdavydov.dev@xxxxxxxxx>
>> ktkhai: Description written.
>> Signed-off-by: Kirill Tkhai <ktkhai@xxxxxxxxxxxxx>
>> ---
>> mm/vmscan.c | 21 ++++++---------------
>> 1 file changed, 6 insertions(+), 15 deletions(-)
>>
>> diff --git a/mm/vmscan.c b/mm/vmscan.c
>> index f26ca1e00efb..6dbc659db120 100644
>> --- a/mm/vmscan.c
>> +++ b/mm/vmscan.c
>> @@ -628,10 +628,8 @@ static unsigned long shrink_slab_memcg(gfp_t gfp_mask, int nid,
>> * @nid is passed along to shrinkers with SHRINKER_NUMA_AWARE set,
>> * unaware shrinkers will receive a node id of 0 instead.
>> *
>> - * @memcg specifies the memory cgroup to target. If it is not NULL,
>> - * only shrinkers with SHRINKER_MEMCG_AWARE set will be called to scan
>> - * objects from the memory cgroup specified. Otherwise, only unaware
>> - * shrinkers are called.
>> + * @memcg specifies the memory cgroup to target. Unaware shrinkers
>> + * are called only if it is the root cgroup.
>> *
>> * @priority is sc->priority, we take the number of objects and >> by priority
>> * in order to get the scan target.
>> @@ -645,7 +643,7 @@ static unsigned long shrink_slab(gfp_t gfp_mask, int nid,
>> struct shrinker *shrinker;
>> unsigned long freed = 0;
>>
>
> Shouldn't there be a VM_BUG_ON(!memcg) here?

memcg can be NULL here in case of memory controller is disabled at boot parameters
and in case of it's a global reclaim. So, such the check we can't add here.

Thanks,
Kirill