drm/i915: Report all objects with allocated pages to the shrinker
authorChris Wilson <chris@chris-wilson.co.uk>
Thu, 30 May 2019 20:35:00 +0000 (21:35 +0100)
committerChris Wilson <chris@chris-wilson.co.uk>
Fri, 31 May 2019 20:23:51 +0000 (21:23 +0100)
Currently, we try to report to the shrinker the precise number of
objects (pages) that are available to be reaped at this moment. This
requires searching all objects with allocated pages to see if they
fulfill the search criteria, and this count is performed quite
frequently. (The shrinker tries to free ~128 pages on each invocation,
before which we count all the objects; counting takes longer than
unbinding the objects!) If we take the pragmatic view that with
sufficient desire, all objects are eventually reapable (they become
inactive, or no longer used as framebuffer etc), we can simply return
the count of pinned pages maintained during get_pages/put_pages rather
than walk the lists every time.

The downside is that we may (slightly) over-report the number of
objects/pages we could shrink and so penalize ourselves by shrinking
more than required. This is mitigated by keeping the order in which we
shrink objects such that we avoid penalizing active and frequently used
objects, and if memory is so tight that we need to free them we would
need to anyway.

v2: Only expose shrinkable objects to the shrinker; a small reduction in
not considering stolen and foreign objects.
v3: Restore the tracking from a "backup" copy from before the gem/ split

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190530203500.26272-2-chris@chris-wilson.co.uk
drivers/gpu/drm/i915/gem/i915_gem_domain.c
drivers/gpu/drm/i915/gem/i915_gem_object.c
drivers/gpu/drm/i915/gem/i915_gem_pages.c
drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
drivers/gpu/drm/i915/gem/i915_gem_stolen.c
drivers/gpu/drm/i915/i915_debugfs.c
drivers/gpu/drm/i915/i915_drv.h
drivers/gpu/drm/i915/i915_gem.c
drivers/gpu/drm/i915/i915_vma.c

index 52b73e90c9f42466064ca2cdb3c1a23e6858d9db..e5deae62681f61d983bdbf33633dae9ecefe05d1 100644 (file)
@@ -475,7 +475,8 @@ static void i915_gem_object_bump_inactive_ggtt(struct drm_i915_gem_object *obj)
        }
        mutex_unlock(&i915->ggtt.vm.mutex);
 
