Re: [PATCH v3 1/3] mm/huge_memory: scale thp_underused() to the folio size

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] mm/huge_memory: wrong rounding in thp_underused() max_ptes_none scaling
--

commit 56eb0cfccba9e5e4276f2a8250815399987bc848
Author: Joanne Koong <joannelkoong@gmail.com>

mm/huge_memory: scale thp_underused() to the folio size

This commit updates thp_underused() to scale khugepaged_max_ptes_none
proportionally to the number of pages in the folio rather than assuming
the folio has HPAGE_PMD_NR pages.

> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index bd44d58e86845..218259940f24d 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -4613,25 +4613,35 @@ static unsigned long deferred_split_count(struct shrinker *shrink,
>  
>  static bool thp_underused(struct folio *folio)
>  {
> -	int num_zero_pages = 0, num_filled_pages = 0;
> -	int i;
> +	const unsigned long nr_pages = folio_nr_pages(folio);
> +	unsigned long num_zero_pages = 0, num_filled_pages = 0;
> +	unsigned long max_ptes_none = khugepaged_max_ptes_none;
> +	unsigned long i;
>  
> -	if (khugepaged_max_ptes_none == HPAGE_PMD_NR - 1)
> +	if (max_ptes_none == HPAGE_PMD_NR - 1)
>  		return false;
>  
>  	if (folio_contain_hwpoisoned_page(folio))
>  		return false;
>  
> -	for (i = 0; i < folio_nr_pages(folio); i++) {
> +	/*
> +	 * khugepaged_max_ptes_none is a number of pages out of HPAGE_PMD_NR.
> +	 * Scale it to this folio's size (rounding up so that the folio is never
> +	 * split when fewer than the configured fraction of its pages are
> +	 * zero-filled).
> +	 */
> +	max_ptes_none = DIV_ROUND_UP(max_ptes_none * nr_pages, HPAGE_PMD_NR);
> +
> +	for (i = 0; i < nr_pages; i++) {
>  		if (pages_identical(folio_page(folio, i), ZERO_PAGE(0))) {
> -			if (++num_zero_pages > khugepaged_max_ptes_none)
> +			if (++num_zero_pages > max_ptes_none)
>  				return true;

[Severity: Medium]
Should max_ptes_none in thp_underused() be scaled with truncating integer
division rather than DIV_ROUND_UP()?

Because the loop checks a strict inequality:

	if (++num_zero_pages > max_ptes_none)
		return true;

max_ptes_none is the maximum number of zero-filled pages tolerated without
splitting, not the minimum count that triggers a split. Since num_zero_pages
is an integer, the condition that the zero-filled fraction exceeds the
configured PMD fraction (num_zero_pages / nr_pages >
khugepaged_max_ptes_none / HPAGE_PMD_NR) is equivalent to:

	num_zero_pages > (khugepaged_max_ptes_none * nr_pages) / HPAGE_PMD_NR

using truncating division. With truncating division, any folio with at most
the configured fraction of zero-filled pages already satisfies
num_zero_pages <= max_ptes_none and is not split.

Rounding max_ptes_none up with DIV_ROUND_UP() alongside the strict > check
requires floor((khugepaged_max_ptes_none * nr_pages) / HPAGE_PMD_NR) + 2
zero pages to trigger a split whenever the product is not an exact multiple
of HPAGE_PMD_NR, allowing sub-PMD folios that exceed the configured zero-page
fraction to escape splitting in deferred_split_scan() -> thp_underused().

In addition, for any sub-PMD folio (nr_pages < HPAGE_PMD_NR), whenever
khugepaged_max_ptes_none is in the range
[HPAGE_PMD_NR - HPAGE_PMD_NR / nr_pages + 1, HPAGE_PMD_NR - 2] (for example,
481..510 for an order-4 mTHP on 4K base pages where HPAGE_PMD_NR is 512),
DIV_ROUND_UP(max_ptes_none * nr_pages, HPAGE_PMD_NR) evaluates to nr_pages.
In that case, ++num_zero_pages > max_ptes_none can never be true during the
nr_pages iterations, so even a 100% zero-filled folio scans all nr_pages
subpages with pages_identical() and returns false, dropping the folio from
deferred_split_lru without splitting.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261023230006.3759543-1-joannelkoong@gmail.com?part=1