drm/i915: Mark the context and address space as closed
authorChris Wilson <chris@chris-wilson.co.uk>
Thu, 4 Aug 2016 06:52:46 +0000 (07:52 +0100)
committerChris Wilson <chris@chris-wilson.co.uk>
Thu, 4 Aug 2016 07:09:33 +0000 (08:09 +0100)
When the user closes the context mark it and the dependent address space
as closed. As we use an asynchronous destruct method, this has two
purposes.  First it allows us to flag the closed context and detect
internal errors if we to create any new objects for it (as it is removed
from the user's namespace, these should be internal bugs only). And
secondly, it allows us to immediately reap stale vma.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1470293567-10811-27-git-send-email-chris@chris-wilson.co.uk
drivers/gpu/drm/i915/i915_drv.h
drivers/gpu/drm/i915/i915_gem.c
drivers/gpu/drm/i915/i915_gem_context.c
drivers/gpu/drm/i915/i915_gem_gtt.c
drivers/gpu/drm/i915/i915_gem_gtt.h
drivers/gpu/drm/i915/i915_gem_stolen.c

index f470ea195253966e08f0a9d754066153ac9008e8..ce472c9bd7f942e12de7b465ccf331f22fc78f9e 100644 (file)
@@ -907,6 +907,7 @@ struct i915_gem_context {
        struct list_head link;
 
        u8 remap_slice;
+       bool closed:1;
 };
 
 enum fb_op_origin {
index 5a66ad4bb1f272dc73f11560b538c04080967b81..85a06dcb2f84f7314f67fa304b9ea78f4d67bb78 100644 (file)
@@ -2857,12 +2857,15 @@ static int __i915_vma_unbind(struct i915_vma *vma, bool wait)
                __i915_vma_iounmap(vma);
        }
 
-       trace_i915_vma_unbind(vma);
-
-       vma->vm->unbind_vma(vma);
+       if (likely(!vma->vm->closed)) {
+               trace_i915_vma_unbind(vma);
+               vma->vm->unbind_vma(vma);
+       }
        vma->bound = 0;
 
-       list_del_init(&vma->vm_link);
+       drm_mm_remove_node(&vma->node);
+       list_move_tail(&vma->vm_link, &vma->vm->unbound_list);
+
        if (vma->is_ggtt) {
                if (vma->ggtt_view.type == I915_GGTT_VIEW_NORMAL) {
                        obj->map_and_fenceable = false;
@@ -2873,8 +2876,6 @@ static int __i915_vma_unbind(struct i915_vma *vma, bool wait)
                vma->ggtt_view.pages = NULL;
        }
 
-       drm_mm_remove_node(&vma->node);
-
        /* Since the unbound list is global, only move to that list if
         * no more VMAs exist. */
        if (--obj->bind_count == 0)
@@ -3116,7 +3117,7 @@ search_free:
                goto err_remove_node;
 
        list_move_tail(&obj->global_list, &dev_priv->mm.bound_list);
-       list_add_tail(&vma->vm_link, &vm->inactive_list);
+       list_move_tail(&vma->vm_link, &vm->inactive_list);
        obj->bind_count++;
 
        return vma;
index 823e74c86eaabd1a8e4f4e165357ff95d57f0ec1..a4ee62335e755669f8f171283122c63d35a99a7c 100644 (file)
@@ -156,6 +156,7 @@ void i915_gem_context_free(struct kref *ctx_ref)
 
        lockdep_assert_held(&ctx->i915->drm.struct_mutex);
        trace_i915_context_free(ctx);
+       GEM_BUG_ON(!ctx->closed);
 
        /*
         * This context is going away and we need to remove all VMAs still
@@ -224,6 +225,37 @@ i915_gem_alloc_context_obj(struct drm_device *dev, size_t size)
        return obj;
 }
 
+static void i915_ppgtt_close(struct i915_address_space *vm)
+{
+       struct list_head *phases[] = {
+               &vm->active_list,
+               &vm->inactive_list,
+               &vm->unbound_list,
+               NULL,
+       }, **phase;
+
+       GEM_BUG_ON(vm->closed);
+       vm->closed = true;
+
+       for (phase = phases; *phase; phase++) {
+               struct i915_vma *vma, *vn;
+
+               list_for_each_entry_safe(vma, vn, *phase, vm_link)
+                       if (!vma->closed)
+                               i915_vma_close(vma);
+       }
+}
+
+static void context_close(struct i915_gem_context *ctx)
+{
+       GEM_BUG_ON(ctx->closed);
+       ctx->closed = true;
+       if (ctx->ppgtt)
+               i915_ppgtt_close(&ctx->ppgtt->base);
+       ctx->file_priv = ERR_PTR(-EBADF);
+       i915_gem_context_put(ctx);
+}
+
 static int assign_hw_id(struct drm_i915_private *dev_priv, unsigned *out)
 {
        int ret;
@@ -305,7 +337,7 @@ __create_hw_context(struct drm_device *dev,
        return ctx;
 
 err_out:
-       i915_gem_context_put(ctx);
+       context_close(ctx);
        return ERR_PTR(ret);
 }
 
@@ -334,7 +366,7 @@ i915_gem_create_context(struct drm_device *dev,
                        DRM_DEBUG_DRIVER("PPGTT setup failed (%ld)\n",
                                         PTR_ERR(ppgtt));
                        idr_remove(&file_priv->context_idr, ctx->user_handle);
-                       i915_gem_context_put(ctx);
+                       context_close(ctx);
                        return ERR_CAST(ppgtt);
                }
 
@@ -505,7 +537,7 @@ void i915_gem_context_fini(struct drm_device *dev)
 
        lockdep_assert_held(&dev->struct_mutex);
 
-       i915_gem_context_put(dctx);
+       context_close(dctx);
        dev_priv->kernel_context = NULL;
 
        ida_destroy(&dev_priv->context_hw_ida);
@@ -515,8 +547,7 @@ static int context_idr_cleanup(int id, void *p, void *data)
 {
        struct i915_gem_context *ctx = p;
 
-       ctx->file_priv = ERR_PTR(-EBADF);
-       i915_gem_context_put(ctx);
+       context_close(ctx);
        return 0;
 }
 
@@ -1014,7 +1045,7 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
        }
 
        idr_remove(&file_priv->context_idr, ctx->user_handle);
-       i915_gem_context_put(ctx);
+       context_close(ctx);
        mutex_unlock(&dev->struct_mutex);
 
        DRM_DEBUG_DRIVER("HW context %d destroyed\n", args->ctx_id);
index 3e9d735cf2c3389cf3bb564c3cc7c7d190c5bc54..d42463c3ff668f1d034c590708991cb948781436 100644 (file)
@@ -2121,6 +2121,7 @@ static void i915_address_space_init(struct i915_address_space *vm,
        drm_mm_init(&vm->mm, vm->start, vm->total);
        INIT_LIST_HEAD(&vm->active_list);
        INIT_LIST_HEAD(&vm->inactive_list);
+       INIT_LIST_HEAD(&vm->unbound_list);
        list_add_tail(&vm->global_link, &dev_priv->vm_list);
 }
 
@@ -2213,9 +2214,10 @@ void  i915_ppgtt_release(struct kref *kref)
 
        trace_i915_ppgtt_release(&ppgtt->base);
 
-       /* vmas should already be unbound */
+       /* vmas should already be unbound and destroyed */
        WARN_ON(!list_empty(&ppgtt->base.active_list));
        WARN_ON(!list_empty(&ppgtt->base.inactive_list));
+       WARN_ON(!list_empty(&ppgtt->base.unbound_list));
 
        list_del(&ppgtt->base.global_link);
        drm_mm_takedown(&ppgtt->base.mm);
@@ -3377,6 +3379,8 @@ __i915_gem_vma_create(struct drm_i915_gem_object *obj,
        struct i915_vma *vma;
        int i;
 
+       GEM_BUG_ON(vm->closed);
+
        if (WARN_ON(i915_is_ggtt(vm) != !!ggtt_view))
                return ERR_PTR(-EINVAL);
 
@@ -3384,11 +3388,11 @@ __i915_gem_vma_create(struct drm_i915_gem_object *obj,
        if (vma == NULL)
                return ERR_PTR(-ENOMEM);
 
-       INIT_LIST_HEAD(&vma->vm_link);
        INIT_LIST_HEAD(&vma->obj_link);
        INIT_LIST_HEAD(&vma->exec_list);
        for (i = 0; i < ARRAY_SIZE(vma->last_read); i++)
                init_request_active(&vma->last_read[i], i915_vma_retire);
+       list_add(&vma->vm_link, &vm->unbound_list);
        vma->vm = vm;
        vma->obj = obj;
        vma->is_ggtt = i915_is_ggtt(vm);
@@ -3429,6 +3433,7 @@ i915_gem_obj_lookup_or_create_ggtt_vma(struct drm_i915_gem_object *obj,
        if (!vma)
                vma = __i915_gem_vma_create(obj, &ggtt->base, view);
 
+       GEM_BUG_ON(vma->closed);
        return vma;
 
 }
index deb9dbcb94012a673283918fa2aa6a3604f83d45..f6cc3fe8e1adbc8eb201c649d0af270bbdc078c3 100644 (file)
@@ -319,6 +319,8 @@ struct i915_address_space {
        u64 start;              /* Start offset always 0 for dri2 */
        u64 total;              /* size addr space maps (ex. 2GB for ggtt) */
 
+       bool closed;
+
        struct i915_page_scratch *scratch_page;
        struct i915_page_table *scratch_pt;
        struct i915_page_directory *scratch_pd;
@@ -347,6 +349,13 @@ struct i915_address_space {
         */
        struct list_head inactive_list;
 
+       /**
+        * List of vma that have been unbound.
+        *
+        * A reference is not held on the buffer while on this list.
+        */
+       struct list_head unbound_list;
+
        /* FIXME: Need a more generic return type */
        gen6_pte_t (*pte_encode)(dma_addr_t addr,
                                 enum i915_cache_level level,
index 2c321c8df7c38c03e22fb57fa779156c81ecf06e..bc91ffe614e2aab55b6f67dc5c3db641d20ad808 100644 (file)
@@ -707,7 +707,7 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_device *dev,
 
        vma->bound |= GLOBAL_BIND;
        __i915_vma_set_map_and_fenceable(vma);
-       list_add_tail(&vma->vm_link, &ggtt->base.inactive_list);
+       list_move_tail(&vma->vm_link, &ggtt->base.inactive_list);
        obj->bind_count++;
 
        list_add_tail(&obj->global_list, &dev_priv->mm.bound_list);