-       if (obj->mm.madv == I915_MADV_WILLNEED) {
+       if (i915_gem_object_is_shrinkable(obj) &&
+           obj->mm.madv == I915_MADV_WILLNEED) {
                struct list_head *list;
 
                spin_lock(&i915->mm.obj_lock);
index 1ec60be0675591fa8180d9ebf3d6092eada8bf17..b840cf179bbe12e862f0f3a0a3bfaa18578f97b7 100644 (file)
@@ -44,25 +44,6 @@ void i915_gem_object_free(struct drm_i915_gem_object *obj)
        return kmem_cache_free(global.slab_objects, obj);
 }
 
-/* some bookkeeping */
-static void i915_gem_info_add_obj(struct drm_i915_private *i915,
-                                 u64 size)
-{
-       spin_lock(&i915->mm.object_stat_lock);
-       i915->mm.object_count++;
-       i915->mm.object_memory += size;
-       spin_unlock(&i915->mm.object_stat_lock);
-}
-
-static void i915_gem_info_remove_obj(struct drm_i915_private *i915,
-                                    u64 size)
-{
-       spin_lock(&i915->mm.object_stat_lock);
-       i915->mm.object_count--;
-       i915->mm.object_memory -= size;
-       spin_unlock(&i915->mm.object_stat_lock);
-}
-
 static void
 frontbuffer_retire(struct i915_active_request *active,
                   struct i915_request *request)
@@ -98,8 +79,6 @@ void i915_gem_object_init(struct drm_i915_gem_object *obj,
        obj->mm.madv = I915_MADV_WILLNEED;
        INIT_RADIX_TREE(&obj->mm.get_page.radix, GFP_KERNEL | __GFP_NOWARN);
        mutex_init(&obj->mm.get_page.lock);
-
-       i915_gem_info_add_obj(to_i915(obj->base.dev), obj->base.size);
 }
 
 /**
@@ -163,11 +142,14 @@ void i915_gem_close_object(struct drm_gem_object *gem, struct drm_file *file)
 
 static bool discard_backing_storage(struct drm_i915_gem_object *obj)
 {
-       /* If we are the last user of the backing storage (be it shmemfs
+       /*
+        * If we are the last user of the backing storage (be it shmemfs
         * pages or stolen etc), we know that the pages are going to be
         * immediately released. In this case, we can then skip copying
         * back the contents from the GPU.
         */
+       if (!i915_gem_object_is_shrinkable(obj))
+               return false;
 
        if (obj->mm.madv != I915_MADV_WILLNEED)
                return false;
@@ -208,13 +190,15 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
                GEM_BUG_ON(!list_empty(&obj->vma.list));
                GEM_BUG_ON(!RB_EMPTY_ROOT(&obj->vma.tree));
 
-               /* This serializes freeing with the shrinker. Since the free
+               /*
+                * This serializes freeing with the shrinker. Since the free
                 * is delayed, first by RCU then by the workqueue, we want the
                 * shrinker to be able to free pages of unreferenced objects,
                 * or else we may oom whilst there are plenty of deferred
                 * freed objects.
                 */
-               if (i915_gem_object_has_pages(obj)) {
+               if (i915_gem_object_has_pages(obj) &&
+                   i915_gem_object_is_shrinkable(obj)) {
                        spin_lock(&i915->mm.obj_lock);
                        list_del_init(&obj->mm.link);
                        spin_unlock(&i915->mm.obj_lock);
@@ -240,7 +224,6 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
 
                reservation_object_fini(&obj->__builtin_resv);
                drm_gem_object_release(&obj->base);
-               i915_gem_info_remove_obj(i915, obj->base.size);
 
                bitmap_free(obj->bit_17);
                i915_gem_object_free(obj);
index e53860147f21006d60425e7e91462995ecf1eeff..7e64fd6bc19bd574d271a4a313ba2bf0fa9f3ed6 100644 (file)
@@ -56,9 +56,13 @@ void __i915_gem_object_set_pages(struct drm_i915_gem_object *obj,
        }
        GEM_BUG_ON(!HAS_PAGE_SIZES(i915, obj->mm.page_sizes.sg));
 
-       spin_lock(&i915->mm.obj_lock);
-       list_add(&obj->mm.link, &i915->mm.unbound_list);
-       spin_unlock(&i915->mm.obj_lock);
+       if (i915_gem_object_is_shrinkable(obj)) {
+               spin_lock(&i915->mm.obj_lock);
+               i915->mm.shrink_count++;
+               i915->mm.shrink_memory += obj->base.size;
+               list_add(&obj->mm.link, &i915->mm.unbound_list);
+               spin_unlock(&i915->mm.obj_lock);
+       }
 }
 
 int ____i915_gem_object_get_pages(struct drm_i915_gem_object *obj)
@@ -146,9 +150,13 @@ __i915_gem_object_unset_pages(struct drm_i915_gem_object *obj)
        if (IS_ERR_OR_NULL(pages))
                return pages;
 
-       spin_lock(&i915->mm.obj_lock);
-       list_del(&obj->mm.link);
-       spin_unlock(&i915->mm.obj_lock);
+       if (i915_gem_object_is_shrinkable(obj)) {
+               spin_lock(&i915->mm.obj_lock);
+               list_del(&obj->mm.link);
+               i915->mm.shrink_count--;
+               i915->mm.shrink_memory -= obj->base.size;
+               spin_unlock(&i915->mm.obj_lock);
+       }
 
        if (obj->mm.mapping) {
                void *ptr;
index 6a93e326abf3ade84d88fe6fe6e7992a1dce7316..d71e630c6fb80a03f86af75ede5897e79491b1f1 100644 (file)
@@ -309,30 +309,14 @@ i915_gem_shrinker_count(struct shrinker *shrinker, struct shrink_control *sc)
 {
        struct drm_i915_private *i915 =
                container_of(shrinker, struct drm_i915_private, mm.shrinker);
-       struct drm_i915_gem_object *obj;
-       unsigned long num_objects = 0;
-       unsigned long count = 0;
+       unsigned long num_objects;
+       unsigned long count;
 
-       spin_lock(&i915->mm.obj_lock);
-       list_for_each_entry(obj, &i915->mm.unbound_list, mm.link)
-               if (can_release_pages(obj)) {
-                       count += obj->base.size >> PAGE_SHIFT;
-                       num_objects++;
-               }
+       count = READ_ONCE(i915->mm.shrink_memory) >> PAGE_SHIFT;
+       num_objects = READ_ONCE(i915->mm.shrink_count);
 
-       list_for_each_entry(obj, &i915->mm.bound_list, mm.link)
-               if (!i915_gem_object_is_active(obj) && can_release_pages(obj)) {
-                       count += obj->base.size >> PAGE_SHIFT;
-                       num_objects++;
-               }
-       list_for_each_entry(obj, &i915->mm.purge_list, mm.link)
-               if (!i915_gem_object_is_active(obj) && can_release_pages(obj)) {
-                       count += obj->base.size >> PAGE_SHIFT;
-                       num_objects++;
-               }
-       spin_unlock(&i915->mm.obj_lock);
-
-       /* Update our preferred vmscan batch size for the next pass.
+       /*
+        * Update our preferred vmscan batch size for the next pass.
         * Our rough guess for an effective batch size is roughly 2
         * available GEM objects worth of pages. That is we don't want
         * the shrinker to fire, until it is worth the cost of freeing an
index 9080a736663abc93b7ac7c8ec74075da7124faaa..84d4f549eb21cb7abe49e4026726223fec1cde0c 100644 (file)
@@ -690,7 +690,7 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_i915_private *dev_priv
        mutex_unlock(&ggtt->vm.mutex);
 
        spin_lock(&dev_priv->mm.obj_lock);
-       list_move_tail(&obj->mm.link, &dev_priv->mm.bound_list);
+       GEM_BUG_ON(i915_gem_object_is_shrinkable(obj));
        obj->bind_count++;
        spin_unlock(&dev_priv->mm.obj_lock);
 
index 74afdeff22457c01f108e82020c367ca201bcf14..f212241a275896d197c4f31ee6afc8a74bef3d18 100644 (file)
@@ -271,7 +271,7 @@ static int i915_gem_stolen_list_info(struct seq_file *m, void *data)
        unsigned long total, count, n;
        int ret;
 
-       total = READ_ONCE(dev_priv->mm.object_count);
+       total = READ_ONCE(dev_priv->mm.shrink_count);
        objects = kvmalloc_array(total, sizeof(*objects), GFP_KERNEL);
        if (!objects)
                return -ENOMEM;
@@ -460,9 +460,9 @@ static int i915_gem_object_info(struct seq_file *m, void *data)
        char buf[80];
        int ret;
 
-       seq_printf(m, "%u objects, %llu bytes\n",
-                  dev_priv->mm.object_count,
-                  dev_priv->mm.object_memory);
+       seq_printf(m, "%u shrinkable objects, %llu bytes\n",
+                  dev_priv->mm.shrink_count,
+                  dev_priv->mm.shrink_memory);
 
        size = count = 0;
        mapped_size = mapped_count = 0;
@@ -552,55 +552,6 @@ static int i915_gem_object_info(struct seq_file *m, void *data)
        return 0;
 }
 
-static int i915_gem_gtt_info(struct seq_file *m, void *data)
-{
-       struct drm_info_node *node = m->private;
-       struct drm_i915_private *dev_priv = node_to_i915(node);
-       struct drm_device *dev = &dev_priv->drm;
-       struct drm_i915_gem_object **objects;
-       struct drm_i915_gem_object *obj;
-       u64 total_obj_size, total_gtt_size;
-       unsigned long nobject, n;
-       int count, ret;
-
-       nobject = READ_ONCE(dev_priv->mm.object_count);
-       objects = kvmalloc_array(nobject, sizeof(*objects), GFP_KERNEL);
-       if (!objects)
-               return -ENOMEM;
-
-       ret = mutex_lock_interruptible(&dev->struct_mutex);
-       if (ret)
-               return ret;
-
-       count = 0;
-       spin_lock(&dev_priv->mm.obj_lock);
-       list_for_each_entry(obj, &dev_priv->mm.bound_list, mm.link) {
-               objects[count++] = obj;
-               if (count == nobject)
-                       break;
-       }
-       spin_unlock(&dev_priv->mm.obj_lock);
-
-       total_obj_size = total_gtt_size = 0;
-       for (n = 0;  n < count; n++) {
-               obj = objects[n];
-
-               seq_puts(m, "   ");
-               describe_obj(m, obj);
-               seq_putc(m, '\n');
-               total_obj_size += obj->base.size;
-               total_gtt_size += i915_gem_obj_total_ggtt_size(obj);
-       }
-
-       mutex_unlock(&dev->struct_mutex);
-
-       seq_printf(m, "Total %d objects, %llu bytes, %llu GTT size\n",
-                  count, total_obj_size, total_gtt_size);
-       kvfree(objects);
-
-       return 0;
-}
-
 static int i915_gem_batch_pool_info(struct seq_file *m, void *data)
 {
        struct drm_i915_private *dev_priv = node_to_i915(m->private);
@@ -4582,7 +4533,6 @@ static const struct file_operations i915_fifo_underrun_reset_ops = {
 static const struct drm_info_list i915_debugfs_list[] = {
        {"i915_capabilities", i915_capabilities, 0},
        {"i915_gem_objects", i915_gem_object_info, 0},
-       {"i915_gem_gtt", i915_gem_gtt_info, 0},
        {"i915_gem_stolen", i915_gem_stolen_list_info },
        {"i915_gem_fence_regs", i915_gem_fence_regs_info, 0},
        {"i915_gem_interrupt", i915_interrupt_info, 0},
index dc21955891c78eccd1d26313a7b40aa121c34550..76f2bf90ed86e2136153e5e06a2c32ebef2e1604 100644 (file)
@@ -926,10 +926,9 @@ struct i915_gem_mm {
        /** Bit 6 swizzling required for Y tiling */
        u32 bit_6_swizzle_y;
 
-       /* accounting, useful for userland debugging */
-       spinlock_t object_stat_lock;
-       u64 object_memory;
-       u32 object_count;
+       /* shrinker accounting, also useful for userland debugging */
+       u64 shrink_memory;
+       u32 shrink_count;
 };
 
 #define I915_IDLE_ENGINES_TIMEOUT (200) /* in ms */
index 1362a8803d2a2fa972de508b863ded41d263f15c..4739a6307c32ee4bc7b31e8ae9c7aea361c8b3e0 100644 (file)
@@ -1137,15 +1137,17 @@ i915_gem_madvise_ioctl(struct drm_device *dev, void *data,
        if (i915_gem_object_has_pages(obj)) {
                struct list_head *list;
 
-               spin_lock(&i915->mm.obj_lock);
-               if (obj->mm.madv != I915_MADV_WILLNEED)
-                       list = &i915->mm.purge_list;
-               else if (obj->bind_count)
-                       list = &i915->mm.bound_list;
-               else
-                       list = &i915->mm.unbound_list;
-               list_move_tail(&obj->mm.link, list);
-               spin_unlock(&i915->mm.obj_lock);
+               if (i915_gem_object_is_shrinkable(obj)) {
+                       spin_lock(&i915->mm.obj_lock);
+                       if (obj->mm.madv != I915_MADV_WILLNEED)
+                               list = &i915->mm.purge_list;
+                       else if (obj->bind_count)
+                               list = &i915->mm.bound_list;
+                       else
+                               list = &i915->mm.unbound_list;
+                       list_move_tail(&obj->mm.link, list);
+                       spin_unlock(&i915->mm.obj_lock);
+               }
        }
 
        /* if the object is no longer attached, discard its backing storage */
@@ -1758,7 +1760,6 @@ i915_gem_load_init_fences(struct drm_i915_private *dev_priv)
 
 static void i915_gem_init__mm(struct drm_i915_private *i915)
 {
-       spin_lock_init(&i915->mm.object_stat_lock);
        spin_lock_init(&i915->mm.obj_lock);
        spin_lock_init(&i915->mm.free_lock);
 
@@ -1808,7 +1809,7 @@ void i915_gem_cleanup_early(struct drm_i915_private *dev_priv)
        i915_gem_drain_freed_objects(dev_priv);
        GEM_BUG_ON(!llist_empty(&dev_priv->mm.free_list));
        GEM_BUG_ON(atomic_read(&dev_priv->mm.free_count));
-       WARN_ON(dev_priv->mm.object_count);
+       WARN_ON(dev_priv->mm.shrink_count);
 
        cleanup_srcu_struct(&dev_priv->gpu_error.reset_backoff_srcu);
 
index f640caec4bae6cc96e552505f1aebd325fd66c05..b7fb7d216f77d481a0bd7fcdd68444c67deac644 100644 (file)
@@ -110,7 +110,8 @@ static void __i915_vma_retire(struct i915_active *ref)
         * so that we don't steal from recently used but inactive objects
         * (unless we are forced to ofc!)
         */
-       obj_bump_mru(obj);
+       if (i915_gem_object_is_shrinkable(obj))
+               obj_bump_mru(obj);
 
        i915_gem_object_put(obj); /* and drop the active reference */
 }
@@ -677,11 +678,14 @@ i915_vma_insert(struct i915_vma *vma, u64 size, u64 alignment, u64 flags)
                struct drm_i915_gem_object *obj = vma->obj;
 
                spin_lock(&dev_priv->mm.obj_lock);
-               list_move_tail(&obj->mm.link, &dev_priv->mm.bound_list);
-               obj->bind_count++;
-               spin_unlock(&dev_priv->mm.obj_lock);
 
+               if (i915_gem_object_is_shrinkable(obj))
+                       list_move_tail(&obj->mm.link, &dev_priv->mm.bound_list);
+
+               obj->bind_count++;
                assert_bind_count(obj);
+
+               spin_unlock(&dev_priv->mm.obj_lock);
        }
 
        return 0;
@@ -717,9 +721,13 @@ i915_vma_remove(struct i915_vma *vma)
                struct drm_i915_gem_object *obj = vma->obj;
 
                spin_lock(&i915->mm.obj_lock);
+
+               GEM_BUG_ON(obj->bind_count == 0);
                if (--obj->bind_count == 0 &&
+                   i915_gem_object_is_shrinkable(obj) &&
                    obj->mm.madv == I915_MADV_WILLNEED)
                        list_move_tail(&obj->mm.link, &i915->mm.unbound_list);
+
                spin_unlock(&i915->mm.obj_lock);
 
                /*