[PATCH 0/2] mm: fix memcg_data reads during a THP split

[PATCH 0/2] mm: fix memcg_data reads during a THP split

[Daejun Park via B4 Relay]

Tao Yi reported an oops in page_cgroup_ino() while reading
/proc/kpagecgroup and traced it to the THP split in an RFC [1]. The
RFC skips frozen folios in the reader. Patch 1 moves the memcg_data
copy in the split instead. Patch 2 orders the same loads in
page_owner and needs patch 1.

Charged large kmalloc pages are frozen, so the RFC would report them
as uncharged. My review of the RFC [2] got no reply. Tested on x86_64
with KASAN, see patch 1.

[1] https://lore.kernel.org/all/20260907124001.1544655-1-escape@deepseek.com/
[2] https://lore.kernel.org/all/20261028040418epcms2p7ac0f584cdfdc6a0ea9b7f20eadeada3d@epcms2p7/

---
Daejun Park (2):
      mm/huge_memory: set memcg_data before a split page leaves the folio
      mm/page_owner: also check PageTail before reading memcg_data

 mm/huge_memory.c | 17 +++++++++++++----
 mm/memcontrol.c  |  9 ++++++++-
 mm/page_owner.c  |  7 +++++++
 3 files changed, 28 insertions(+), 5 deletions(-)
---
base-commit: a13b500946e5dc467976838572b8ddd2fd777ca7
change-id: 20261028-kpagecgroup-split-8a749c6ee27e

Best regards,
-- 
Daejun Park <daejun7.park@samsung.com>

[PATCH 1/2] mm/huge_memory: set memcg_data before a split page leaves the folio

[Daejun Park via B4 Relay]

From: Daejun Park <daejun7.park@samsung.com>

__split_folio_to_order() copies memcg_data to a new folio after
clear_compound_head(). Until then, memcg_data of the first tail page
holds the folio's _nr_pages. page_cgroup_ino() can see that page as a
folio in the window and take _nr_pages for an objcg pointer. Reading
/proc/kpagecgroup as root then oopses while folios are split to
order 0.

Copy memcg_data before the smp_wmb(), as before commit fa23a338de93
("mm: separate folio_split_memcg_refs() from split_page_memcg()"). Add
the paired smp_rmb() to page_cgroup_ino().

Fixes: fa23a338de93 ("mm: separate folio_split_memcg_refs() from split_page_memcg()")
Reported-by: Tao Yi <escape@deepseek.com>
Closes: https://lore.kernel.org/all/20260907124001.1544655-1-escape@deepseek.com/
Cc: stable@vger.kernel.org
Signed-off-by: Daejun Park <daejun7.park@samsung.com>
---
Tested on x86_64 with KASAN (7.3-rc7 and mm-hotfixes-unstable) in an
8-vCPU QEMU guest: six threads read /proc/kpagecgroup for page 1 of 64
THPs that debugfs split_huge_pages splits to order 0. Base oopsed in 6
of 6 runs, 4 of them within 1.5 s. With patch 1, 8 runs of 300 s had
no oops.

Before 7.4, page_cgroup_ino() and folio_memcg_check() take non-const
pointers, so a stable backport needs "struct folio *folio" there. The
hunk applies to those trees without a conflict, and the build then
fails with CONFIG_WERROR=y (-Wdiscarded-qualifiers).
---
 mm/huge_memory.c | 17 +++++++++++++----
 mm/memcontrol.c  |  9 ++++++++-
 2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/mm/huge_memory.c b/mm/huge_memory.c
