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
