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 386E4C2BBCA for ; Tue, 25 Jun 2024 07:19:10 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id BC4536B033A; Tue, 25 Jun 2024 03:19:09 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id B75106B033B; Tue, 25 Jun 2024 03:19:09 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id A15806B033C; Tue, 25 Jun 2024 03:19:09 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0013.hostedemail.com [216.40.44.13]) by kanga.kvack.org (Postfix) with ESMTP id 82BF76B033A for ; Tue, 25 Jun 2024 03:19:09 -0400 (EDT) Received: from smtpin22.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay04.hostedemail.com (Postfix) with ESMTP id 254A51A185E for ; Tue, 25 Jun 2024 07:19:09 +0000 (UTC) X-FDA: 82268559618.22.D2463B2 Received: from mail-ej1-f43.google.com (mail-ej1-f43.google.com [209.85.218.43]) by imf26.hostedemail.com (Postfix) with ESMTP id 26A32140006 for ; Tue, 25 Jun 2024 07:19:06 +0000 (UTC) Authentication-Results: imf26.hostedemail.com; dkim=pass header.d=suse.com header.s=google header.b="SuYB/eUv"; spf=pass (imf26.hostedemail.com: domain of mhocko@suse.com designates 209.85.218.43 as permitted sender) smtp.mailfrom=mhocko@suse.com; dmarc=pass (policy=quarantine) header.from=suse.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1719299936; 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: in-reply-to:in-reply-to:references:references:dkim-signature; bh=Oa1dXwsTDyXpTpChTur8Cwz6bqWTxj/Pfd0DAoAHav4=; b=r0jsL18C4jh9lqs1Un1NI0UN6gUyTdYOQoapJDUyYRJHWAJl99ODzqq/A/SJobrObVI6c2 DHe4Zb4n6SoVvzTJsoF/jGqVAj2CYrHQNG6YWspX7enQwtd8uYcIqwjc0b0ffj935N6nTF L0l5WzL9hWNwjxkTpO7yjGx0rqmgjIs= ARC-Authentication-Results: i=1; imf26.hostedemail.com; dkim=pass header.d=suse.com header.s=google header.b="SuYB/eUv"; spf=pass (imf26.hostedemail.com: domain of mhocko@suse.com designates 209.85.218.43 as permitted sender) smtp.mailfrom=mhocko@suse.com; dmarc=pass (policy=quarantine) header.from=suse.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1719299936; a=rsa-sha256; cv=none; b=tpHpQK7bIDVZLFLa8lZ1Sw7sYCQ6d1X010mj+jujZPFfEexm+AzCu39obU/Ie76AS1qsgw rEd87ymt7nYYNOfMX6aFle2eGLr+1e6wDRibO7x2De4JhDXUrXLOC15iClUdjL9M1mzzGv By+7XsLPtnCgaFuETy+qi/SLNA03RJY= Received: by mail-ej1-f43.google.com with SMTP id a640c23a62f3a-a724b3a32d2so243766066b.2 for ; Tue, 25 Jun 2024 00:19:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=google; t=1719299945; x=1719904745; darn=kvack.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=Oa1dXwsTDyXpTpChTur8Cwz6bqWTxj/Pfd0DAoAHav4=; b=SuYB/eUvrvJu51Huz+nrowqIFkKiaWkxyEhjbp/BYHB04HAZGgmY/y99fNFJNMaveI R9r5nL0bI/GezahK3UhG6dDqV/wFwuPn9/ZXm1nypjwJve81ddAWhcb736FzIg0/KjF/ PfVWVwFL+kLGr0FOVj0mHmh4JgpBjpgvj9PSm0slfuWgF7esHwMoD4HcS8wWi+ocTDeL vx1WUdKH64WKK+ZEDqh8tNhaIVuIC0jsT5M6V6PLkaIiwLNqQpVGQujcFHSwMgEIyfbV pj9ka8dbRXb93J52QHtk8P2XnKPi28w+2OrniSJEV6tnzg9aNAKmEX2Qdl3XvbVB5oOk Aq6g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1719299945; x=1719904745; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=Oa1dXwsTDyXpTpChTur8Cwz6bqWTxj/Pfd0DAoAHav4=; b=l199wiFfjlI5O8/Ex6EbroP9SPixioGhY82kLp1OnjrwYtQdXIV269flK/kdhV1mN3 DjBP8UXYTQwKBf/HFK7RhUyxjvzxcKcHBG3Uj1CVHI+iM+ohQmbpcM/znRarqQ8mGURb sC2T7es/bSxBcy8AoHQ099GUsPTw6kPi6Fo+si1b/GBA7VDDHSpV2FJXfwdB+zogCt0f kOh1J4Outn/syGg4K9Yj60e/8laVkwPjI1BnIE8apg8yiH4sJakMZyT2Sat/ww1vJxi5 tnhVcijp5yy5qJ3/rv0F5HdE4XNyyRZBXf1+B+R9AnHLsWzqURKsExBKY/Z0gXbAD+VY RhhQ== X-Forwarded-Encrypted: i=1; AJvYcCU352IJrPw+eSHLiOtdelTv2SVGZWFePY/65ZZygsvqQgsOlpdY4KSxfwbC1uHKt5nyQajJBtG2yFM+qqbNp+rtaqc= X-Gm-Message-State: AOJu0YzVDuf65bhMqzRbbD7EPwJxBngrA5sVrTUIjl7R6G97+9ingNFa bUKYGmUlRhKeJQ+rUlW+yDWM6o9CihHQ/AqKrKQnWzk2fkbTGSliARrL7wLbXdc= X-Google-Smtp-Source: AGHT+IHXi0A2g6VkORAwIDvbqiLAFMyUcih2zGBTlHUq4ZwB9wc0AmGajmkQoxOqW28NkDbWoVACbA== X-Received: by 2002:a17:906:aace:b0:a6f:4b4a:588e with SMTP id a640c23a62f3a-a7245b80dfbmr363665766b.34.1719299945448; Tue, 25 Jun 2024 00:19:05 -0700 (PDT) Received: from localhost (109-81-95-13.rct.o2.cz. [109.81.95.13]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-a6fe03b4528sm377057866b.206.2024.06.25.00.19.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 25 Jun 2024 00:19:05 -0700 (PDT) Date: Tue, 25 Jun 2024 09:19:04 +0200 From: Michal Hocko To: Roman Gushchin Cc: Andrew Morton , Johannes Weiner , Shakeel Butt , Muchun Song , linux-kernel@vger.kernel.org, cgroups@vger.kernel.org, linux-mm@kvack.org Subject: Re: [PATCH v2 13/14] mm: memcg: put cgroup v1-related members of task_struct under config option Message-ID: References: <20240625005906.106920-1-roman.gushchin@linux.dev> <20240625005906.106920-14-roman.gushchin@linux.dev> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240625005906.106920-14-roman.gushchin@linux.dev> X-Stat-Signature: mxdffnpkumapka8het5sf5mk8a3pahcc X-Rspam-User: X-Rspamd-Queue-Id: 26A32140006 X-Rspamd-Server: rspam02 X-HE-Tag: 1719299946-784342 X-HE-Meta: U2FsdGVkX1+tEQnRTpWbXTdte4UyAJNoBLWHFED9b7JS758KvhM3rTY7cQttroUpb6XuX6VoAMwEbJIE6k2DFjqDIVpr+3nAbEtOS6XNvIpjogHyMyBxBasicX16/O4m4MY2l5OUk0p4wHM4bcfls2hcdki79l43mcaNEjuLVIGRqaGOGkRIhKc3Cu7DNbwIgOkZttqVUjIClVJ477MKTGRIG2vGJpuL1xBjeMdfKuLUfEHUufZ6noD0ZRfGzQpsFQKKmU0cmqBx49LZreDGbS0Yddyb54o0y/iNfBkN/JLt0GGClKj7iF3jUcZ7C5qGupsLM5VmCYtXkK2RcyKDu9vXDEbzqcF7noBmkPTRf/fDHjGGvgj8BjH7mjEm/Cv9Wu0DwJXsgJLCLqKjpMb/Ad6X2mJJUWwt/Ai54goli0AEcSTViY4QQB2HPy9xeUFf8U564dB1GO29XhfZVlvYbyrGCnE63M9YBXAgCW2ctpDySt0wZ9O1yDsnbql80oQQGzlQHPTsRw9ft1Igr1pdC3aqiLVCIqql2QtlNhg+Z26bxlzQyP/nWVDzTzj3Z0qmIaCtz8M/naVlGvYnbGih59Syj5eb1YWLcfX6DeggdZn76YaF0E7+fA30Bv+z1IRzBzdHexNPEtqSJKrNqFcYDxMqiYQZW+o8WCUD1R4nwBPeXXOlcMicdeIHBRdAc0f+o5ncYBXdU8tIEbRbhOiqDe06BFsUXjwzhNnurTY7UFhAcyDGUeLs/ND/5FLvbGV6GygkXuPBZsA5wseP1V2EBPKcHslFmuZCVs5vq8g8U8PALkEGznJBoEKsfxxv1P51t25PZmsg1xxGv8BIh4QI4ZfrUNBJbvExp0NFlp/b0Uysl2f/GTPAIVGFXD/obAQ6THvEpjswDXd27nxi4vJSiHmNfzhfelNSXlF/Rr84P/ikArWLEY34Ljx23PwPeGhHZBr68h5YZDdbffkbkYb WY6DpGOI xqA7NLciU+teocpuW+MDB+2Roa0BT9DMyXaVSeDSUvgJCSuAh2h8ln3q77aYqFo0H7rl3QuFcO2scSkVl0Zwz7ZK9V6S+//wEifBgJJWnjjTfpIklIXseh8jn/MNINMCx2GvJT238u20Lv/Ty5PyUxAORSiVhOnCuaVrgSA8Zt3DlJcZakXSGYK0t6FPWiV1boCnlVbvOIsW32pv9S0NLxjuNuBfEC/hd/KVq3DE1tcFidjo8Qx+hx27GMtDqP4NtEIw491gum5Kq3EtOLIQa/VoB/tTkXFyGKj1Ep534IGLlhJgxvCXYb1o6H+iFS5QFxymDXR6nHoFmu6IZZ28uyhwRrRrdfYtxTPd3QeT1HVrmyJGMczPS7wuxGmyaye5COE08VvORoP2r59s= 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 Mon 24-06-24 17:59:05, Roman Gushchin wrote: > Guard cgroup v1-related members of task_struct under the CONFIG_MEMCG_V1 > config option, so that users who adopted cgroup v2 don't have to waste > the memory for fields which are never accessed. This patch does more than that, right? It is essentially making the whole v1 code conditional. Please change the wording accordingly. I also think we should make it more clear when to enable the option. I would propose the following for the config option help text: Legacy cgroup v1 memory controller which has been deprecated by cgroup v2 implementation. The v1 is there for legacy applications which haven't migrated to the new cgroup v2 interface yet. If you do not have any such application then you are completely fine leaving this option disabled. Please note that feature set of the legacy memory controller is likely going to shrink due to deprecation process. New deployments with v1 controller are highly discouraged. > Signed-off-by: Roman Gushchin With that updated feel free to add Acked-by: Michal Hocko > --- > include/linux/memcontrol.h | 6 +++--- > init/Kconfig | 9 +++++++++ > mm/Makefile | 3 ++- > mm/memcontrol-v1.h | 21 ++++++++++++++++++++- > mm/memcontrol.c | 10 +++++++--- > 5 files changed, 41 insertions(+), 8 deletions(-) > > diff --git a/include/linux/memcontrol.h b/include/linux/memcontrol.h > index a70d64ed04f5..796cfa842346 100644 > --- a/include/linux/memcontrol.h > +++ b/include/linux/memcontrol.h > @@ -1851,7 +1851,7 @@ static inline bool mem_cgroup_zswap_writeback_enabled(struct mem_cgroup *memcg) > > /* Cgroup v1-related declarations */ > > -#ifdef CONFIG_MEMCG > +#ifdef CONFIG_MEMCG_V1 > unsigned long memcg1_soft_limit_reclaim(pg_data_t *pgdat, int order, > gfp_t gfp_mask, > unsigned long *total_scanned); > @@ -1883,7 +1883,7 @@ static inline void mem_cgroup_unlock_pages(void) > rcu_read_unlock(); > } > > -#else /* CONFIG_MEMCG */ > +#else /* CONFIG_MEMCG_V1 */ > static inline > unsigned long memcg1_soft_limit_reclaim(pg_data_t *pgdat, int order, > gfp_t gfp_mask, > @@ -1922,6 +1922,6 @@ static inline bool mem_cgroup_oom_synchronize(bool wait) > return false; > } > > -#endif /* CONFIG_MEMCG */ > +#endif /* CONFIG_MEMCG_V1 */ > > #endif /* _LINUX_MEMCONTROL_H */ > diff --git a/init/Kconfig b/init/Kconfig > index febdea2afc3b..5191b6435b4e 100644 > --- a/init/Kconfig > +++ b/init/Kconfig > @@ -969,6 +969,15 @@ config MEMCG > help > Provides control over the memory footprint of tasks in a cgroup. > > +config MEMCG_V1 > + bool "Legacy memory controller" > + depends on MEMCG > + default n > + help > + Legacy cgroup v1 memory controller. > + > + San N is unsure. > + > config MEMCG_KMEM > bool > depends on MEMCG > diff --git a/mm/Makefile b/mm/Makefile > index 124d4dea2035..d2915f8c9dc0 100644 > --- a/mm/Makefile > +++ b/mm/Makefile > @@ -96,7 +96,8 @@ obj-$(CONFIG_NUMA) += memory-tiers.o > obj-$(CONFIG_DEVICE_MIGRATION) += migrate_device.o > obj-$(CONFIG_TRANSPARENT_HUGEPAGE) += huge_memory.o khugepaged.o > obj-$(CONFIG_PAGE_COUNTER) += page_counter.o > -obj-$(CONFIG_MEMCG) += memcontrol.o memcontrol-v1.o vmpressure.o > +obj-$(CONFIG_MEMCG_V1) += memcontrol-v1.o > +obj-$(CONFIG_MEMCG) += memcontrol.o vmpressure.o > ifdef CONFIG_SWAP > obj-$(CONFIG_MEMCG) += swap_cgroup.o > endif > diff --git a/mm/memcontrol-v1.h b/mm/memcontrol-v1.h > index 89d420793048..64b053d7f131 100644 > --- a/mm/memcontrol-v1.h > +++ b/mm/memcontrol-v1.h > @@ -75,7 +75,7 @@ unsigned long memcg_page_state_local_output(struct mem_cgroup *memcg, int item); > int memory_stat_show(struct seq_file *m, void *v); > > /* Cgroup v1-specific declarations */ > - > +#ifdef CONFIG_MEMCG_V1 > void memcg1_remove_from_trees(struct mem_cgroup *memcg); > > static inline void memcg1_soft_limit_reset(struct mem_cgroup *memcg) > @@ -110,4 +110,23 @@ void memcg1_stat_format(struct mem_cgroup *memcg, struct seq_buf *s); > extern struct cftype memsw_files[]; > extern struct cftype mem_cgroup_legacy_files[]; > > +#else /* CONFIG_MEMCG_V1 */ > + > +static inline void memcg1_remove_from_trees(struct mem_cgroup *memcg) {} > +static inline void memcg1_soft_limit_reset(struct mem_cgroup *memcg) {} > +static inline bool memcg1_wait_acct_move(struct mem_cgroup *memcg) { return false; } > +static inline void memcg1_css_offline(struct mem_cgroup *memcg) {} > + > +static inline bool memcg1_oom_prepare(struct mem_cgroup *memcg, bool *locked) { return true; } > +static inline void memcg1_oom_finish(struct mem_cgroup *memcg, bool locked) {} > +static inline void memcg1_oom_recover(struct mem_cgroup *memcg) {} > + > +static inline void memcg1_check_events(struct mem_cgroup *memcg, int nid) {} > + > +static inline void memcg1_stat_format(struct mem_cgroup *memcg, struct seq_buf *s) {} > + > +extern struct cftype memsw_files[]; > +extern struct cftype mem_cgroup_legacy_files[]; > +#endif /* CONFIG_MEMCG_V1 */ > + > #endif /* __MM_MEMCONTROL_V1_H */ > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index c7341e811945..d2e1f8baeae8 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -4471,18 +4471,20 @@ struct cgroup_subsys memory_cgrp_subsys = { > .css_free = mem_cgroup_css_free, > .css_reset = mem_cgroup_css_reset, > .css_rstat_flush = mem_cgroup_css_rstat_flush, > - .can_attach = memcg1_can_attach, > #if defined(CONFIG_LRU_GEN) || defined(CONFIG_MEMCG_KMEM) > .attach = mem_cgroup_attach, > #endif > - .cancel_attach = memcg1_cancel_attach, > - .post_attach = memcg1_move_task, > #ifdef CONFIG_MEMCG_KMEM > .fork = mem_cgroup_fork, > .exit = mem_cgroup_exit, > #endif > .dfl_cftypes = memory_files, > +#ifdef CONFIG_MEMCG_V1 > + .can_attach = memcg1_can_attach, > + .cancel_attach = memcg1_cancel_attach, > + .post_attach = memcg1_move_task, > .legacy_cftypes = mem_cgroup_legacy_files, > +#endif > .early_init = 0, > }; > > @@ -5653,7 +5655,9 @@ static int __init mem_cgroup_swap_init(void) > return 0; > > WARN_ON(cgroup_add_dfl_cftypes(&memory_cgrp_subsys, swap_files)); > +#ifdef CONFIG_MEMCG_V1 > WARN_ON(cgroup_add_legacy_cftypes(&memory_cgrp_subsys, memsw_files)); > +#endif > #if defined(CONFIG_MEMCG_KMEM) && defined(CONFIG_ZSWAP) > WARN_ON(cgroup_add_dfl_cftypes(&memory_cgrp_subsys, zswap_files)); > #endif > -- > 2.45.2 -- Michal Hocko SUSE Labs