index 14599d0fff..adc5a47ee7 100644
--- a/mm/huge_memory.c
+++ b/mm/huge_memory.c
@@ -3768,7 +3768,19 @@ static void __split_folio_to_order(struct folio *folio, int old_order,
 		if (folio_test_swapcache(folio))
 			new_folio->swap.val = folio->swap.val + i;
 
-		/* Page flags must be visible before we make the page non-compound. */
+#ifdef CONFIG_MEMCG
+		/*
+		 * In the first tail page, memcg_data still holds
+		 * folio->_nr_pages. Lockless PFN walkers (page_cgroup_ino(),
+		 * page_owner) read memcg_data once the page is not a tail.
+		 */
+		new_folio->memcg_data = folio->memcg_data;
+#endif
+
+		/*
+		 * Page flags and memcg_data must be visible before we make the
+		 * page non-compound.
+		 */
 		smp_wmb();
 
 		/*
@@ -3803,9 +3815,6 @@ static void __split_folio_to_order(struct folio *folio, int old_order,
 			folio_set_young(new_folio);
 		if (folio_test_idle(folio))
 			folio_set_idle(new_folio);
-#ifdef CONFIG_MEMCG
-		new_folio->memcg_data = folio->memcg_data;
-#endif
 
 		folio_xchg_last_cpupid(new_folio, folio_last_cpupid(folio));
 	}
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 84f4a8fd4d..c244d56446 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -375,12 +375,19 @@ struct cgroup_subsys_state *get_mem_cgroup_css_from_folio(const struct folio *fo
  */
 ino_t page_cgroup_ino(const struct page *page)
 {
+	const struct folio *folio;
 	struct mem_cgroup *memcg;
 	unsigned long ino = 0;
 
 	rcu_read_lock();
 	/* page_folio() is racy here, but the entire function is racy anyway */
-	memcg = folio_memcg_check(page_folio(page));
+	folio = page_folio(page);
+	/*
+	 * Order the memcg_data load after the compound_info load in
+	 * page_folio(). Pairs with smp_wmb() in __split_folio_to_order().
+	 */
+	smp_rmb();
+	memcg = folio_memcg_check(folio);
 
 	while (memcg && !css_is_online(&memcg->css))
 		memcg = parent_mem_cgroup(memcg);

-- 
2.43.0

Re: [PATCH 1/2] mm/huge_memory: set memcg_data before a split page leaves the folio

[Zi Yan]

On Wed Nov 4, 2026 at 4:59 AM EST, Daejun Park via B4 Relay wrote:
> From: Daejun Park <daejun7.park@samsung.com>
>
> __split_folio_to_order() copies memcg_data to a new folio after
> clear_compound_head(). Until then, memcg_data of the first tail page
> holds the folio's _nr_pages. page_cgroup_ino() can see that page as a
> folio in the window and take _nr_pages for an objcg pointer. Reading
> /proc/kpagecgroup as root then oopses while folios are split to
> order 0.
>
> Copy memcg_data before the smp_wmb(), as before commit fa23a338de93
> ("mm: separate folio_split_memcg_refs() from split_page_memcg()"). Add
> the paired smp_rmb() to page_cgroup_ino().
>
> Fixes: fa23a338de93 ("mm: separate folio_split_memcg_refs() from split_page_memcg()")
> Reported-by: Tao Yi <escape@deepseek.com>
> Closes: https://lore.kernel.org/all/20260907124001.1544655-1-escape@deepseek.com/
> Cc: stable@vger.kernel.org
> Signed-off-by: Daejun Park <daejun7.park@samsung.com>
> ---
> Tested on x86_64 with KASAN (7.3-rc7 and mm-hotfixes-unstable) in an
> 8-vCPU QEMU guest: six threads read /proc/kpagecgroup for page 1 of 64
> THPs that debugfs split_huge_pages splits to order 0. Base oopsed in 6
> of 6 runs, 4 of them within 1.5 s. With patch 1, 8 runs of 300 s had
> no oops.
>
> Before 7.4, page_cgroup_ino() and folio_memcg_check() take non-const
> pointers, so a stable backport needs "struct folio *folio" there. The
> hunk applies to those trees without a conflict, and the build then
> fails with CONFIG_WERROR=y (-Wdiscarded-qualifiers).
> ---
>  mm/huge_memory.c | 17 +++++++++++++----
>  mm/memcontrol.c  |  9 ++++++++-
>  2 files changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 14599d0fff..adc5a47ee7 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -3768,7 +3768,19 @@ static void __split_folio_to_order(struct folio *folio, int old_order,
>  		if (folio_test_swapcache(folio))
>  			new_folio->swap.val = folio->swap.val + i;
>  
> -		/* Page flags must be visible before we make the page non-compound. */
> +#ifdef CONFIG_MEMCG
> +		/*
> +		 * In the first tail page, memcg_data still holds
> +		 * folio->_nr_pages. Lockless PFN walkers (page_cgroup_ino(),
> +		 * page_owner) read memcg_data once the page is not a tail.
> +		 */

How about? Because clear_compound_head() makes the new_folio be
recognized as a folio by reference-free readers.

Assign memcg data before clear_compound_head(), so lockless,
reference-free readers get the right value instead of bogus data from
the overlaid ->_nr_pages in the first tail page.

> +		new_folio->memcg_data = folio->memcg_data;
> +#endif
> +
> +		/*
> +		 * Page flags and memcg_data must be visible before we make the
> +		 * page non-compound.
> +		 */
>  		smp_wmb();
>  
>  		/*
> @@ -3803,9 +3815,6 @@ static void __split_folio_to_order(struct folio *folio, int old_order,
>  			folio_set_young(new_folio);
>  		if (folio_test_idle(folio))
>  			folio_set_idle(new_folio);
> -#ifdef CONFIG_MEMCG
> -		new_folio->memcg_data = folio->memcg_data;
> -#endif
>  
>  		folio_xchg_last_cpupid(new_folio, folio_last_cpupid(folio));
>  	}
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 84f4a8fd4d..c244d56446 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -375,12 +375,19 @@ struct cgroup_subsys_state *get_mem_cgroup_css_from_folio(const struct folio *fo
>   */
>  ino_t page_cgroup_ino(const struct page *page)
>  {
> +	const struct folio *folio;
>  	struct mem_cgroup *memcg;
>  	unsigned long ino = 0;
>  
>  	rcu_read_lock();
>  	/* page_folio() is racy here, but the entire function is racy anyway */
> -	memcg = folio_memcg_check(page_folio(page));
> +	folio = page_folio(page);
> +	/*
> +	 * Order the memcg_data load after the compound_info load in
> +	 * page_folio(). Pairs with smp_wmb() in __split_folio_to_order().
> +	 */
> +	smp_rmb();
> +	memcg = folio_memcg_check(folio);
>  
>  	while (memcg && !css_is_online(&memcg->css))
>  		memcg = parent_mem_cgroup(memcg);

Otherwise, LGTM. Thanks.

Reviewed-by: Zi Yan <ziy@nvidia.com>


-- 
Best Regards,
Yan, Zi

RE:(2) [PATCH 1/2] mm/huge_memory: set memcg_data before a split page leaves the folio

[Daejun Park]

On Wed Nov 4, 2026 at 11:33 AM EST, Zi Yan wrote:
> Assign memcg data before clear_compound_head(), so lockless,
> reference-free readers get the right value instead of bogus data from
> the overlaid ->_nr_pages in the first tail page.

Thanks, agreed. Your wording says why the assignment has to come before
clear_compound_head(), which my comment left implicit.

Thanks,
Daejun

[PATCH 2/2] mm/page_owner: also check PageTail before reading memcg_data

[Daejun Park via B4 Relay]

From: Daejun Park <daejun7.park@samsung.com>

print_page_owner_memcg() loads memcg_data before it tests PageTail().
In the first tail page, memcg_data holds the folio's _nr_pages. A split
can make the page a folio between the two loads. The stale _nr_pages
then passes the PageTail() test and is used as an objcg pointer.

__split_folio_to_order() stores memcg_data before the smp_wmb() that
precedes clear_compound_head(). Also test PageTail() before the load,
with a paired smp_rmb(). Keep the test after the load for a page that
becomes a tail in between.

Fixes: 4996fc547f5b ("mm: let _folio_nr_pages overlay memcg_data in first tail page")
Signed-off-by: Daejun Park <daejun7.park@samsung.com>
---
Needs patch 1 for the store order. Found by reading the code, not
reproduced. The page_owner oops seen with a delay after
clear_compound_head() is the window patch 1 closes.
---
 mm/page_owner.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/mm/page_owner.c b/mm/page_owner.c
index cfc31c92d7..058d4dec84 100644
--- a/mm/page_owner.c
+++ b/mm/page_owner.c
@@ -564,6 +564,13 @@ static inline int print_page_owner_memcg(char *kbuf, size_t count, int ret,
 	char name[80];
 
 	rcu_read_lock();
+	if (PageTail(page))
+		goto out_unlock;
+	/*
+	 * Order the memcg_data load after the PageTail() load. Pairs with
+	 * smp_wmb() in __split_folio_to_order().
+	 */
+	smp_rmb();
 	memcg_data = READ_ONCE(page->memcg_data);
 	if (!memcg_data || PageTail(page))
 		goto out_unlock;

-- 
2.43.0

Re: [PATCH 2/2] mm/page_owner: also check PageTail before reading memcg_data

[Zi Yan]

On Wed Nov 4, 2026 at 4:59 AM EST, Daejun Park via B4 Relay wrote:
> From: Daejun Park <daejun7.park@samsung.com>
>
> print_page_owner_memcg() loads memcg_data before it tests PageTail().
> In the first tail page, memcg_data holds the folio's _nr_pages. A split
> can make the page a folio between the two loads. The stale _nr_pages
> then passes the PageTail() test and is used as an objcg pointer.
>
> __split_folio_to_order() stores memcg_data before the smp_wmb() that
> precedes clear_compound_head(). Also test PageTail() before the load,
> with a paired smp_rmb(). Keep the test after the load for a page that
> becomes a tail in between.
>
> Fixes: 4996fc547f5b ("mm: let _folio_nr_pages overlay memcg_data in first tail page")
> Signed-off-by: Daejun Park <daejun7.park@samsung.com>
> ---
> Needs patch 1 for the store order. Found by reading the code, not
> reproduced. The page_owner oops seen with a delay after
> clear_compound_head() is the window patch 1 closes.
> ---
>  mm/page_owner.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>

Makes sense.

Reviewed-by: Zi Yan <ziy@nvidia.com>

-- 
Best Regards,
Yan, Zi