| Message ID | 20180419141616.1100-1-mika.kuoppala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@chris-wilson.co.uk> > Cc: Eero Tamminen <eero.t.tamminen@intel.com> > Signed-off-by: Mika Kuoppala <mika.kuoppala@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. "%d unused objects (without any attached vma)" I think works better. > + 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). -Chris
Chris Wilson <chris@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@chris-wilson.co.uk> >> Cc: Eero Tamminen <eero.t.tamminen@intel.com> >> Signed-off-by: Mika Kuoppala <mika.kuoppala@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
Quoting Mika Kuoppala (2018-04-19 16:34:27) > Chris Wilson <chris@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@chris-wilson.co.uk> > >> Cc: Eero Tamminen <eero.t.tamminen@intel.com> > >> Signed-off-by: Mika Kuoppala <mika.kuoppala@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" ? They may still be used as a container for user memory, they are just not used in the GTT. Getting the meaning across in a succinct manner is tricky. > >> + 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. Not quite, just that objects may be referenced by more than client and so client_count may be greater than the total number of objects. -Chris
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); + seq_printf(m, "%d objects without client\n", + dev_priv->mm.object_count - client_count); + return 0; }
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@chris-wilson.co.uk> Cc: Eero Tamminen <eero.t.tamminen@intel.com> Signed-off-by: Mika Kuoppala <mika.kuoppala@linux.intel.com> --- drivers/gpu/drm/i915/i915_debugfs.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-)