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

Chris Wilson
2015-10-12T10:10:18Z

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