Re: [PATCH v1 4/4] mm/memory: document restore_exclusive_pte()

[David Hildenbrand <david@redhat.com>]

On 31.01.25 01:20, Alistair Popple wrote:
> On Thu, Jan 30, 2025 at 11:43:25AM +0100, Simona Vetter wrote:
>> On Thu, Jan 30, 2025 at 11:27:37AM +1100, Alistair Popple wrote:
>>> On Wed, Jan 29, 2025 at 12:58:02PM +0100, David Hildenbrand wrote:
>>>> Let's document how this function is to be used, and why the requirement
>>>> for the folio lock might maybe be dropped in the future.
>>>
>>> Sorry, only just catching up on your other thread. The folio lock was to ensure
>>> the GPU got a chance to make forward progress by mapping the page. Without it
>>> the CPU could immediately invalidate the entry before the GPU had a chance to
>>> retry the fault.
>>>
>>> Obviously performance wise having such thrashing is terrible, so should
>>> really be avoided by userspace, but the lock at least allowed such programs
>>> to complete.
>>
>> Imo this is not a legit use-case. If userspace concurrently (instead of
>> clearly alternating) uses the same 4k page for gpu atomics and on the cpu,
>> it just gets to keep the fallout.
>>
>> Plus there's no guarantee that we hold the folio_lock long enough for the
>> gpu to actually complete the atomic, so this isn't even really helping
>> with forward progress even if this somehow would be a legit usecase.
> 
> Yes, agree it's not a legit real world use case. In practice though it was
> useful for testing this and other driver code by thrashing to generate a lot
> device/cpu faults and invalidations. Obviously "just for testing" is not a great
> justification though, so if it's causing problems we could get rid of it.

Okay, I'll make that clear in the documentation. Getting rid of the 
folio lock might be really beneficial in some cases.

-- 
Cheers,

David / dhildenb