On 7/9/24 9:14 AM, Suren Baghdasaryan wrote: > On Mon, Jul 8, 2024 at 2:42 PM Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote: >> >> >> The patch titled >> Subject: mm/memcg: alignment memcg_data define condition >> has been added to the -mm mm-unstable branch. Its filename is >> mm-memcg-alignment-memcg_data-define-condition.patch > > FYI, just posted my objection to this change at > https://lore.kernel.org/all/CAJuCfpEjB8wvodBDB__yR8pF5F3uGMPxue-tap68RYCO0O-K1Q@xxxxxxxxxxxxxx/ > It will break CONFIG_MEM_ALLOC_PROFILING=y && CONFIG_MEMCG=n case. Hi Suren, Thanks a lot for point out the problem! Now I understand why we put page/folio.memcg_data under CONFIG_SLAB_OBJ_EXT. We need the memcg_data member only for SLAB_MATCH(memcg_data, obj_exts), even no CONFIG_MEMCG, isn't it? So for the config logical and SLAB_MATCH needs, could we keep this patch with a fix like the following? The match only checked with both configs enabled. Thanks a lot! Alex diff --git a/mm/slab.h b/mm/slab.h index 3586e6183224..9e7ccaf868e2 100644 --- a/mm/slab.h +++ b/mm/slab.h @@ -97,7 +97,7 @@ struct slab { SLAB_MATCH(flags, __page_flags); SLAB_MATCH(compound_head, slab_cache); /* Ensure bit 0 is clear */ SLAB_MATCH(_refcount, __page_refcount); -#ifdef CONFIG_SLAB_OBJ_EXT +#if defined(CONFIG_SLAB_OBJ_EXT) && defined(CONFIG_MEMCG) SLAB_MATCH(memcg_data, obj_exts); #endif #undef SLAB_MATCH > >> >> This patch will shortly appear at >> https://git.kernel.org/pub/scm/linux/kernel/git/akpm/25-new.git/tree/patches/mm-memcg-alignment-memcg_data-define-condition.patch >> >> This patch will later appear in the mm-unstable branch at >> git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm >> >> Before you just go and hit "reply", please: >> a) Consider who else should be cc'ed >> b) Prefer to cc a suitable mailing list as well >> c) Ideally: find the original patch on the mailing list and do a >> reply-to-all to that, adding suitable additional cc's >> >> *** Remember to use Documentation/process/submit-checklist.rst when testing your code *** >> >> The -mm tree is included into linux-next via the mm-everything >> branch at git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm >> and is updated there every 2-3 working days >> >> ------------------------------------------------------ >> From: "Alex Shi (Tencent)" <alexs@xxxxxxxxxx> >> Subject: mm/memcg: alignment memcg_data define condition >> Date: Mon, 8 Jul 2024 14:32:36 +0800 >> >> commit 21c690a349ba ("mm: introduce slabobj_ext to support slab object >> extensions") changed the folio/page->memcg_data define condition from >> MEMCG to SLAB_OBJ_EXT. The code works well, since config SLAB_OBJ_EXT is >> fold into MEMCG in init/Kconfig. >> >> But many related functions that deal with memcg_data still defined under >> MEMCG instead of SLAB_OBJ_EXT, and FOLIO_MATCH with memcg_data are defined >> under MEMCG too. That looks weird and incorrect with memcg_data raw >> meaning. >> >> So let's put memcg_data under MEMCG config to alignment the definition >> with FOLIO_MATCH and its usage in functions. >> >> Link: https://lkml.kernel.org/r/20240708063236.1096395-1-alexs@xxxxxxxxxx >> Signed-off-by: Alex Shi (Tencent) <alexs@xxxxxxxxxx> >> Cc: Johannes Weiner <hannes@xxxxxxxxxxx> >> Cc: Kees Cook <keescook@xxxxxxxxxxxx> >> Cc: Pasha Tatashin <pasha.tatashin@xxxxxxxxxx> >> Cc: Suren Baghdasaryan <surenb@xxxxxxxxxx> >> Cc: Vlastimil Babka <vbabka@xxxxxxx> >> Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> >> --- >> >> include/linux/mm_types.h | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> --- a/include/linux/mm_types.h~mm-memcg-alignment-memcg_data-define-condition >> +++ a/include/linux/mm_types.h >> @@ -180,7 +180,7 @@ struct page { >> /* Usage count. *DO NOT USE DIRECTLY*. See page_ref.h */ >> atomic_t _refcount; >> >> -#ifdef CONFIG_SLAB_OBJ_EXT >> +#ifdef CONFIG_MEMCG >> unsigned long memcg_data; >> #endif >> >> @@ -343,7 +343,7 @@ struct folio { >> }; >> atomic_t _mapcount; >> atomic_t _refcount; >> -#ifdef CONFIG_SLAB_OBJ_EXT >> +#ifdef CONFIG_MEMCG >> unsigned long memcg_data; >> #endif >> #if defined(WANT_PAGE_VIRTUAL) >> _ >> >> Patches currently in -mm which might be from alexs@xxxxxxxxxx are >> >> mm-memcg-alignment-memcg_data-define-condition.patch >>
