linux-kernel - Re: [PATCH -next v3 2/2] memcg: remove mem_cgroup

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <476fcde5-d54d-4b15-9870-844b3b8c700a@huaweicloud.com>
Date: Tue, 16 Dec 2025 20:34:40 +0800
From: Chen Ridong <chenridong@...weicloud.com>
To: Michal Koutný <mkoutny@...e.com>
Cc: hannes@...xchg.org, mhocko@...nel.org, roman.gushchin@...ux.dev,
 shakeel.butt@...ux.dev, muchun.song@...ux.dev, akpm@...ux-foundation.org,
 axelrasmussen@...gle.com, yuanchu@...gle.com, weixugc@...gle.com,
 david@...nel.org, zhengqi.arch@...edance.com, lorenzo.stoakes@...cle.com,
 cgroups@...r.kernel.org, linux-mm@...ck.org, linux-kernel@...r.kernel.org,
 lujialin4@...wei.com
Subject: Re: [PATCH -next v3 2/2] memcg: remove mem_cgroup_size()



On 2025/12/16 0:28, Michal Koutný wrote:
> Hi Ridong.
> 
> On Thu, Dec 11, 2025 at 01:30:19AM +0000, Chen Ridong <chenridong@...weicloud.com> wrote:
>> From: Chen Ridong <chenridong@...wei.com>
>>
>> The mem_cgroup_size helper is used only in apply_proportional_protection
>> to read the current memory usage. Its semantics are unclear and
>> inconsistent with other sites, which directly call page_counter_read for
>> the same purpose.
>>
>> Remove this helper and get its usage via mem_cgroup_protection for
>> clarity. Additionally, rename the local variable 'cgroup_size' to 'usage'
>> to better reflect its meaning.
>>
>> No functional changes intended.
>>
>> Signed-off-by: Chen Ridong <chenridong@...wei.com>
> 
> Why does mem_cgroup_calculate_protection "calculate" usage for its
> callers? Couldn't you just the change source in
> apply_proportional_protection()?
> 
> Thanks,
> Michal
> 

I apologize for missing this message earlier.

In my v2 patch, I was reading the usage directly:

+		unsigned long usage = page_counter_read(&memcg->memory);

This works fine when CONFIG_MEMCG=y, but fails to compile when memory cgroups are disabled. To
handle this, I initially added #ifdef CONFIG_MEMCG guards.

Following Johannes's suggestion, I have now moved this logic into mem_cgroup_protection() to
eliminate the #ifdef and keep the code cleaner.

Discussion:
https://lore.kernel.org/all/20251210163634.GB643576@cmpxchg.org/

>> @@ -2485,7 +2485,6 @@ static unsigned long apply_proportional_protection(struct mem_cgroup *memcg,
>>  		 * again by how much of the total memory used is under
>>  		 * hard protection.
>>  		 */
>> -		unsigned long cgroup_size = mem_cgroup_size(memcg);
> +		unsigned long cgroup_size = page_counter_read(memcg);
> 
>>  		unsigned long protection;
>>  
>>  		/* memory.low scaling, make sure we retry before OOM */
>> @@ -2497,9 +2496,9 @@ static unsigned long apply_proportional_protection(struct mem_cgroup *memcg,
>>  		}
>>  
>>  		/* Avoid TOCTOU with earlier protection check */
>> -		cgroup_size = max(cgroup_size, protection);
>> +		usage = max(usage, protection);
>>  
>> -		scan -= scan * protection / (cgroup_size + 1);
>> +		scan -= scan * protection / (usage + 1);
>>  
>>  		/*
>>  		 * Minimally target SWAP_CLUSTER_MAX pages to keep

-- 
Best regards,
Ridong