Re: [PATCH v2] mm/huge_memory: fix folio isn't locked in softleaf_to_folio()

[Jinjiang Tu <tujinjiang@huawei.com>]

在 2026/3/18 17:02, David Hildenbrand (Arm) 写道:
> On 3/18/26 02:20, Jinjiang Tu wrote:
>> On arm64 server, we found folio that get from migration entry isn't locked
>> in softleaf_to_folio(). This issue triggers when mTHP splitting and
>> zap_nonpresent_ptes() races, and the root cause is lack of memory barrier
>> in softleaf_to_folio(). The race is as follows:
>>
>> 	CPU0                                             CPU1
>>
>> deferred_split_scan()                              zap_nonpresent_ptes()
>>    lock folio
>>    split_folio()
>>      unmap_folio()
>>        change ptes to migration entries
>>      __split_folio_to_order()                         softleaf_to_folio()
>>        set flags(including PG_locked) for tail pages    folio = pfn_folio(softleaf_to_pfn(entry))
>>        smp_wmb()                                        VM_WARN_ON_ONCE(!folio_test_locked(folio))
>>        prep_compound_page() for tail pages
>>
>> In __split_folio_to_order(), smp_wmb() guarantees page flags of tail pages
>> are visible before the tail page becomes non-compound. smp_wmb() should
>> be paired with smp_rmb() in softleaf_to_folio(), which is missed. As a
>> result, if zap_nonpresent_ptes() accesses migration entry that stores
>> tail pfn, softleaf_to_folio() may see the updated compound_head of tail
>> page before page->flags.
>>
>> To fix it, add missing smp_rmb() if the softleaf entry is migration entry
>> in softleaf_to_folio() and softleaf_to_page().
>>
>> Fixes: e9b61f19858a ("thp: reintroduce split_huge_page()")
>> Signed-off-by: Jinjiang Tu <tujinjiang@huawei.com>
>> ---
>>
>> Change since v1:
>>   * update fix tag
>>   * use helper softleaf_migration_entry_check()
>>
>>   include/linux/leafops.h | 29 ++++++++++++++++++-----------
>>   1 file changed, 18 insertions(+), 11 deletions(-)
>>
>> diff --git a/include/linux/leafops.h b/include/linux/leafops.h
>> index a9ff94b744f2..c7dbc3fb8ab6 100644
>> --- a/include/linux/leafops.h
>> +++ b/include/linux/leafops.h
>> @@ -363,6 +363,22 @@ static inline unsigned long softleaf_to_pfn(softleaf_t entry)
>>   	return swp_offset(entry) & SWP_PFN_MASK;
>>   }
>>   
>> +static inline void softleaf_migration_entry_check(softleaf_t entry,
>> +			struct folio *folio)
>> +{
>> +	if (!softleaf_is_migration(entry))
>> +		return;
>> +
>> +	/* See __split_folio_to_order() comment */
>> +	smp_rmb();
>> +
>> +	/*
>> +	 * Any use of migration entries may only occur while the
>> +	 * corresponding page is locked
>> +	 */
>> +	VM_WARN_ON_ONCE(!folio_test_locked(folio));
>> +}
>> +
>>   /**
>>    * softleaf_to_page() - Obtains struct page for PFN encoded within leaf entry.
>>    * @entry: Leaf entry, softleaf_has_pfn(@entry) must return true.
>> @@ -374,11 +390,7 @@ static inline struct page *softleaf_to_page(softleaf_t entry)
>>   	struct page *page = pfn_to_page(softleaf_to_pfn(entry));
>>   
>>   	VM_WARN_ON_ONCE(!softleaf_has_pfn(entry));
>> -	/*
>> -	 * Any use of migration entries may only occur while the
>> -	 * corresponding page is locked
>> -	 */
>> -	VM_WARN_ON_ONCE(softleaf_is_migration(entry) && !PageLocked(page));
>> +	softleaf_migration_entry_check(entry, page_folio(page));
> It might be better to do
>
> if (softleaf_is_migration(entry))
> 	softleaf_migration_entry_check(entry, page_folio(page));
>
> Removing the softleaf_is_migration() check from
> softleaf_migration_entry_check(). Then, we don't do the unconditional
> page_folio() and don't call the function for non-migration-entries.

Indeed. Although the compiler may be able to optimize it, it's better to write
like above.

Thanks.

> With that LGTM.
>