Re: [PATCH v4] mm: Use a folio in the softleaf_is_device_private path

On 8/18/26 4:56 PM, Lorenzo Stoakes (ARM) wrote:
> On Tue, Aug 18, 2026 at 09:30:40AM +0200, David Hildenbrand (Arm) wrote:
>> On 8/18/26 04:13, Hongfu Li wrote:
>>>>> So after migrate_to_ram() the folio might have been split or otherwise somehow
>>>>> the page doesn't belong to the same locked, refcount-incremented folio it did
>>>>> before?
>>>> I suspect a split.
>>>>
>>>>> That's kinda a footgun... but this documents it at least.
>>>>>
>>>>> I think a comment explaining how this can happen would be helpful though as this
>>>>> doesn't seem intuitive.
>>>> I think, conceptually, calling into something that consumes a page (vmf->page)
>>>> always needs care when operating on folios.
>>>>
>>>> Passing the vmf to some callback might be the odd thing here, because the
>>>> vmf->page contract is not really clear.
>>> I'm very sorry for introducing this regression.
>>> Thank you for identifying and testing this issue.
>>>
>>> I will double-check this code path and apply your fix to run further
>>> tests locally.
>>>
>>> Would it make sense to add a comment here like the below, to make this
>>> subtle behavior clearer for future readers?
>>>
>>> diff --git a/mm/memory.c b/mm/memory.c
>>> index 4134ac607ee0..2b6d6c863ecb 100644
>>> --- a/mm/memory.c
>>> +++ b/mm/memory.c
>>> @@ -4937,6 +4937,12 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
>>>                                  pte_unmap_unlock(vmf->pte, vmf->ptl);
>>>                                  pgmap = page_pgmap(vmf->page);
>>>                                  ret = pgmap->ops->migrate_to_ram(vmf);
>>> +                               /*
>>> +                                * migrate_to_ram() can split a large folio, updating
>>> +                                * vmf->page to a different folio. Re-fetch folio for
>>> +                                * correct unlock/put.
>>> +                                */
>> "migrate_to_ram() might have split the folio."
>>
>> Should be sufficient I guess.
> Agreed, no need for a schloppy long comment here!
>
> 	/* migrate_to_ram() might have split the folio. */
> 	folio = page_folio(vmf->page);
>
> Should be fine.
>
> The commit message should be updated to reflect it too and mention that we are
> going from 4 -> 2 compound_head()'s now.

Thanks a lot for the suggestion. A shorter comment is indeed better.

Will use your suggested one‑line comment, add the folio re‑fetch, and
update commit message for v5.

>> --
>> Cheers,
>>
>> David
> --
> Cheers, Lorenzo

-- 
Best regards,
Hongfu