From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5F9A1C83000 for ; Mon, 30 Jun 2025 07:17:01 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id E0C9F8D0002; Mon, 30 Jun 2025 03:17:00 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id DE3A28D0001; Mon, 30 Jun 2025 03:17:00 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id CF9F18D0002; Mon, 30 Jun 2025 03:17:00 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id BE7BC8D0001 for ; Mon, 30 Jun 2025 03:17:00 -0400 (EDT) Received: from smtpin11.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 5ACAB58D45 for ; Mon, 30 Jun 2025 07:17:00 +0000 (UTC) X-FDA: 83611210200.11.1C4BCC1 Received: from mail-wr1-f53.google.com (mail-wr1-f53.google.com [209.85.221.53]) by imf16.hostedemail.com (Postfix) with ESMTP id D397618000E for ; Mon, 30 Jun 2025 07:16:57 +0000 (UTC) Authentication-Results: imf16.hostedemail.com; dkim=pass header.d=bytedance.com header.s=google header.b=B2r8Pb6n; spf=pass (imf16.hostedemail.com: domain of songmuchun@bytedance.com designates 209.85.221.53 as permitted sender) smtp.mailfrom=songmuchun@bytedance.com; dmarc=pass (policy=quarantine) header.from=bytedance.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1751267818; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=XtwfhBZq+/qOoD0+cgMV2ISyypIUfUwX6SyoRcyAoeE=; b=AnANNQEPeHjtMkEh+kLwE8d2CBT1XPYHkujURMgIgVHWVesdN+kvX+UviMLfhcZKzIzHpq f0rnoHEAdGu4JGDkTGoAl1G0QAIpfZ+VSaeV+LtNZ4uyEHpV26KY2uWVfpwvVfyArNY8EB bdUmnkH/j5FypAV6KYO71wewyIJNecE= ARC-Authentication-Results: i=1; imf16.hostedemail.com; dkim=pass header.d=bytedance.com header.s=google header.b=B2r8Pb6n; spf=pass (imf16.hostedemail.com: domain of songmuchun@bytedance.com designates 209.85.221.53 as permitted sender) smtp.mailfrom=songmuchun@bytedance.com; dmarc=pass (policy=quarantine) header.from=bytedance.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1751267818; a=rsa-sha256; cv=none; b=thvteTm3b7Jx+MdjbRCErZz+fATBNHSAzS164lBUxadBQqEAXfimzDCY43/yLB5xuU1w4v YdaHSrIByyA8ra6qtadL/x6HdvEU8BWs0BiT7UWadH6fH79K3qlW6W6iF3fCH+6p+xLx+7 Ig1VBzhDatxnzdBf+7u5nLaOszYnL/U= Received: by mail-wr1-f53.google.com with SMTP id ffacd0b85a97d-3a536ecbf6fso2282125f8f.2 for ; Mon, 30 Jun 2025 00:16:57 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bytedance.com; s=google; t=1751267816; x=1751872616; darn=kvack.org; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:from:to:cc:subject:date :message-id:reply-to; bh=XtwfhBZq+/qOoD0+cgMV2ISyypIUfUwX6SyoRcyAoeE=; b=B2r8Pb6nB5Te6A5zY8/WCaUIx+QfKxaDlBw0vPySDk7/8XHS14kNQOcKxypT5JI5K5 4gbIWwBdXStOymSw2XMCEYgPWAI9EDylV5tDBmWk5PWfyfKmpZTet6eE0ugQjgEsYNb3 Lm5lunadEFx9m59lLQDze/eB0DWop1sS41LFz0pjoTlCEZQYAgIEig5lddu6ccZzbLh2 WSTUZUwCwwD8m+l0VHHShijW6AbbHYt8/CVs+woXeRkxsaVypl4QD4TFaleP3UscZAB6 8a2d6Qr8S9d5Y79XBJEl3EppUqsZQHsxu1fcn/0oKDJatIv3nspzQoHioqG8fBT15I3C 9bww== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1751267816; x=1751872616; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=XtwfhBZq+/qOoD0+cgMV2ISyypIUfUwX6SyoRcyAoeE=; b=OVLohSXhEXRabydBXKZXjNTK13+7qq4DdfvLtS+EQvq+l0rFpJ+RVWpfXRi6G2Mlcv LPrT+n3y7h98Gf24LTOjTCHSCYHgTTpWaKw0CxefF/PKkVFduZ0m3weZoZ98O84F0Axb nI1zrd3ZOVukhkDhqu4RQdNX3NK/PQYE67aglCF6A5K5HnOVAYtUwRYlPsb0tNCe0+he 1Wv4ddveoPJG0TidyGaXb0TftD55D96+xPL1CTRx7vUXjILd3G18EcjnyqpdhlEIGP2m J1GJnyyveXARPWCw5Raqdk1R2oo3a8+TPLG+W8oJBPR/DD18CmXKGcOThbxdnfu0AasL OV5A== X-Forwarded-Encrypted: i=1; AJvYcCXrXnncDglLPBcRtdIkiIzw8gzUwfMcOJqqAk365LySU4XAztDD8F535tTxym0eFx93aEScTuvARA==@kvack.org X-Gm-Message-State: AOJu0YwZqsj1LVLfre6VK8dtAYpkGU+tQQSJ/aKZVcUye3O7cmpXvAnb WGjcCebSQkZ+yxGeNBoS9nupQFvlx8O9rkkSSSVYk6T2r/p1K3pfSub4aGDujdQsjGYJNfKxnWd 5P0ArNZrfIDOQseD98OQ7jmAsXDaOcBgizCpv9PmhAg== X-Gm-Gg: ASbGncvnjz7XNiWdJg8uLgD9fgzTXI7ceq5e+IE4E+N8R+qiD0vw4wtZc1qK7vnuZH+ 5NY5PzqRqGqXbRTD7co34IjHiAd1mjOV7ob51a1xjR9nqdkTG6wC5TdztPLN6es6SWCywWI31DJ KJ379WgiBg2Q6x3hcW9k93cb5reFzQKmq5Y5G+2ryYxE2zYw== X-Google-Smtp-Source: AGHT+IGWcMoGPyQen9EzSdhD+7SA3oKhs+KyVeAZvmZJHvemv5d7E0XLkUi6FCGl+40J2OvnKEDqRXte7pQqFVwSe1A= X-Received: by 2002:adf:b60b:0:b0:3a4:f939:b53 with SMTP id ffacd0b85a97d-3a8ffcc9d53mr8921516f8f.38.1751267816068; Mon, 30 Jun 2025 00:16:56 -0700 (PDT) MIME-Version: 1.0 References: <20250415024532.26632-1-songmuchun@bytedance.com> <20250415024532.26632-11-songmuchun@bytedance.com> In-Reply-To: From: Muchun Song Date: Mon, 30 Jun 2025 15:16:18 +0800 X-Gm-Features: Ac12FXw01TxCGLVgLGT-K9XFIEOAOjYaHiqekorDSE1Mqeqfw5mapo0SJXdnmhc Message-ID: Subject: Re: Re: [PATCH RFC 10/28] mm: memcontrol: return root object cgroup for root memory cgroup To: Chen Ridong Cc: Johannes Weiner , Michal Hocko , Roman Gushchin , Shakeel Butt , muchun.song@linux.dev, Andrew Morton , Dave Chinner , Qi Zheng , yosry.ahmed@linux.dev, Nhat Pham , chengming.zhou@linux.dev, LKML , Cgroups , Linux Memory Management List , hamzamahfooz@linux.microsoft.com, apais@linux.microsoft.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Rspamd-Server: rspam06 X-Rspamd-Queue-Id: D397618000E X-Stat-Signature: 5mjyzu5qqsbc7s1i3tj5by3a6n637jhk X-HE-Tag: 1751267817-728093 X-HE-Meta: U2FsdGVkX186tmaNv+tkMrqNEKX0vMYEC3+mHldZZB1djgsPpjroipfAoBhvbtjbMy5rODhPFWIanTCIfh/KRI37pH65pZdEV/ypCaBddrP1skkIWBz9lX92EVu+9VMEtpmvtGRHc2N6JF8YbFDjB6agEvAfgbyemd6/0QRjBdjZYhiGwGBg9y3wZb5N9RMrvA50SB9PohaUmo1y+zLaq7daNPps6gPyprgGAlp/HDRs1JXJbmea3Fykr+mhtbnmLAc7/gx4GKEP21LUmfCcEdS+RCBjki1e1xvwYMstoskeL9pdigekzHDBcORE8g24URIPiVHpo/kix0w7VGRNPK3DJZcGGJD7zemxC9VLoPNkuNtsW/ODqWNoHYBWb2ZqnV7F5X2ALrSrnLApScPQply7MtdP4LvKFiXjkSg68wLUut9g+YPvS2NSjE5YjOCWvSSC05/g2lmcWU8uZDs+u3wh5E6618r+2xSLp5+AvFL0JjxPExH5avUR1hqH7VyoM1wZ2XP7pXYZPoYR/NJ9da5GvQh+Vic5SS2cLdEN7pwspgb+XszQjVkfZdFv5S5+Kr/oSfYbilDqepY27TJsDIyRUY1Gh4mIPz7CRXhGhUzCC4ckFyxhCCF4Kao+viFC81CgbswKlvnOFQ1twFmtlJKRjCdijRVaZWxErt6nvUDwOYF9/k9Sv4tbjDU8P8U66MNWDTOv+vV9rHUV4sW7QgSJFT91P1ATIuSkDkz/SZ3BggFj3wChp+9oG30kC/ZMStiJ1MeZnzKNogCl/sMUh+EJfGcdGwak+6QKVnWAO9sz5ZAZrVAdotP5eG4DBSQSr6kntiGltP5WU3LKB1Yll4QGeLyQq4S7Earl0JIJ6Lh9h8i56vG7/H/Z7PpbGf/+u0AqEDISGl1s69Vdv4p5ny/fwz/uLgD+F2n8J3WoIkZHgAoFR0XWD+rzQsfV/aaxCAWUAyb5l1BYqs9EjbO gMbDLbfZ OlNqUXW7hLjze0tRQK1fn1WffWtSLX3ko9oUd+SyzSlclDBbjXczoF5kvZoql8bshD57RRS6lUL8iiC35F7GapVj8n8dtKlq4VGoo3388scbzHFyD3zYTXf7GQ6OeeAOMU/76UKGHEC1xi/QPblkXEkNnzclTk7MaQxjdlz7AxikevvoXRjiN4ik9CCvu7UA56PxxlBZpWtdP/999jI4LjohQm/xnxgXyNi8ULxZV65L5vTGPCG3qf3BM1usT9yBsm5nnWKWLi0FUOZKr1FuhzYIdDgLtsR65zt/E X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Sat, Jun 28, 2025 at 11:09=E2=80=AFAM Chen Ridong wrote: > > > > On 2025/4/15 10:45, Muchun Song wrote: > > Memory cgroup functions such as get_mem_cgroup_from_folio() and > > get_mem_cgroup_from_mm() return a valid memory cgroup pointer, > > even for the root memory cgroup. In contrast, the situation for > > object cgroups has been different. > > > > Previously, the root object cgroup couldn't be returned because > > it didn't exist. Now that a valid root object cgroup exists, for > > the sake of consistency, it's necessary to align the behavior of > > object-cgroup-related operations with that of memory cgroup APIs. > > > > Signed-off-by: Muchun Song > > --- > > include/linux/memcontrol.h | 29 ++++++++++++++++++------- > > mm/memcontrol.c | 44 ++++++++++++++++++++------------------ > > mm/percpu.c | 2 +- > > 3 files changed, 45 insertions(+), 30 deletions(-) > > > > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h > > index bb4f203733f3..e74922d5755d 100644 > > --- a/include/linux/memcontrol.h > > +++ b/include/linux/memcontrol.h > > @@ -319,6 +319,7 @@ struct mem_cgroup { > > #define MEMCG_CHARGE_BATCH 64U > > > > extern struct mem_cgroup *root_mem_cgroup; > > +extern struct obj_cgroup *root_obj_cgroup; > > > > enum page_memcg_data_flags { > > /* page->memcg_data is a pointer to an slabobj_ext vector */ > > @@ -528,6 +529,11 @@ static inline bool mem_cgroup_is_root(struct mem_c= group *memcg) > > return (memcg =3D=3D root_mem_cgroup); > > } > > > > +static inline bool obj_cgroup_is_root(const struct obj_cgroup *objcg) > > +{ > > + return objcg =3D=3D root_obj_cgroup; > > +} > > + > > static inline bool mem_cgroup_disabled(void) > > { > > return !cgroup_subsys_enabled(memory_cgrp_subsys); > > @@ -752,23 +758,26 @@ struct mem_cgroup *mem_cgroup_from_css(struct cgr= oup_subsys_state *css){ > > > > static inline bool obj_cgroup_tryget(struct obj_cgroup *objcg) > > { > > + if (obj_cgroup_is_root(objcg)) > > + return true; > > return percpu_ref_tryget(&objcg->refcnt); > > } > > > > -static inline void obj_cgroup_get(struct obj_cgroup *objcg) > > +static inline void obj_cgroup_get_many(struct obj_cgroup *objcg, > > + unsigned long nr) > > { > > - percpu_ref_get(&objcg->refcnt); > > + if (!obj_cgroup_is_root(objcg)) > > + percpu_ref_get_many(&objcg->refcnt, nr); > > } > > > > -static inline void obj_cgroup_get_many(struct obj_cgroup *objcg, > > - unsigned long nr) > > +static inline void obj_cgroup_get(struct obj_cgroup *objcg) > > { > > - percpu_ref_get_many(&objcg->refcnt, nr); > > + obj_cgroup_get_many(objcg, 1); > > } > > > > static inline void obj_cgroup_put(struct obj_cgroup *objcg) > > { > > - if (objcg) > > + if (objcg && !obj_cgroup_is_root(objcg)) > > percpu_ref_put(&objcg->refcnt); > > } > > > > @@ -1101,6 +1110,11 @@ static inline bool mem_cgroup_is_root(struct mem= _cgroup *memcg) > > return true; > > } > > > > +static inline bool obj_cgroup_is_root(const struct obj_cgroup *objcg) > > +{ > > + return true; > > +} > > + > > static inline bool mem_cgroup_disabled(void) > > { > > return true; > > @@ -1684,8 +1698,7 @@ static inline struct obj_cgroup *get_obj_cgroup_f= rom_current(void) > > { > > struct obj_cgroup *objcg =3D current_obj_cgroup(); > > > > - if (objcg) > > - obj_cgroup_get(objcg); > > + obj_cgroup_get(objcg); > > > > return objcg; > > } > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > > index a6362d11b46c..4aadc1b87db3 100644 > > --- a/mm/memcontrol.c > > +++ b/mm/memcontrol.c > > @@ -81,6 +81,7 @@ struct cgroup_subsys memory_cgrp_subsys __read_mostly= ; > > EXPORT_SYMBOL(memory_cgrp_subsys); > > > > struct mem_cgroup *root_mem_cgroup __read_mostly; > > +struct obj_cgroup *root_obj_cgroup __read_mostly; > > > > /* Active memory cgroup to use from an interrupt context */ > > DEFINE_PER_CPU(struct mem_cgroup *, int_active_memcg); > > @@ -2525,15 +2526,14 @@ struct mem_cgroup *mem_cgroup_from_slab_obj(voi= d *p) > > > > static struct obj_cgroup *__get_obj_cgroup_from_memcg(struct mem_cgrou= p *memcg) > > { > > - struct obj_cgroup *objcg =3D NULL; > > + for (; memcg; memcg =3D parent_mem_cgroup(memcg)) { > > + struct obj_cgroup *objcg =3D rcu_dereference(memcg->objcg= ); > > > > - for (; !mem_cgroup_is_root(memcg); memcg =3D parent_mem_cgroup(me= mcg)) { > > - objcg =3D rcu_dereference(memcg->objcg); > > if (likely(objcg && obj_cgroup_tryget(objcg))) > > - break; > > - objcg =3D NULL; > > + return objcg; > > } > > - return objcg; > > + > > + return NULL; > > } > > > > It appears that the return NULL statement might be dead code in this > context. And would it be preferable to use return root_obj_cgroup instead= ? I do not think so. The parameter of @memcg could be NULL passed from current_objcg_update(). Returning NULL in this case makes sense to me. It is not reasonable to return root_obj_cgroup for a NULL memcg for me. Muchun, Thanks. > > Best regards, > Ridong > > > static struct obj_cgroup *current_objcg_update(void) > > @@ -2604,18 +2604,17 @@ __always_inline struct obj_cgroup *current_obj_= cgroup(void) > > * Objcg reference is kept by the task, so it's safe > > * to use the objcg by the current task. > > */ > > - return objcg; > > + return objcg ? : root_obj_cgroup; > > } > > > > memcg =3D this_cpu_read(int_active_memcg); > > if (unlikely(memcg)) > > goto from_memcg; > > > > - return NULL; > > + return root_obj_cgroup; > > > > from_memcg: > > - objcg =3D NULL; > > - for (; !mem_cgroup_is_root(memcg); memcg =3D parent_mem_cgroup(me= mcg)) { > > + for (; memcg; memcg =3D parent_mem_cgroup(memcg)) { > > /* > > * Memcg pointer is protected by scope (see set_active_me= mcg()) > > * and is pinning the corresponding objcg, so objcg can't= go > > @@ -2624,10 +2623,10 @@ __always_inline struct obj_cgroup *current_obj_= cgroup(void) > > */ > > objcg =3D rcu_dereference_check(memcg->objcg, 1); > > if (likely(objcg)) > > - break; > > + return objcg; > > } > > > > - return objcg; > > + return root_obj_cgroup; > > } > > > > struct obj_cgroup *get_obj_cgroup_from_folio(struct folio *folio) > > @@ -2641,14 +2640,8 @@ struct obj_cgroup *get_obj_cgroup_from_folio(str= uct folio *folio) > > objcg =3D __folio_objcg(folio); > > obj_cgroup_get(objcg); > > } else { > > - struct mem_cgroup *memcg; > > - > > rcu_read_lock(); > > - memcg =3D __folio_memcg(folio); > > - if (memcg) > > - objcg =3D __get_obj_cgroup_from_memcg(memcg); > > - else > > - objcg =3D NULL; > > + objcg =3D __get_obj_cgroup_from_memcg(__folio_memcg(folio= )); > > rcu_read_unlock(); > > } > > return objcg; > > @@ -2733,7 +2726,7 @@ int __memcg_kmem_charge_page(struct page *page, g= fp_t gfp, int order) > > int ret =3D 0; > > > > objcg =3D current_obj_cgroup(); > > - if (objcg) { > > + if (!obj_cgroup_is_root(objcg)) { > > ret =3D obj_cgroup_charge_pages(objcg, gfp, 1 << order); > > if (!ret) { > > obj_cgroup_get(objcg); > > @@ -3036,7 +3029,7 @@ bool __memcg_slab_post_alloc_hook(struct kmem_cac= he *s, struct list_lru *lru, > > * obj_cgroup_get() is used to get a permanent reference. > > */ > > objcg =3D current_obj_cgroup(); > > - if (!objcg) > > + if (obj_cgroup_is_root(objcg)) > > return true; > > > > /* > > @@ -3708,6 +3701,9 @@ static int mem_cgroup_css_online(struct cgroup_su= bsys_state *css) > > if (!objcg) > > goto free_shrinker; > > > > + if (unlikely(mem_cgroup_is_root(memcg))) > > + root_obj_cgroup =3D objcg; > > + > > objcg->memcg =3D memcg; > > rcu_assign_pointer(memcg->objcg, objcg); > > obj_cgroup_get(objcg); > > @@ -5302,6 +5298,9 @@ void obj_cgroup_charge_zswap(struct obj_cgroup *o= bjcg, size_t size) > > if (!cgroup_subsys_on_dfl(memory_cgrp_subsys)) > > return; > > > > + if (obj_cgroup_is_root(objcg)) > > + return; > > + > > VM_WARN_ON_ONCE(!(current->flags & PF_MEMALLOC)); > > > > /* PF_MEMALLOC context, charging must succeed */ > > @@ -5329,6 +5328,9 @@ void obj_cgroup_uncharge_zswap(struct obj_cgroup = *objcg, size_t size) > > if (!cgroup_subsys_on_dfl(memory_cgrp_subsys)) > > return; > > > > + if (obj_cgroup_is_root(objcg)) > > + return; > > + > > obj_cgroup_uncharge(objcg, size); > > > > rcu_read_lock(); > > diff --git a/mm/percpu.c b/mm/percpu.c > > index b35494c8ede2..3e54c6fca9bd 100644 > > --- a/mm/percpu.c > > +++ b/mm/percpu.c > > @@ -1616,7 +1616,7 @@ static bool pcpu_memcg_pre_alloc_hook(size_t size= , gfp_t gfp, > > return true; > > > > objcg =3D current_obj_cgroup(); > > - if (!objcg) > > + if (obj_cgroup_is_root(objcg)) > > return true; > > > > if (obj_cgroup_charge(objcg, gfp, pcpu_obj_full_size(size))) >