Re: [PATCH v3] mm/rmap: fix incorrect pte restoration for lazyfree folios

[Wei Yang]

On Tue, Mar 03, 2026 at 05:55:58PM +0530, Dev Jain wrote:
[...]
>>>
>>> Fixes: 354dffd29575 ("mm: support batched unmap for lazyfree large folios during reclamation")
>>> Cc: stable <stable@xxxxxxxxxx>
>>> Signed-off-by: Dev Jain <dev.jain@xxxxxxx>
>>> ---
>>> Patch applies on mm-unstable (9af4957ef127).
>>>
>>> v2->v3:
>>> - Don't special case for anon folios
>>>
>>> v1->v2:
>>> - Just respect the writable bit instead of hacking in a pte_wrprotect() in
>>>   failure path
>>> - Also handle soft-dirty bit
>>>
>>> mm/rmap.c | 9 ++++++++-
>>> 1 file changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/mm/rmap.c b/mm/rmap.c
>>> index bff8f222004e4..5a3e408e3f179 100644
>>> --- a/mm/rmap.c
>>> +++ b/mm/rmap.c
>>> @@ -1955,7 +1955,14 @@ static inline unsigned int folio_unmap_pte_batch(struct folio *folio,
>>> 	if (userfaultfd_wp(vma))
>>> 		return 1;
>>>
>>> -	return folio_pte_batch(folio, pvmw->pte, pte, max_nr);
>>> +	/*
>>> +	 * If unmap fails, we need to restore the ptes. To avoid accidentally
>>> +	 * upgrading write permissions for ptes that were not originally
>>> +	 * writable, and to avoid losing the soft-dirty bit, use the
>>> +	 * appropriate FPB flags.
>>> +	 */
>>> +	return folio_pte_batch_flags(folio, vma, pvmw->pte, &pte, max_nr,
>>> +				     FPB_RESPECT_WRITE | FPB_RESPECT_SOFT_DIRTY);
>>> }
>>>
>> 
>> Hi, Dev
>> 
>> When reading the code, I got one confusion. Current call flow is like below:
>> 
>>     try_to_unmap_one();
>>         nr_pages = folio_unmap_pte_batch(folio, &pvmw, flags, pteval);
>> 	..
>> 	pteval = get_and_clear_ptes(mm, address, pvmw.pte, nr_pages);
>> 	..
>> 	set_ptes(mm, address, pvmw.pte, pteval, nr_pages);
>> 
>> We get pteval by folio_unmap_pte_batch() but it is set again by
>
>folio_unmap_pte_batch() gives the batch size, not pteval. pteval is
>given by get_and_clear_ptes() after accumulating a/d bits.
>
>> get_and_clear_ptes(), which maybe a different value. Then we use this pteval
>> to restore ptes.
>> 
>> So even we fix folio_unmap_pte_batch(), how this impact on the final restored
>> value?
>
>By respecting writable bit, we ensure that the ptes in the batch do not
>have a mix of writable and non writable ptes.
>

Thanks for your explanation. I got it.

Then the fix LGTM.

Reviewed-by: Wei Yang <richard.weiyang@xxxxxxxxx>

>So, if pteval returned by get_and_clear_ptes() is writable, then it is
>guaranteed via folio_unmap_pte_batch() that the all pte values of
>these nr_pages consecutive ptes, are writable. And vice versa.
>
>> 
>> Hope I don't miss something.
>> 
>>> /*
>>> -- 
>>> 2.34.1
>>>
>> 

-- 
Wei Yang
Help you, Help me