| 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
| ||
|
Message-ID: <b2a64a62-9cf2-4d81-bcd0-1af7e64a34db@linux.dev>
Date: Fri, 21 Nov 2025 16:17:52 +0800
From: Qi Zheng <qi.zheng@...ux.dev>
To: Chen Ridong <chenridong@...weicloud.com>, hannes@...xchg.org,
hughd@...gle.com, mhocko@...e.com, roman.gushchin@...ux.dev,
shakeel.butt@...ux.dev, muchun.song@...ux.dev, david@...hat.com,
lorenzo.stoakes@...cle.com, ziy@...dia.com, harry.yoo@...cle.com,
imran.f.khan@...cle.com, kamalesh.babulal@...cle.com,
axelrasmussen@...gle.com, yuanchu@...gle.com, weixugc@...gle.com,
akpm@...ux-foundation.org
Cc: linux-mm@...ck.org, linux-kernel@...r.kernel.org,
cgroups@...r.kernel.org, Muchun Song <songmuchun@...edance.com>,
Qi Zheng <zhengqi.arch@...edance.com>
Subject: Re: [PATCH v1 05/26] mm: memcontrol: allocate object cgroup for
non-kmem case
On 11/21/25 11:58 AM, Chen Ridong wrote:
>
>
> On 2025/10/28 21:58, Qi Zheng wrote:
>> From: Muchun Song <songmuchun@...edance.com>
>>
>> Pagecache pages are charged at allocation time and hold a reference
>> to the original memory cgroup until reclaimed. Depending on memory
>> pressure, page sharing patterns between different cgroups and cgroup
>> creation/destruction rates, many dying memory cgroups can be pinned
>> by pagecache pages, reducing page reclaim efficiency and wasting
>> memory. Converting LRU folios and most other raw memory cgroup pins
>> to the object cgroup direction can fix this long-living problem.
>>
>> As a result, the objcg infrastructure is no longer solely applicable
>> to the kmem case. In this patch, we extend the scope of the objcg
>> infrastructure beyond the kmem case, enabling LRU folios to reuse
>> it for folio charging purposes.
>>
>> It should be noted that LRU folios are not accounted for at the root
>> level, yet the folio->memcg_data points to the root_mem_cgroup. Hence,
>> the folio->memcg_data of LRU folios always points to a valid pointer.
>> However, the root_mem_cgroup does not possess an object cgroup.
>> Therefore, we also allocate an object cgroup for the root_mem_cgroup.
>>
>> Signed-off-by: Muchun Song <songmuchun@...edance.com>
>> Signed-off-by: Qi Zheng <zhengqi.arch@...edance.com>
>> ---
>> mm/memcontrol.c | 51 +++++++++++++++++++++++--------------------------
>> 1 file changed, 24 insertions(+), 27 deletions(-)
>>
>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>> index d5257465c9d75..2afd7f99ca101 100644
>> --- a/mm/memcontrol.c
>> +++ b/mm/memcontrol.c
>> @@ -204,10 +204,10 @@ static struct obj_cgroup *obj_cgroup_alloc(void)
>> return objcg;
>> }
>>
>> -static void memcg_reparent_objcgs(struct mem_cgroup *memcg,
>> - struct mem_cgroup *parent)
>> +static void memcg_reparent_objcgs(struct mem_cgroup *memcg)
>> {
>> struct obj_cgroup *objcg, *iter;
>> + struct mem_cgroup *parent = parent_mem_cgroup(memcg);
>>
>> objcg = rcu_replace_pointer(memcg->objcg, NULL, true);
>>
>> @@ -3302,30 +3302,17 @@ unsigned long mem_cgroup_usage(struct mem_cgroup *memcg, bool swap)
>> return val;
>> }
>>
>> -static int memcg_online_kmem(struct mem_cgroup *memcg)
>> +static void memcg_online_kmem(struct mem_cgroup *memcg)
>> {
>> - struct obj_cgroup *objcg;
>> -
>> if (mem_cgroup_kmem_disabled())
>> - return 0;
>> + return;
>>
>> if (unlikely(mem_cgroup_is_root(memcg)))
>> - return 0;
>> -
>> - objcg = obj_cgroup_alloc();
>> - if (!objcg)
>> - return -ENOMEM;
>> -
>> - objcg->memcg = memcg;
>> - rcu_assign_pointer(memcg->objcg, objcg);
>> - obj_cgroup_get(objcg);
>> - memcg->orig_objcg = objcg;
>> + return;
>>
>> static_branch_enable(&memcg_kmem_online_key);
>>
>> memcg->kmemcg_id = memcg->id.id;
>> -
>> - return 0;
>> }
>>
>> static void memcg_offline_kmem(struct mem_cgroup *memcg)
>> @@ -3340,12 +3327,6 @@ static void memcg_offline_kmem(struct mem_cgroup *memcg)
>>
>> parent = parent_mem_cgroup(memcg);
>> memcg_reparent_list_lrus(memcg, parent);
>> -
>> - /*
>> - * Objcg's reparenting must be after list_lru's, make sure list_lru
>> - * helpers won't use parent's list_lru until child is drained.
>> - */
>> - memcg_reparent_objcgs(memcg, parent);
>> }
>>
>> #ifdef CONFIG_CGROUP_WRITEBACK
>> @@ -3862,9 +3843,9 @@ mem_cgroup_css_alloc(struct cgroup_subsys_state *parent_css)
>> static int mem_cgroup_css_online(struct cgroup_subsys_state *css)
>> {
>> struct mem_cgroup *memcg = mem_cgroup_from_css(css);
>> + struct obj_cgroup *objcg;
>>
>> - if (memcg_online_kmem(memcg))
>> - goto remove_id;
>> + memcg_online_kmem(memcg);
>>
>> /*
>> * A memcg must be visible for expand_shrinker_info()
>> @@ -3874,6 +3855,15 @@ static int mem_cgroup_css_online(struct cgroup_subsys_state *css)
>> if (alloc_shrinker_info(memcg))
>> goto offline_kmem;
>>
>> + objcg = obj_cgroup_alloc();
>> + if (!objcg)
>> + goto free_shrinker;
>> +
>> + objcg->memcg = memcg;
>> + rcu_assign_pointer(memcg->objcg, objcg);
>> + obj_cgroup_get(objcg);
>> + memcg->orig_objcg = objcg;
>> +
>
> Will it be better to add a helper function like obj_cgroup_init()?
This part is not complicated, and it is only called in this one place,
so perhaps there is no need to add a helper function?
Of course, it doesn't matter, I'm fine with either method.
>
>> if (unlikely(mem_cgroup_is_root(memcg)) && !mem_cgroup_disabled())
>> queue_delayed_work(system_unbound_wq, &stats_flush_dwork,
>> FLUSH_TIME);
>> @@ -3896,9 +3886,10 @@ static int mem_cgroup_css_online(struct cgroup_subsys_state *css)
>> xa_store(&mem_cgroup_ids, memcg->id.id, memcg, GFP_KERNEL);
>>
>> return 0;
>> +free_shrinker:
>> + free_shrinker_info(memcg);
>> offline_kmem:
>> memcg_offline_kmem(memcg);
>> -remove_id:
>> mem_cgroup_id_remove(memcg);
>> return -ENOMEM;
>> }
>> @@ -3916,6 +3907,12 @@ static void mem_cgroup_css_offline(struct cgroup_subsys_state *css)
>>
>> memcg_offline_kmem(memcg);
>> reparent_deferred_split_queue(memcg);
>> + /*
>> + * The reparenting of objcg must be after the reparenting of the
>> + * list_lru and deferred_split_queue above, which ensures that they will
>> + * not mistakenly get the parent list_lru and deferred_split_queue.
>> + */
>> + memcg_reparent_objcgs(memcg);
>> reparent_shrinker_deferred(memcg);
>> wb_memcg_offline(memcg);
>> lru_gen_offline_memcg(memcg);
>
Powered by blists - more mailing lists