Re: [PATCH v3 1/9] mm/rmap: initialize nr_pages to 1 at loop start in try_to_unmap_one

[David Hildenbrand (Arm)]

On 5/11/26 10:18, Dev Jain wrote:
> 
> 
> On 11/05/26 12:18 pm, David Hildenbrand (Arm) wrote:
>> On 5/6/26 11:44, Dev Jain wrote:
>>> Initialize nr_pages to 1 at the start of each loop iteration, like
>>> folio_referenced_one() does.
>>>
>>> Without this, nr_pages computed by a previous folio_unmap_pte_batch() call
>>> can be reused on a later iteration that does not run
>>> folio_unmap_pte_batch() again.
>>>
>>> I don’t think this is causing a bug today, but it is fragile.
>>>
>>> A real bug would require this sequence within the same try_to_unmap_one()
>>> call:
>>>
>>> 1. Hit the pte_present(pteval) branch and set nr_pages > 1.
>>> 2. Later hit the else branch and do pte_clear() for device-exclusive PTE,
>>>    and execute rest of the code with nr_pages > 1.
>>
>> Right, for hugetlb folios it should always stay at 1.
>>
>>>
>>> Executing the above would imply a lazyfree folio is mapped by a mix of
>>> present PTEs and device-exclusive PTEs.
>>
>> Why lazyfree? We use nr_pages also for
>>
>> 	folio_remove_rmap_ptes(folio, subpage, nr_pages, vma);
>>
>> and
>>
>> 	folio_put_refs(folio, nr_pages);
>>
>> Given that make_device_exclusive() operates on individual PTEs, wouldn't it be
>> possible to trigger that?
> 
> At the point of this patch, batching is supported for lazyfree and file folios.
> make_device_exclusive does not operate on file folios.

That makes sense.

You write "In practice, device-exclusive PTEs imply a GUP pin on the folio, and
lazyfree unmapping aborts try_to_unmap_one() when it detects that
condition. ".

But I don't think the get_user_page_vma_remote() will set the pte/folio dirty?

And the pin is only temporary. The caller of make_device_exclusive() will
essentially immediately drop that reference.

So can't we just hit that?

1) Mark PTE-mapped folio lazyfree. Folio+ptes are clean. Can still be writable.

2) Convert last PTE to device-exclusive. get_user_page_vma_remote() only need
writable ptes, not dirty ptes. Caller drops the reference.

3) try_to_unmap_one()


Note that make_device_exclusive() documents: "device-exclusive entries are
considered "clean" and "old" by core-mm. Device drivers must update the folio
state when informed by MMU notifiers."

But if it wasn't dirtied, there should be nothing guaranteeing that MMU
notifiers will set the folio dirty when MMU notifiers are triggered.

-- 
Cheers,

David