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

["Zi Yan" <ziy@nvidia.com>]

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