On 09/09/2015 04:42 PM, Chris Wilson wrote:
On Wed, Sep 09, 2015 at 04:20:08PM +0100, Tvrtko Ursulin wrote:
On 09/09/2015 04:08 PM, Chris Wilson wrote:
On Wed, Sep 09, 2015 at 03:45:40PM +0100, Tvrtko Ursulin wrote:
On 08/10/2015 09:51 AM, Chris Wilson wrote:
Whilst discussing possible ways to trigger an invalidate_range on a
userptr with an aliased GGTT mmapping (and so cause a struct_mutex
deadlock), the conclusion is that we can, and we must, prevent any
possible deadlock by avoiding taking the mutex at all during
invalidate_range. This has numerous advantages all of which stem from
avoid the sleeping function from inside the unknown context. In
particular, it simplifies the invalidate_range because we no longer
have to juggle the spinlock/mutex and can just hold the spinlock
for the entire walk. To compensate, we have to make get_pages a bit more
complicated in order to serialise with a pending cancel_userptr worker.
As we hold the struct_mutex, we have no choice but to return EAGAIN and
hope that the worker is then flushed before we retry after reacquiring
the struct_mutex.
The important caveat is that the invalidate_range itself is no longer
synchronous. There exists a small but definite period in time in which
the old PTE's page remain accessible via the GPU. Note however that the
physical pages themselves are not invalidated by the mmu_notifier, just
the CPU view of the address space. The impact should be limited to a
delay in pages being flushed, rather than a possibility of writing to
the wrong pages. The only race condition that this worsens is remapping
an userptr active on the GPU where fresh work may still reference the
old pages due to struct_mutex contention. Given that userspace is racing
with the GPU, it is fair to say that the results are undefined.
v2: Only queue (and importantly only take one refcnt) the worker once.
This one I looked at at the time of previous posting and it looked
fine, minus one wrong line of thinking of mine. On a brief look it
still looks good, so:
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
I assume Michał has run all these through the relevant test cases?
Slightly related, I now worry about the WARN_ONs in
__cancel_userptr__worker since they look to be triggerable by
malicious userspace which is not good.
They could always be I thought, if you could somehow pin the userptr
into a hardware register and then unmap the vma. That is a scary thought
and one I would like a WARN for. That should be the only way, and I shudder
at the prospect of working out who to send the SIGBUS to.
Is it not enough to submit work to the GPU and while it is running
engineer a lot of signals and munmap?
No, we block signals inside the worker, which should reduce it down to
EINVAL/EBUSY or EIO from unbind (and a subsequent WARN from put).
Yeah two lines above was obviously too far for me to spot the
interruptible business...
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
