[Intel-gfx] [PATCH] drm/i915: Show number of objects without client

[Mika Kuoppala]

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

> Quoting Mika Kuoppala (2018-04-19 15:16:16)
>> Output the number of objects not tied to a client
>> or to a vma. This amount should be quite small and
>> on oom issues we can rule out significant bo leaks
>> quickly by inspecting these values. Note that we are not
>> fully accurate due to how we take and release the locks
>> during transversal of related lists.
>> 
>> Cc: Chris Wilson <chris at chris-wilson.co.uk>
>> Cc: Eero Tamminen <eero.t.tamminen at intel.com>
>> Signed-off-by: Mika Kuoppala <mika.kuoppala at linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_debugfs.c | 24 +++++++++++++++++-------
>>  1 file changed, 17 insertions(+), 7 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
>> index e0274f41bc76..b1cbecfca716 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -354,8 +354,8 @@ static int per_file_stats(int id, void *ptr, void *data)
>>                            stats.unbound); \
>>  } while (0)
>>  
>> -static void print_batch_pool_stats(struct seq_file *m,
>> -                                  struct drm_i915_private *dev_priv)
>> +static unsigned int print_batch_pool_stats(struct seq_file *m,
>> +                                          struct drm_i915_private *dev_priv)
>>  {
>>         struct drm_i915_gem_object *obj;
>>         struct file_stats stats;
>> @@ -375,6 +375,7 @@ static void print_batch_pool_stats(struct seq_file *m,
>>         }
>>  
>>         print_file_stats(m, "[k]batch pool", stats);
>> +       return stats.count;
>>  }
>>  
>>  static int per_file_ctx_stats(int id, void *ptr, void *data)
>> @@ -392,8 +393,8 @@ static int per_file_ctx_stats(int id, void *ptr, void *data)
>>         return 0;
>>  }
>>  
>> -static void print_context_stats(struct seq_file *m,
>> -                               struct drm_i915_private *dev_priv)
>> +static unsigned int print_context_stats(struct seq_file *m,
>> +                                       struct drm_i915_private *dev_priv)
>>  {
>>         struct drm_device *dev = &dev_priv->drm;
>>         struct file_stats stats;
>> @@ -412,6 +413,7 @@ static void print_context_stats(struct seq_file *m,
>>         mutex_unlock(&dev->struct_mutex);
>>  
>>         print_file_stats(m, "[k]contexts", stats);
>> +       return stats.count;
>>  }
>>  
>>  static int i915_gem_object_info(struct seq_file *m, void *data)
>> @@ -422,7 +424,7 @@ static int i915_gem_object_info(struct seq_file *m, void *data)
>>         u32 count, mapped_count, purgeable_count, dpy_count, huge_count;
>>         u64 size, mapped_size, purgeable_size, dpy_size, huge_size;
>>         struct drm_i915_gem_object *obj;
>> -       unsigned int page_sizes = 0;
>> +       unsigned int page_sizes = 0, client_count = 0, vma_count = 0;
>>         struct drm_file *file;
>>         char buf[80];
>>         int ret;
>> @@ -462,6 +464,7 @@ static int i915_gem_object_info(struct seq_file *m, void *data)
>>                 }
>>         }
>>         seq_printf(m, "%u unbound objects, %llu bytes\n", count, size);
>> +       vma_count += count;
>>  
>>         size = count = dpy_size = dpy_count = 0;
>>         list_for_each_entry(obj, &dev_priv->mm.bound_list, mm.link) {
>> @@ -490,6 +493,7 @@ static int i915_gem_object_info(struct seq_file *m, void *data)
>>                 }
>>         }
>>         spin_unlock(&dev_priv->mm.obj_lock);
>> +       vma_count += count;
>>  
>>         seq_printf(m, "%u bound objects, %llu bytes\n",
>>                    count, size);
>> @@ -511,11 +515,11 @@ static int i915_gem_object_info(struct seq_file *m, void *data)
>>                                         buf, sizeof(buf)));
>>  
>>         seq_putc(m, '\n');
>> -       print_batch_pool_stats(m, dev_priv);
>> +       client_count += print_batch_pool_stats(m, dev_priv);
>>         mutex_unlock(&dev->struct_mutex);
>>  
>>         mutex_lock(&dev->filelist_mutex);
>> -       print_context_stats(m, dev_priv);
>> +       client_count += print_context_stats(m, dev_priv);
>>         list_for_each_entry_reverse(file, &dev->filelist, lhead) {
>>                 struct file_stats stats;
>>                 struct drm_i915_file_private *file_priv = file->driver_priv;
>> @@ -543,12 +547,18 @@ static int i915_gem_object_info(struct seq_file *m, void *data)
>>                                 request->ctx->pid : file->pid,
>>                                 PIDTYPE_PID);
>>                 print_file_stats(m, task ? task->comm : "<unknown>", stats);
>> +               client_count += stats.count;
>>                 rcu_read_unlock();
>>  
>>                 mutex_unlock(&dev->struct_mutex);
>>         }
>>         mutex_unlock(&dev->filelist_mutex);
>>  
>> +       seq_printf(m, "\n%d objects without vma\n",
>> +                  dev_priv->mm.object_count - vma_count);
>
> What does "without vma" mean? Instantiated but never used on the gpu, a
> very small subset of unbound. I'm not sure if it has value and worry
> that "without vma" is unclear internal language.
>

Noticed that after reboot on a system we had 3 objects
without usage. Thought it could add some value as a noticing
leaks.

> "%d unused objects (without any attached vma)" I think works better.

Or just "unused objects" ?

>
>> +       seq_printf(m, "%d objects without client\n",
>> +                  dev_priv->mm.object_count - client_count);
>
> What does "without client" mean? What are you going to do when it is
> negative (as computed it can legitimately be negative).

"objects without handle" better? or "unassociated objects" ?

Negative would be wrong, so the tracking of batchbool/context
must be wrong in here then.

-Mika