Skip to contentSkip to conversation

mm/memcg: alignment memcg_data define condition

PATCH Superseded [email protected] 2 participants 2 messages Jul 8, 2024 – Jul 9, 2024

Subsystems (2)
Memory management MM core

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

[email protected]

author
#0 lore raw

PATCH 1/1 mm/memcg: alignment memcg_data define condition

From: "Alex Shi (Tencent)" <[email protected]>

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.

Signed-off-by Alex Shi (Tencent) <[email protected]>
---
 include/linux/mm_types.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
include/linux/mm_types.h +2 -2
index ef09c4eef6d3..c067db6fa711 100644
@@ -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)
Signature · 1 line
2.43.0
reviewer
#1 lore raw
Quoted 13 lines · On Sun, Jul 7, 2024 at 11:27 PM <[email protected]> wrote:

From: "Alex Shi (Tencent)" <[email protected]>

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.

Quoted 4 lines

So let's put memcg_data under MEMCG config to alignment the definition with FOLIO_MATCH and its usage in functions.

Signed-off-by Alex Shi (Tencent) <[email protected]>

Hi Alex,
I believe your change breaks this condition:

https://elixir.bootlin.com/linux/v6.10-rc7/source/mm/slab.h#L101

for the cases when CONFIG_SLAB_OBJ_EXT=y but CONFIG_MEMCG=n. page.memcg_data will not be there to match slab.obj_exts.
You can test that by setting CONFIG_MEM_ALLOC_PROFILING=y and CONFIG_MEMCG=n.

Therefore sorry but NAK.

Quoted 30 lines
---
 include/linux/mm_types.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
include/linux/mm_types.h +2 -3
index ef09c4eef6d3..c067db6fa711 100644
@@ -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)
--

2.43.0