On 15/07/2021 19:21, Kenneth Graunke wrote:
On Thursday, July 15, 2021 4:27:44 AM PDT Tvrtko Ursulin wrote:
On 15/07/2021 12:07, Daniel Vetter wrote:
On Thu, Jul 15, 2021 at 11:33:10AM +0100, Tvrtko Ursulin wrote:
On 15/07/2021 11:15, Matthew Auld wrote:
From: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Jason Ekstrand requested a more efficient method than userptr+set-domain
to determine if the userptr object was backed by a complete set of pages
upon creation. To be more efficient than simply populating the userptr
using get_user_pages() (as done by the call to set-domain or execbuf),
we can walk the tree of vm_area_struct and check for gaps or vma not
backed by struct page (VM_PFNMAP). The question is how to handle
VM_MIXEDMAP which may be either struct page or pfn backed...
With discrete are going to drop support for set_domain(), so offering a
way to probe the pages, without having to resort to dummy batches has
been requested.
v2:
- add new query param for the PROPBE flag, so userspace can easily
check if the kernel supports it(Jason).
- use mmap_read_{lock, unlock}.
- add some kernel-doc.
1)
I think probing is too weak to be offered as part of the uapi. What probes
successfully at create time might not be there anymore at usage time. So if
the pointer is not trusted at one point, why should it be at a later stage?
Only thing which works for me is populate (so get_pages) at create time. But
again with no guarantees they are still there at use time clearly
documented.
Populate is exactly as racy as probe. We don't support pinned userptr
anymore.
Yes, wrote so myself - "..again with no guarantees they are still there
at use time..".
Perhaps I don't understand what problem is probe supposed to solve. It
doesn't deal 1:1 with set_domain removal since that one actually did
get_pages so that would be populate. But fact remains regardless that if
userspace is given a pointer it doesn't trust, _and_ wants the check it
for this reason or that, then probe solves nothing. Unless there is
actually at minimum some protocol to reply to whoever sent the pointer
like "not that pointer please".
That's exactly the point. GL_AMD_pinned_memory requires us the OpenGL
implementation to return an error for "not that pointer, please", at the
time when said pointer is supplied - not at first use.
Sure, there can be reasons why it might seem fine up front, and not work
later. But an early check of "just no, you're doing it totally wrong"
at the right moment can be helpful for application developers. While it
shouldn't really happen, if it ever did, it would be a lot more obvious
to debug than "much later on, when something randomly flushed the GPU
commands we were building, something went wrong, and we don't know why."
Ack, thanks for the clarification. For this limited use case I agree probe works.
Regards,
Tvrtko
P.S. I made a mistake (?) of looking at the GL_AMD_pinned_memory spec:
"""
3) Can the application free the memory?
RESOLVED: YES. However, the underlying buffer object should
have been deleted from the OpenGL to prevent any access by
the GPU, or the result is undefined, including program or even system
termination.
"""
Scary stuff that spec of userspace level API would allow such kernel bugs!
