Re: [PATCH 07/15] drm/i915: Stop using obj->obj_exec_link outside of execbuf

Mika Kuoppala
2017-02-24T12:34:59Z

Chris Wilson <chris@chris-wilson.co.uk> writes:

> i915_gem_stolen_list_info() sneakily takes advantage of the
> obj->obj_exec_link to save itself from having to allocate. Enough of the
> subterfuge, just allocate an array of pointers and sort them instead of
> the list.
>

Justifiable by itself but I suspect you have plans for mutex.

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 52 ++++++++++++++++++++-----------------
> 1 file changed, 28 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index ddae8e442176..75efa1ae234e 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -27,7 +27,7 @@
> */
>
> #include <linux/debugfs.h>
> -#include <linux/list_sort.h>
> +#include <linux/sort.h>
> #include "intel_drv.h"
>
> static inline struct drm_i915_private *node_to_i915(struct drm_info_node *node)
> @@ -230,13 +230,12 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj)
> seq_printf(m, " (frontbuffer: 0x%03x)", frontbuffer_bits);
> }
>
> -static int obj_rank_by_stolen(void *priv,
> - struct list_head *A, struct list_head *B)
> +static int obj_rank_by_stolen(const void *A, const void *B)
> {
> - struct drm_i915_gem_object *a =
> - container_of(A, struct drm_i915_gem_object, obj_exec_link);
> - struct drm_i915_gem_object *b =
> - container_of(B, struct drm_i915_gem_object, obj_exec_link);
> + const struct drm_i915_gem_object *a =
> + *(const struct drm_i915_gem_object **)A;
> + const struct drm_i915_gem_object *b =
> + *(const struct drm_i915_gem_object **)B;
>
> if (a->stolen->start < b->stolen->start)
> return -1;
> @@ -249,49 +248,54 @@ static int i915_gem_stolen_list_info(struct seq_file *m, void *data)
> {
> struct drm_i915_private *dev_priv = node_to_i915(m->private);
> 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;
> - LIST_HEAD(stolen);
> - int count, ret;
> + unsigned long count, n;
> + int ret;
>
> ret = mutex_lock_interruptible(&dev->struct_mutex);
> if (ret)
> return ret;
>
> + objects = drm_malloc_ab(dev_priv->mm.object_count, sizeof(*objects));
> + if (!objects) {
> + ret = -ENOMEM;
> + goto out_unlock;
> + }
> +
> total_obj_size = total_gtt_size = count = 0;
> list_for_each_entry(obj, &dev_priv->mm.bound_list, global_link) {
> if (obj->stolen == NULL)
> continue;
>
> - list_add(&obj->obj_exec_link, &stolen);
> -
> + objects[count++] = obj;
> total_obj_size += obj->base.size;
> total_gtt_size += i915_gem_obj_total_ggtt_size(obj);
> - count++;
> }
> list_for_each_entry(obj, &dev_priv->mm.unbound_list, global_link) {
> if (obj->stolen == NULL)
> continue;
>
> - list_add(&obj->obj_exec_link, &stolen);
> -
> + objects[count++] = obj;
> total_obj_size += obj->base.size;
> - count++;
> }
> - list_sort(NULL, &stolen, obj_rank_by_stolen);
> +
> + sort(objects, count, sizeof(*objects), obj_rank_by_stolen, NULL);
> +
> seq_puts(m, "Stolen:\n");
> - while (!list_empty(&stolen)) {
> - obj = list_first_entry(&stolen, typeof(*obj), obj_exec_link);
> + for (n = 0; n < count; n++) {
> seq_puts(m, " ");
> - describe_obj(m, obj);
> + describe_obj(m, objects[n]);
> seq_putc(m, '\n');
> - list_del_init(&obj->obj_exec_link);
> }
> - mutex_unlock(&dev->struct_mutex);
> -
> - seq_printf(m, "Total %d objects, %llu bytes, %llu GTT size\n",
> + seq_printf(m, "Total %lu objects, %llu bytes, %llu GTT size\n",
> count, total_obj_size, total_gtt_size);
> - return 0;
> +
> + drm_free_large(objects);
> +out_unlock:
> + mutex_unlock(&dev->struct_mutex);
> + return ret;
> }
>
> struct file_stats {
> --
> 2.11.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx