Re: [PATCH] drm/i915: Convert WARNs during userptr revoke to SIGBUS

[Chris Wilson <chris@chris-wilson.co.uk>]

On Mon, Oct 12, 2015 at 10:31:35AM +0100, Chris Wilson wrote:
> We basically need to clone the obj, move the pages and vma over and so
> as the vma die the pages are unreferenced. All new use will be forced to
> call gup and be fine.
> 
> Ok, that smells ok, I'll see how doable that is.

Hmm. If we take the vma-centric driver as granted (i.e. using the vma as the
token when pinning, the vma holds fences etc), the tricky part if that we
don't hold a reference from the pinned vma to the object. pin_display to
the rescue!

Initial sketch:

static struct drm_i915_gem_object *steal_pages(struct drm_i915_gem_object *obj)
{
        struct drm_i915_gem_object *clone;
        struct i915_vma *vma;
        int i;

        BUG_ON(obj->stolen);

        clone = i915_gem_object_alloc(obj->base.dev);
        if (clone == NULL)
                return clone;

        drm_gem_private_object_init(obj->base.dev,
                                    &clone->base,
                                    obj->base.size);
        i915_gem_object_init(clone, obj->ops);

        list_splice_init(&obj->vma_list, &clone->vma_list);
        list_for_each_entry(vma, &clone->vma_list, obj_link)
                vma->obj = clone;

        if (obj->pin_display) {
                clone->pin_display = obj->pin_display;
                while (obj->pin_display--) {
                        drm_gem_object_reference(&clone->base);
                        drm_gem_object_unreference(&obj->base);
                }
        }

        clone->bind_count = obj->bind_count;
        obj->bind_count = 0;
        /* vma_ht / vma_hashed */

        for (i = 0; i < I915_NUM_RINGS; i++) {
                if (obj->last_read[i].request == NULL)
                        continue;

                clone->last_read[i].request = obj->last_read[i].request;
                list_replace_init(&obj->last_read[i].link,
                                  &clone->last_read[i].link);
                clone->flags |= 1 << (i + I915_BO_ACTIVE_SHIFT);
        }
        if (obj->last_write.request) {
                clone->last_write.request = obj->last_write.request;
                list_replace_init(&obj->last_write.link,
                                  &clone->last_write.link);
        }

        clone->dirty = obj->dirty;
        obj->dirty = false;

        clone->tiling_mode = obj->tiling_mode;
        clone->stride = obj->stride;

        clone->pin_display = obj->pin_display;
        obj->pin_display = 0;

        clone->madv = I915_MADV_DONTNEED;
        clone->pages = obj->pages;
        clone->pages_pin_count = obj->pages_pin_count;
        clone->get_page = obj->get_page;
        clone->vmapping = obj->vmapping;
        obj->pages = NULL;
        obj->pages_pin_count = 0;
        obj->vmapping = NULL;

        clone->bit_17 = obj->bit_17;
        obj->bit_17 = NULL;

        i915_gem_release_mmap(obj);

        if (I915_BO_IS_ACTIVE(clone))
                clone->active_reference = true;
        else
                drm_gem_object_unreference(&clone->base);

        return clone;
}

It does have the presumption that we have either an active reference or a
pinned reference.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx