From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-yx2-f1.google.com (mail-yx2-f1.google.com [74.125.224.129]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id B8CE0330D29 for ; Wed, 9 Sep 2026 18:21:11 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=74.125.224.129 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1788978074; cv=none; b=KvcN0AyI/chG15sNYIuO99+j6+UamxRXBYBZ57Kg+TfH4059LsVw5spMRDT847lq2aRA/b3BnghstXaZIxWPs3A3t8Wp+Omn1FUdVyrGEZJaDVRsYTm34qgY45ql8/oFQQsu2ZyUqH3Sn87sSDvYHRrjYLT3u1+vYjAPZeVECCM= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1788978074; c=relaxed/simple; bh=C+OpCeT5gS8c3fbfvJQiNaFeaqhR2+5vFvUmbRntlSk=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=foxH4PSVl6zewvIXn6v+szPyDdzY5P3KokAGuW+ok/L3Le0G2j6OWuizOp+larhDnrs8W8HDsNNLJ8GbEkf53Vswg8j73snNHySMyGQkX3HQug+ovTukZMJtQr/tGeKqIN1mfOEsrgY1hmytrb75LTtw9gwfq74mIUts8yY/PTw= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=cmpxchg.org; spf=pass smtp.mailfrom=cmpxchg.org; dkim=pass (2048-bit key) header.d=cmpxchg.org header.i=@cmpxchg.org header.b=pfuiC65T; arc=none smtp.client-ip=74.125.224.129 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=cmpxchg.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=cmpxchg.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=cmpxchg.org header.i=@cmpxchg.org header.b="pfuiC65T" Received: by mail-yx2-f1.google.com with SMTP id 956f58d0204a3-66d27631526so2021122d50.1 for ; Wed, 09 Sep 2026 11:21:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=cmpxchg.org; s=google; t=1788978070; x=1789582870; darn=vger.kernel.org; h=in-reply-to:content-disposition:content-type:mime-version :references:message-id:subject:cc:to:from:date:from:to:cc:subject :date:message-id:reply-to:content-type; bh=CUMCov5LhTuYJkVeQH8wQuxKR0Qh1nB1q0JH2zRc7+s=; b=pfuiC65TobxFZbLAXgQfD2CK5cMzsd+fV+skQhg0AAw9ZrPdn7x9HMGHN3JtFD11Oa 2VuuKObfrGQZoXnc5UwFbkCnS1o5ZRCIo32YT46rzsWI/v5t9Ry2w1Gd9OAGDqNHlOBy b+MWIN0MAG4xqybTh12fmfpr4y5y89FM7mDq8FtEp0HhL9Z4WA0qkV5eLC5u3UUQgvCN YSUuqoRH+BTz2qeSgR1kmncHSCeY9G/XXlGAOfJsu4VF02LVJrwIpfvSlKoLfVBRJmfw 7iDQ9VSjHfeUuinGWLo2oubUyAiRUpOs4LY1mUyO07I3V3i0fEF7G/I2jK1lBtcdDXw6 Rb2Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1788978070; x=1789582870; h=in-reply-to:content-disposition:content-type:mime-version :references:message-id:subject:cc:to:from:date:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to :content-type; bh=CUMCov5LhTuYJkVeQH8wQuxKR0Qh1nB1q0JH2zRc7+s=; b=KWH8YWlBRmkVklVKW59bHbXOUIvnGQ3nYPEhnPHJwy/kT00alFv6AtK2F+xnSQ4K2f H9rAQHSGCWDP8KSonQh8KBBVKgvR1qv0gInYpxrfoVtWDjb5mb9XPGqow6hUmMvqMfvX jI4+w3CYLZ2sO6F03Jm08uPKH8Wmzw11xuKctP+2KU2+FhI39f+dVpZCvBw7vxUzAnkt SnAUJxEMNPQPThs0i9oL/J3oGLA207kXWTlxXhC29VqOBJ+oz4XRRW14dONn/xuYl885 AwtL0pPD+3ihAubvM2cKRcRk3gWX6x9MwwrUUNWtOs4oTKqoCoD6SpOmQvn/+SISbcdT QQmA== X-Forwarded-Encrypted: i=1; AKwUvByvApBSG1QNEvt0D6W/v2kdXj4IodXggpCkw3q+1cKC07QqIPH+63aL3e4btl/20x4eWoDDpchZLNhzOVc=@vger.kernel.org X-Gm-Message-State: AFuF++kXy2r1IYTOWjnUVSN6WS12iv6prk1d0qi7qSmmL5qGKdk6x1/3 qp9xujx3V5+3/1Jl6i/GtWQT4hOsnNBXH1tgUZjXGyctT264DY5rJb0EVXC33yEKJRI= X-Gm-Gg: AYBFou3XfcT+kudafpTafa6qekc1G2g9qk35hg/k7SoAk7g0jnz2UOkWV+asshpGxip ozRju89J0+8ZL77EUN5kNTJ4qqs+Z6st/kyuK5ynwkk3BPJ6VGGlsYqEESAYbI15zviSbCvM+nv BA1Eo4VGTPaWPktjDg+jJTdwNz4+dSMaYne4VRNAGbANWhZRVrKRjWEuVf0Wr9fs3ciWcWT/HXy iipT586+uQVvlHN9buyS8EOs9ltkUKJf/VPXpC8nHC1bHnuWzzb+rTIsmJ1mrK1Pg+D/wdVw41t i1zKLpHTXfjoU4l/NtIQQ48BHZoEMUFKIan8oqHHitSXZg8xOMtuV4O+pCWFkc88tTxW3mzWIAD IL5vrSqVdW8pMqoaYVnQDZ4cDPVblTRXpVcIQ1kGwYwuV1bkjqJEJQCiHYARHMi8vW2b79/ncdy u71PCbPc2Djk3s9OEH9zXLCJb09zFM9Tv7NTLU2FXL1b/qtZLyVIpWPcjqYmmVmVz+c0Tr8zc= X-Received: by 2002:a53:e447:0:b0:66f:c1bc:c081 with SMTP id 956f58d0204a3-66fc1bcc626mr7160601d50.73.1788978070364; Wed, 09 Sep 2026 11:21:10 -0700 (PDT) Received: from localhost ([2603:7001:f100:500:365a:60ff:fe62:ff29]) by smtp.gmail.com with ESMTPSA id 6a1803df08f44-910406871b6sm152808526d6.32.2026.09.09.11.21.08 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 09 Sep 2026 11:21:08 -0700 (PDT) Date: Wed, 9 Sep 2026 14:21:04 -0400 From: Johannes Weiner To: Qinyun Tan Cc: Andrew Morton , Michal Hocko , Roman Gushchin , Shakeel Butt , Muchun Song , Michal =?iso-8859-1?Q?Koutn=FD?= , David Hildenbrand , Zi Yan , Baolin Wang , Usama Arif , Dave Chinner , Qi Zheng , Yosry Ahmed , Nhat Pham , Chengming Zhou , Xunlei Pang , cgroups@vger.kernel.org, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 1/4] mm: memcontrol: drop kmemcg_id and use the memcg ID for list_lru indexing Message-ID: References: <20260907110111.2286932-1-qinyuntan@linux.alibaba.com> <20260907110111.2286932-2-qinyuntan@linux.alibaba.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260907110111.2286932-2-qinyuntan@linux.alibaba.com> On Mon, Sep 07, 2026 at 07:01:08PM +0800, Qinyun Tan wrote: > kmemcg_id is a copy of the memcg ID assigned in memcg_online_kmem(), > and is only used as the list_lru xarray index. With > cgroup.memory=nokmem the assignment never happens, so every memcg > resolves to the per-node lists. The next patch needs the index to > work under nokmem as well, so drop the copy and use the memcg ID. > > The ID works just as well as the copy did: root and NULL still > return -1 and use the per-node lists, and the ID is only released > after the list_lru reparenting, so a stale or recycled ID can never > reach a live list_lru entry. > > The early return of memcg_offline_kmem() under nokmem is dropped as > well, so the reparenting also covers lrus that stay memcg aware > without kmem accounting. > > Signed-off-by: Qinyun Tan > --- > include/linux/memcontrol.h | 8 +++++--- > mm/list_lru.c | 10 +++++----- > mm/memcontrol.c | 6 ------ > 3 files changed, 10 insertions(+), 14 deletions(-) > > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h > index fdf4812e1d818..edeb287978934 100644 > --- a/include/linux/memcontrol.h > +++ b/include/linux/memcontrol.h > @@ -254,7 +254,6 @@ struct mem_cgroup { > #if BITS_PER_LONG < 64 > seqlock_t socket_pressure_seqlock; > #endif > - int kmemcg_id; > > #ifdef CONFIG_CGROUP_WRITEBACK > struct list_head cgwb_list; > @@ -1775,12 +1774,15 @@ static inline void memcg_kmem_uncharge_page(struct page *page, int order) > } > > /* > - * A helper for accessing memcg's kmem_id, used for getting > + * A helper for accessing the memcg ID, used for getting > * corresponding LRU lists. > */ > static inline int memcg_kmem_id(struct mem_cgroup *memcg) > { > - return memcg ? memcg->kmemcg_id : -1; > + if (!memcg || mem_cgroup_is_root(memcg)) > + return -1; > + > + return memcg->id.id; > } This is a private ID with lifetime only guaranteed for online groups. Reparenting happens right before it dies at offlining right now, but this is not a great dependency to have. Use mem_cgroup_id() instead and just get rid of that helper. > struct mem_cgroup *mem_cgroup_from_virt(void *p); > diff --git a/mm/list_lru.c b/mm/list_lru.c > index a4522ca93ebcb..6fd4e9af84396 100644 > --- a/mm/list_lru.c > +++ b/mm/list_lru.c > @@ -502,7 +502,7 @@ static void memcg_reparent_list_lru_one(struct list_lru *lru, int nid, > struct list_lru_one *src, > struct mem_cgroup *dst_memcg) > { > - int dst_idx = dst_memcg->kmemcg_id; > + int dst_idx = memcg_kmem_id(dst_memcg); > struct list_lru_one *dst; > > spin_lock_irq(&src->lock); > @@ -536,7 +536,7 @@ void memcg_reparent_list_lrus(struct mem_cgroup *memcg, struct mem_cgroup *paren > * allocating a new mlru since CSS_DYING is already set for this > * memcg a rcu grace period ago. > */ > - mlru = xa_load(&lru->xa, memcg->kmemcg_id); > + mlru = xa_load(&lru->xa, memcg_kmem_id(memcg)); > if (!mlru) > continue; > > @@ -551,7 +551,7 @@ void memcg_reparent_list_lrus(struct mem_cgroup *memcg, struct mem_cgroup *paren > for_each_node(i) > memcg_reparent_list_lru_one(lru, i, &mlru->node[i], parent); > > - xa_erase_irq(&lru->xa, memcg->kmemcg_id); > + xa_erase_irq(&lru->xa, memcg_kmem_id(memcg)); > > /* > * Here all list_lrus corresponding to the cgroup are guaranteed > @@ -566,7 +566,7 @@ void memcg_reparent_list_lrus(struct mem_cgroup *memcg, struct mem_cgroup *paren > static inline bool memcg_list_lru_allocated(struct mem_cgroup *memcg, > struct list_lru *lru) > { > - int idx = memcg->kmemcg_id; > + int idx = memcg_kmem_id(memcg); > > return idx < 0 || xa_load(&lru->xa, idx); > } > @@ -602,7 +602,7 @@ static int __memcg_list_lru_alloc(struct mem_cgroup *memcg, > if (!mlru) > return -ENOMEM; > } > - xas_set(&xas, pos->kmemcg_id); > + xas_set(&xas, memcg_kmem_id(pos)); > do { > xas_lock_irqsave(&xas, flags); > if (!xas_load(&xas) && !css_is_dying(&pos->css)) { > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 7ce50bccf1264..619d4c1f2e8f2 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -3780,17 +3780,12 @@ static void memcg_online_kmem(struct mem_cgroup *memcg) > return; > > static_branch_enable(&memcg_kmem_online_key); > - > - memcg->kmemcg_id = memcg->id.id; > } > > static void memcg_offline_kmem(struct mem_cgroup *memcg) > { > struct mem_cgroup *parent; > > - if (mem_cgroup_kmem_disabled()) > - return; > - > if (unlikely(mem_cgroup_is_root(memcg))) > return; Both of these functions do very little now and the asymmetry you're adding on the mem_cgroup_kmem_disabled() check looks odd. Please just inline them into mem_cgroup_css_online()/offline(): onlining: if (!mem_cgroup_kmem_disabled() && likely(!mem_cgroup_is_root())) static_branch_enable(&memcg_kmem_online_key); offlining: memcg_reparent_list_lrus(memcg, parent); The root check is unnecessary because roots are not destroyed. But if you'd rather not make that change here, keep the root check, and leave its removal to a separate cleanup patch, that's fine too.