Re: [PATCH] mm: fix finish_fault() handling for large folios

[David Hildenbrand <david@redhat.com>]

On 26.02.25 17:28, Matthew Wilcox wrote:
> On Wed, Feb 26, 2025 at 04:42:46PM +0100, David Hildenbrand wrote:
>> On 26.02.25 15:03, Matthew Wilcox wrote:
>>> On Wed, Feb 26, 2025 at 06:48:15AM -0500, Brian Geffon wrote:
>>>> When handling faults for anon shmem finish_fault() will attempt to install
>>>> ptes for the entire folio. Unfortunately if it encounters a single
>>>> non-pte_none entry in that range it will bail, even if the pte that
>>>> triggered the fault is still pte_none. When this situation happens the
>>>> fault will be retried endlessly never making forward progress.
>>>>
>>>> This patch fixes this behavior and if it detects that a pte in the range
>>>> is not pte_none it will fall back to setting just the pte for the
>>>> address that triggered the fault.
>>>
>>> Surely there's a similar problem in do_anonymous_page()?
>>
>> I recall we handle it in there correctly the last time I stared at it.
>>
>> We check pte_none to decide which folio size we can allocate (including
>> basing the decision on other factors like VMA etc), and after retaking the
>> PTL, we recheck vmf_pte_changed / pte_range_none() to make sure there were
>> no races.
> 
> Ah, so then we'll retry and allocate a folio of the right size the next
> time?

IIRC we'll retry the fault in case we had a race. Likely, if we had a 
race, somebody else installed a (large) folio and we essentially have to 
second fault. If, for some reason, the race only touched parts of the 
PTEs we tried to modify, we'll get another fault and allocate something 
(smaller) that would fit into the new empty range.

So yes, we're more flexible because we're allocating the folios and 
don't have to take whatever folio size is in the pagecache in consideration.

-- 
Cheers,

David / dhildenb