On Thu, Sep 24, 2015 at 11:23:48AM +0100, Tvrtko Ursulin wrote:
>
> On 09/23/2015 09:07 PM, Chris Wilson wrote:
> >If the client revokes the virtual address it asked to be mapped into GPU
> >space via userptr (by using anything along the lines of mmap, mprotect,
> >madvise, munmap, ftruncate etc) the mmu notifier sends a range
> >invalidate command to userptr. Upon receiving the invalidation signal
> >for the revoked range, we try to release the struct pages we pinned into
> >the GTT. However, this can fail if any of the GPU's VMA are pinned for
> >use by the hardware (i.e. despite the user's intention we cannot
> >relinquish the client's address range and keep uptodate with whatever is
> >placed in there). Currently we emit a few WARN so that we would notice
> >if this every occurred in the wild; it has. Sadly this means we need to
> >replace those WARNs with the proper SIGBUS to the offending clients
> >instead.
>
> How does it happen? Frame buffer?
Ignoring the issue of -EIO since patches to fix that path also haven't
landed, the primary cause is through binding the userptr to a scanout
(framebuffer). This is not recommended usage for userptr since the CPU
view is then incoherent, but not impossible.
> >Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> >Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> >Cc: Michał Winiarski <michal.winiarski@xxxxxxxxx>
> >---
> > drivers/gpu/drm/i915/i915_gem_userptr.c | 41 +++++++++++++++++++++++++++++----
> > 1 file changed, 37 insertions(+), 4 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c
> >index f75d90118888..efb404b9fe69 100644
> >--- a/drivers/gpu/drm/i915/i915_gem_userptr.c
> >+++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
> >@@ -81,11 +81,44 @@ static void __cancel_userptr__worker(struct work_struct *work)
>
> This line is a reminder the previous series still hasn't landed. I
> think it was all r-b-ed, with only my request to not rely on
> release_pages (or something) handle negative and zero page count.
>
> > was_interruptible = dev_priv->mm.interruptible;
> > dev_priv->mm.interruptible = false;
> >
> >- list_for_each_entry_safe(vma, tmp, &obj->vma_list, obj_link) {
> >- int ret = i915_vma_unbind(vma);
> >- WARN_ON(ret && ret != -EIO);
> >+ list_for_each_entry_safe(vma, tmp, &obj->vma_list, obj_link)
> >+ i915_vma_unbind(vma);
> >+ if (i915_gem_object_put_pages(obj)) {
> >+ struct task_struct *p;
> >+
> >+ DRM_ERROR("Unable to revoke ownership by userptr of"
> >+ " invalidated address range, sending SIGBUS"
> >+ " to attached clients.\n");
> >+
> >+ rcu_read_lock();
> >+ for_each_process(p) {
>
> I don't think this is safe this without holding the tasklist_lock.
Hmm, it's the only lock taken in the oom-killer for sending the signal.
The list will not change nor will tasks disappear whilst we hold the
read-lock so it seems sane.
-Chirs
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
