Re: [RFC PATCH 2/2] mm/huge_memory: detect partially mapped shared folios without per-page mapcounts

["Jose A. Perez de Azpillaga" <azpijr@gmail.com>]

On 11/12/26 12:36 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue to consider:
> - [Medium] mm/huge_memory: deferred_split_scan() runs PMD
>   thp_underused() on mTHPs
> --
> 
> commit 2f9d38ddfb170d3656f9925bef1e82e9b94ecc37
> Author: Jose A. Perez de Azpillaga <azpijr@gmail.com>
> 
> mm/huge_memory: detect partially mapped shared folios without per-page mapcounts
> 
> This commit queues shared anonymous folios onto the deferred split list
> without setting PG_partially_mapped when CONFIG_NO_PAGE_MAPCOUNT cannot tell
> at unmap time whether all subpages remain mapped, and walks the rmap in
> deferred_split_scan() to check them.
> 
>> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
>> index 8c4a69c797edd..f80fa028dd340 100644
>> --- a/mm/huge_memory.c
>> +++ b/mm/huge_memory.c
> 
> [ ... ]
> 
>> @@ -4744,6 +4811,15 @@ static unsigned long deferred_split_scan(struct shrinker *shrink,
>>  		list_lru_unlock_irq(lru);
>>  		rcu_read_unlock();
>>  
>> +		if (!folio_test_partially_mapped(folio) &&
>> +		    folio_needs_mapped_check(folio)) {
>> +			if (!folio_trylock(folio))
>> +				goto requeue;
>> +			/* Sets PG_partially_mapped and puts it back on the queue. */
>> +			if (folio_mapped_shared_partially(folio))
>> +				deferred_split_folio(folio, true);
>> +			folio_unlock(folio);
>> +		}
>>  		if (!folio_test_partially_mapped(folio)) {
>>  			/*
>>  			 * See try_to_map_unused_to_zeropage(): we cannot
>> @@ -4752,7 +4828,7 @@ static unsigned long deferred_split_scan(struct shrinker *shrink,
>>  			 */
>>  			if (folio_test_mlocked(folio))
>>  				goto next;
>> -			underused = thp_underused(folio);
>> +			underused = split_underused_thp && thp_underused(folio);
>>  			if (!underused)
>>  				goto next;
>>  		}
> 
> [Severity: Medium]
> Can deferred_split_scan() now call thp_underused() on a sub-PMD mTHP?
> 
> Previously, only map_anon_folio_pmd_nopf() queued folios with
> partially_mapped == false, so any folio reaching the !folio_test_partially_mapped(folio)
> branch was PMD-sized.  __folio_remove_rmap() now queues shared anonymous
> folios of any order > 1 with partially_mapped == false when a partial unmap in
> one process leaves the folio fully mapped in another.  When
> folio_mapped_shared_partially() returns false for such a sub-PMD mTHP,
> execution falls through to thp_underused(folio):
> 
> mm/huge_memory.c:thp_underused() {
>     ...
> 	for (i = 0; i < folio_nr_pages(folio); i++) {
> 		if (pages_identical(folio_page(folio, i), ZERO_PAGE(0))) {
> 			if (++num_zero_pages > khugepaged_max_ptes_none)
> 				return true;
> 		} else {
> 			if (++num_filled_pages >= HPAGE_PMD_NR - khugepaged_max_ptes_none)
> 				return false;
> 		}
> 	}
>     ...
> }
> 
> Because thp_underused() compares against unscaled HPAGE_PMD_NR and
> khugepaged_max_ptes_none thresholds, any khugepaged_max_ptes_none below
> HPAGE_PMD_NR - 1 causes reclaim to scan subpages with pages_identical() even
> when neither threshold can be reached, or to split fully mapped sub-PMD mTHPs
> and increment THP_UNDERUSED_SPLIT_PAGE.  Checking folio_test_pmd_mappable(folio)
> before calling thp_underused(folio) would avoid this.
> 

okay, seems correct. I'll make thp_underused() return false for folios
that are not PMD-mappable in v2 then.

-- 
cheers,
jose a. p-a