All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 3/4] drm/i915: Per-process stats work better when evaluated per-process
@ 2014-03-13 14:57 Rodrigo Vivi
  2014-03-19  0:05 ` Ben Widawsky
  0 siblings, 1 reply; 9+ messages in thread
From: Rodrigo Vivi @ 2014-03-13 14:57 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ben Widawsky

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

The idea of printing objects used by each process is to judge how each
process is using them. This means that we need to evaluate whether the
object is bound for that particular process, rather than just whether it
is bound into the global GTT.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ben Widawsky <benjamin.widawsky@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c     | 34 ++++++++++++++++++++++++++-------
 drivers/gpu/drm/i915/i915_drv.h         |  2 ++
 drivers/gpu/drm/i915/i915_gem_context.c |  1 +
 3 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index a90d31c..ed3965f 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -299,28 +299,46 @@ static int i915_gem_stolen_list_info(struct seq_file *m, void *data)
 } while (0)
 
 struct file_stats {
+	struct drm_i915_file_private *file_priv;
 	int count;
-	size_t total, active, inactive, unbound;
+	size_t total, global, active, inactive, unbound;
 };
 
 static int per_file_stats(int id, void *ptr, void *data)
 {
 	struct drm_i915_gem_object *obj = ptr;
 	struct file_stats *stats = data;
+	struct i915_vma *vma;
 
 	stats->count++;
 	stats->total += obj->base.size;
 
-	if (i915_gem_obj_ggtt_bound(obj)) {
-		if (!list_empty(&obj->ring_list))
+	list_for_each_entry(vma, &obj->vma_list, vma_link) {
+		struct i915_hw_ppgtt *ppgtt;
+
+		if (!drm_mm_node_allocated(&vma->node))
+			continue;
+
+		ppgtt = container_of(vma->vm, struct i915_hw_ppgtt, base);
+		if (ppgtt->ctx == NULL) {
+			stats->global += obj->base.size;
+			continue;
+		}
+
+		if (ppgtt->ctx->file_priv != stats->file_priv)
+			continue;
+
+		if (obj->ring) /* XXX per-vma statistic */
 			stats->active += obj->base.size;
 		else
 			stats->inactive += obj->base.size;
-	} else {
-		if (!list_empty(&obj->global_list))
-			stats->unbound += obj->base.size;
+
+		return 0;
 	}
 
+	if (!list_empty(&obj->global_list))
+		stats->unbound += obj->base.size;
+
 	return 0;
 }
 
@@ -411,6 +429,7 @@ static int i915_gem_object_info(struct seq_file *m, void* data)
 		struct task_struct *task;
 
 		memset(&stats, 0, sizeof(stats));
+		stats.file_priv = file->driver_priv;
 		idr_for_each(&file->object_idr, per_file_stats, &stats);
 		/*
 		 * Although we have a valid reference on file->pid, that does
@@ -420,12 +439,13 @@ static int i915_gem_object_info(struct seq_file *m, void* data)
 		 */
 		rcu_read_lock();
 		task = pid_task(file->pid, PIDTYPE_PID);
-		seq_printf(m, "%s: %u objects, %zu bytes (%zu active, %zu inactive, %zu unbound)\n",
+		seq_printf(m, "%s: %u objects, %zu bytes (%zu active, %zu inactive, %zu global, %zu unbound)\n",
 			   task ? task->comm : "<unknown>",
 			   stats.count,
 			   stats.total,
 			   stats.active,
 			   stats.inactive,
+			   stats.global,
 			   stats.unbound);
 		rcu_read_unlock();
 	}
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2a319ba..b76c6de 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -721,6 +721,8 @@ struct i915_hw_ppgtt {
 		dma_addr_t *gen8_pt_dma_addr[4];
 	};
 
+	struct i915_hw_context *ctx;
+
 	int (*enable)(struct i915_hw_ppgtt *ppgtt);
 	int (*switch_mm)(struct i915_hw_ppgtt *ppgtt,
 			 struct intel_ring_buffer *ring,
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index ce41cff..1a94b07 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -215,6 +215,7 @@ create_vm_for_ctx(struct drm_device *dev, struct i915_hw_context *ctx)
 		return ERR_PTR(ret);
 	}
 
+	ppgtt->ctx = ctx;
 	return ppgtt;
 }
 
-- 
1.8.5.3

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/4] drm/i915: Per-process stats work better when evaluated per-process
  2014-03-13 14:57 [PATCH 3/4] drm/i915: Per-process stats work better when evaluated per-process Rodrigo Vivi
@ 2014-03-19  0:05 ` Ben Widawsky
  2014-03-19  6:55   ` Chris Wilson
  0 siblings, 1 reply; 9+ messages in thread
From: Ben Widawsky @ 2014-03-19  0:05 UTC (permalink / raw)
  To: Rodrigo Vivi, Chris Wilson; +Cc: intel-gfx

On Thu, Mar 13, 2014 at 11:57:00AM -0300, Rodrigo Vivi wrote:
> From: Chris Wilson <chris@chris-wilson.co.uk>
> 
> The idea of printing objects used by each process is to judge how each
> process is using them. This means that we need to evaluate whether the
> object is bound for that particular process, rather than just whether it
> is bound into the global GTT.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Ben Widawsky <benjamin.widawsky@intel.com>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c     | 34 ++++++++++++++++++++++++++-------
>  drivers/gpu/drm/i915/i915_drv.h         |  2 ++
>  drivers/gpu/drm/i915/i915_gem_context.c |  1 +
>  3 files changed, 30 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index a90d31c..ed3965f 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -299,28 +299,46 @@ static int i915_gem_stolen_list_info(struct seq_file *m, void *data)
>  } while (0)
>  
>  struct file_stats {
> +	struct drm_i915_file_private *file_priv;
>  	int count;
> -	size_t total, active, inactive, unbound;
> +	size_t total, global, active, inactive, unbound;
>  };
>  
>  static int per_file_stats(int id, void *ptr, void *data)
>  {
>  	struct drm_i915_gem_object *obj = ptr;
>  	struct file_stats *stats = data;
> +	struct i915_vma *vma;
>  
>  	stats->count++;
>  	stats->total += obj->base.size;
>  
> -	if (i915_gem_obj_ggtt_bound(obj)) {
> -		if (!list_empty(&obj->ring_list))
> +	list_for_each_entry(vma, &obj->vma_list, vma_link) {
> +		struct i915_hw_ppgtt *ppgtt;
> +
> +		if (!drm_mm_node_allocated(&vma->node))
> +			continue;
> +
> +		ppgtt = container_of(vma->vm, struct i915_hw_ppgtt, base);
> +		if (ppgtt->ctx == NULL) {
> +			stats->global += obj->base.size;
> +			continue;
> +		}

I'm not really clear how this is supposed to work for global. Can you
make me happy and change it to:

if (i915_is_ggtt(vma->vm))

> +
> +		if (ppgtt->ctx->file_priv != stats->file_priv)
> +			continue;
> +
> +		if (obj->ring) /* XXX per-vma statistic */
>  			stats->active += obj->base.size;

Doesn't active get counted too many times if multiple VMAs exist for the
same active object (not a new problem to this patch)?

>  		else
>  			stats->inactive += obj->base.size;
> -	} else {
> -		if (!list_empty(&obj->global_list))
> -			stats->unbound += obj->base.size;
> +
> +		return 0;
>  	}
>  
> +	if (!list_empty(&obj->global_list))
> +		stats->unbound += obj->base.size;
> +
>  	return 0;
>  }
>  
> @@ -411,6 +429,7 @@ static int i915_gem_object_info(struct seq_file *m, void* data)
>  		struct task_struct *task;
>  
>  		memset(&stats, 0, sizeof(stats));
> +		stats.file_priv = file->driver_priv;
>  		idr_for_each(&file->object_idr, per_file_stats, &stats);
>  		/*
>  		 * Although we have a valid reference on file->pid, that does
> @@ -420,12 +439,13 @@ static int i915_gem_object_info(struct seq_file *m, void* data)
>  		 */
>  		rcu_read_lock();
>  		task = pid_task(file->pid, PIDTYPE_PID);
> -		seq_printf(m, "%s: %u objects, %zu bytes (%zu active, %zu inactive, %zu unbound)\n",
> +		seq_printf(m, "%s: %u objects, %zu bytes (%zu active, %zu inactive, %zu global, %zu unbound)\n",
>  			   task ? task->comm : "<unknown>",
>  			   stats.count,
>  			   stats.total,
>  			   stats.active,
>  			   stats.inactive,
> +			   stats.global,
>  			   stats.unbound);
>  		rcu_read_unlock();
>  	}
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 2a319ba..b76c6de 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -721,6 +721,8 @@ struct i915_hw_ppgtt {
>  		dma_addr_t *gen8_pt_dma_addr[4];
>  	};
>  
> +	struct i915_hw_context *ctx;
> +
>  	int (*enable)(struct i915_hw_ppgtt *ppgtt);
>  	int (*switch_mm)(struct i915_hw_ppgtt *ppgtt,
>  			 struct intel_ring_buffer *ring,
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index ce41cff..1a94b07 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -215,6 +215,7 @@ create_vm_for_ctx(struct drm_device *dev, struct i915_hw_context *ctx)
>  		return ERR_PTR(ret);
>  	}
>  
> +	ppgtt->ctx = ctx;
>  	return ppgtt;
>  }
>  
> -- 
> 1.8.5.3
> 

-- 
Ben Widawsky, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/4] drm/i915: Per-process stats work better when evaluated per-process
  2014-03-19  0:05 ` Ben Widawsky
@ 2014-03-19  6:55   ` Chris Wilson
  2014-03-19 13:45     ` [PATCH 1/2] " Chris Wilson
  0 siblings, 1 reply; 9+ messages in thread
From: Chris Wilson @ 2014-03-19  6:55 UTC (permalink / raw)
  To: Ben Widawsky; +Cc: intel-gfx

On Tue, Mar 18, 2014 at 05:05:45PM -0700, Ben Widawsky wrote:
> On Thu, Mar 13, 2014 at 11:57:00AM -0300, Rodrigo Vivi wrote:
> > From: Chris Wilson <chris@chris-wilson.co.uk>
> > 
> > The idea of printing objects used by each process is to judge how each
> > process is using them. This means that we need to evaluate whether the
> > object is bound for that particular process, rather than just whether it
> > is bound into the global GTT.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Ben Widawsky <benjamin.widawsky@intel.com>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@gmail.com>
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c     | 34 ++++++++++++++++++++++++++-------
> >  drivers/gpu/drm/i915/i915_drv.h         |  2 ++
> >  drivers/gpu/drm/i915/i915_gem_context.c |  1 +
> >  3 files changed, 30 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index a90d31c..ed3965f 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -299,28 +299,46 @@ static int i915_gem_stolen_list_info(struct seq_file *m, void *data)
> >  } while (0)
> >  
> >  struct file_stats {
> > +	struct drm_i915_file_private *file_priv;
> >  	int count;
> > -	size_t total, active, inactive, unbound;
> > +	size_t total, global, active, inactive, unbound;
> >  };
> >  
> >  static int per_file_stats(int id, void *ptr, void *data)
> >  {
> >  	struct drm_i915_gem_object *obj = ptr;
> >  	struct file_stats *stats = data;
> > +	struct i915_vma *vma;
> >  
> >  	stats->count++;
> >  	stats->total += obj->base.size;
> >  
> > -	if (i915_gem_obj_ggtt_bound(obj)) {
> > -		if (!list_empty(&obj->ring_list))
> > +	list_for_each_entry(vma, &obj->vma_list, vma_link) {
> > +		struct i915_hw_ppgtt *ppgtt;
> > +
> > +		if (!drm_mm_node_allocated(&vma->node))
> > +			continue;
> > +
> > +		ppgtt = container_of(vma->vm, struct i915_hw_ppgtt, base);
> > +		if (ppgtt->ctx == NULL) {
> > +			stats->global += obj->base.size;
> > +			continue;
> > +		}
> 
> I'm not really clear how this is supposed to work for global. Can you
> make me happy and change it to:
> 
> if (i915_is_ggtt(vma->vm))

That better reflects ggtt. How about
if (i915_is_ggtt(vma->vm)) { global++; continue; }
if (ppgtt->ctx == NULL) { default++; continue; }
 
> > +
> > +		if (ppgtt->ctx->file_priv != stats->file_priv)
> > +			continue;
> > +
> > +		if (obj->ring) /* XXX per-vma statistic */
> >  			stats->active += obj->base.size;
> 
> Doesn't active get counted too many times if multiple VMAs exist for the
> same active object (not a new problem to this patch)?

Yes. It's a bridge I'm willing to cross in the future!
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/2] drm/i915: Per-process stats work better when evaluated per-process
  2014-03-19  6:55   ` Chris Wilson
@ 2014-03-19 13:45     ` Chris Wilson
  2014-03-19 13:45       ` [PATCH 2/2] drm/i915: Print how many objects are shared in per-process stats Chris Wilson
  2014-03-19 17:30       ` [PATCH 1/2] drm/i915: Per-process stats work better when evaluated per-process Ben Widawsky
  0 siblings, 2 replies; 9+ messages in thread
From: Chris Wilson @ 2014-03-19 13:45 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ben Widawsky

The idea of printing objects used by each process is to judge how each
process is using them. This means that we need to evaluate whether the
object is bound for that particular process, rather than just whether it
is bound into the global GTT.

v2: Restore the non-full-ppgtt path for simplicity as we may not even
    create vma with older hardware.

v3: Tweak handling of global entries and default context entries.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ben Widawsky <benjamin.widawsky@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c     | 49 +++++++++++++++++++++++++++------
 drivers/gpu/drm/i915/i915_drv.h         |  2 ++
 drivers/gpu/drm/i915/i915_gem_context.c |  1 +
 3 files changed, 43 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 3e0d1906ef8d..4e1787ee8f37 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -299,28 +299,57 @@ static int i915_gem_stolen_list_info(struct seq_file *m, void *data)
 } while (0)
 
 struct file_stats {
+	struct drm_i915_file_private *file_priv;
 	int count;
-	size_t total, active, inactive, unbound;
+	size_t total, global, active, inactive, unbound;
 };
 
 static int per_file_stats(int id, void *ptr, void *data)
 {
 	struct drm_i915_gem_object *obj = ptr;
 	struct file_stats *stats = data;
+	struct i915_vma *vma;
 
 	stats->count++;
 	stats->total += obj->base.size;
 
-	if (i915_gem_obj_ggtt_bound(obj)) {
-		if (!list_empty(&obj->ring_list))
-			stats->active += obj->base.size;
-		else
-			stats->inactive += obj->base.size;
+	if (USES_FULL_PPGTT(obj->base.dev)) {
+		list_for_each_entry(vma, &obj->vma_list, vma_link) {
+			struct i915_hw_ppgtt *ppgtt;
+
+			if (!drm_mm_node_allocated(&vma->node))
+				continue;
+
+			if (i915_is_ggtt(vma->vm)) {
+				stats->global += obj->base.size;
+				continue;
+			}
+
+			ppgtt = container_of(vma->vm, struct i915_hw_ppgtt, base);
+			if (ppgtt->ctx && ppgtt->ctx->file_priv != stats->file_priv)
+				continue;
+
+			if (obj->ring) /* XXX per-vma statistic */
+				stats->active += obj->base.size;
+			else
+				stats->inactive += obj->base.size;
+
+			return 0;
+		}
 	} else {
-		if (!list_empty(&obj->global_list))
-			stats->unbound += obj->base.size;
+		if (i915_gem_obj_ggtt_bound(obj)) {
+			stats->global += obj->base.size;
+			if (obj->ring)
+				stats->active += obj->base.size;
+			else
+				stats->inactive += obj->base.size;
+			return 0;
+		}
 	}
 
+	if (!list_empty(&obj->global_list))
+		stats->unbound += obj->base.size;
+
 	return 0;
 }
 
@@ -411,6 +440,7 @@ static int i915_gem_object_info(struct seq_file *m, void* data)
 		struct task_struct *task;
 
 		memset(&stats, 0, sizeof(stats));
+		stats.file_priv = file->driver_priv;
 		idr_for_each(&file->object_idr, per_file_stats, &stats);
 		/*
 		 * Although we have a valid reference on file->pid, that does
@@ -420,12 +450,13 @@ static int i915_gem_object_info(struct seq_file *m, void* data)
 		 */
 		rcu_read_lock();
 		task = pid_task(file->pid, PIDTYPE_PID);
-		seq_printf(m, "%s: %u objects, %zu bytes (%zu active, %zu inactive, %zu unbound)\n",
+		seq_printf(m, "%s: %u objects, %zu bytes (%zu active, %zu inactive, %zu global, %zu unbound)\n",
 			   task ? task->comm : "<unknown>",
 			   stats.count,
 			   stats.total,
 			   stats.active,
 			   stats.inactive,
+			   stats.global,
 			   stats.unbound);
 		rcu_read_unlock();
 	}
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c4ab6d6ab926..950c242e49d5 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -730,6 +730,8 @@ struct i915_hw_ppgtt {
 		dma_addr_t *gen8_pt_dma_addr[4];
 	};
 
+	struct i915_hw_context *ctx;
+
 	int (*enable)(struct i915_hw_ppgtt *ppgtt);
 	int (*switch_mm)(struct i915_hw_ppgtt *ppgtt,
 			 struct intel_ring_buffer *ring,
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 40f430612324..b57b48cc5087 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -218,6 +218,7 @@ create_vm_for_ctx(struct drm_device *dev, struct i915_hw_context *ctx)
 		return ERR_PTR(ret);
 	}
 
+	ppgtt->ctx = ctx;
 	return ppgtt;
 }
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/2] drm/i915: Print how many objects are shared in per-process stats
  2014-03-19 13:45     ` [PATCH 1/2] " Chris Wilson
@ 2014-03-19 13:45       ` Chris Wilson
  2014-03-20  4:42         ` [PATCH 2/2] drm/i915: Print how many objects are shared in per-process statsg Ben Widawsky
  2014-03-19 17:30       ` [PATCH 1/2] drm/i915: Per-process stats work better when evaluated per-process Ben Widawsky
  1 sibling, 1 reply; 9+ messages in thread
From: Chris Wilson @ 2014-03-19 13:45 UTC (permalink / raw)
  To: intel-gfx

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 4e1787ee8f37..9cc1c9360238 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -301,7 +301,9 @@ static int i915_gem_stolen_list_info(struct seq_file *m, void *data)
 struct file_stats {
 	struct drm_i915_file_private *file_priv;
 	int count;
-	size_t total, global, active, inactive, unbound;
+	size_t total, unbound;
+	size_t global, shared;
+	size_t active, inactive;
 };
 
 static int per_file_stats(int id, void *ptr, void *data)
@@ -313,6 +315,9 @@ static int per_file_stats(int id, void *ptr, void *data)
 	stats->count++;
 	stats->total += obj->base.size;
 
+	if (obj->base.name || obj->base.dma_buf)
+		stats->shared += obj->base.size;
+
 	if (USES_FULL_PPGTT(obj->base.dev)) {
 		list_for_each_entry(vma, &obj->vma_list, vma_link) {
 			struct i915_hw_ppgtt *ppgtt;
@@ -450,13 +455,14 @@ static int i915_gem_object_info(struct seq_file *m, void* data)
 		 */
 		rcu_read_lock();
 		task = pid_task(file->pid, PIDTYPE_PID);
-		seq_printf(m, "%s: %u objects, %zu bytes (%zu active, %zu inactive, %zu global, %zu unbound)\n",
+		seq_printf(m, "%s: %u objects, %zu bytes (%zu active, %zu inactive, %zu global, %zu shared, %zu unbound)\n",
 			   task ? task->comm : "<unknown>",
 			   stats.count,
 			   stats.total,
 			   stats.active,
 			   stats.inactive,
 			   stats.global,
+			   stats.shared,
 			   stats.unbound);
 		rcu_read_unlock();
 	}
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/2] drm/i915: Per-process stats work better when evaluated per-process
  2014-03-19 13:45     ` [PATCH 1/2] " Chris Wilson
  2014-03-19 13:45       ` [PATCH 2/2] drm/i915: Print how many objects are shared in per-process stats Chris Wilson
@ 2014-03-19 17:30       ` Ben Widawsky
  1 sibling, 0 replies; 9+ messages in thread
From: Ben Widawsky @ 2014-03-19 17:30 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

On Wed, Mar 19, 2014 at 01:45:45PM +0000, Chris Wilson wrote:
> The idea of printing objects used by each process is to judge how each
> process is using them. This means that we need to evaluate whether the
> object is bound for that particular process, rather than just whether it
> is bound into the global GTT.
> 
> v2: Restore the non-full-ppgtt path for simplicity as we may not even
>     create vma with older hardware.
> 
> v3: Tweak handling of global entries and default context entries.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Ben Widawsky <benjamin.widawsky@intel.com>
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>

[snip]

-- 
Ben Widawsky, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] drm/i915: Print how many objects are shared in per-process statsg
  2014-03-19 13:45       ` [PATCH 2/2] drm/i915: Print how many objects are shared in per-process stats Chris Wilson
@ 2014-03-20  4:42         ` Ben Widawsky
  2014-03-20  7:40           ` Chris Wilson
  0 siblings, 1 reply; 9+ messages in thread
From: Ben Widawsky @ 2014-03-20  4:42 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

On Wed, Mar 19, 2014 at 01:45:46PM +0000, Chris Wilson wrote:
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Any clue how you intend to use this for a commit message (I'm actually
curious)? Also, the subject is wrong, you're counting size, not
quantity. Anyhoo, looks correct.

Reviewed-by: Ben Widawsky <ben@bwidawsk.net>

> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 4e1787ee8f37..9cc1c9360238 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -301,7 +301,9 @@ static int i915_gem_stolen_list_info(struct seq_file *m, void *data)
>  struct file_stats {
>  	struct drm_i915_file_private *file_priv;
>  	int count;
> -	size_t total, global, active, inactive, unbound;
> +	size_t total, unbound;
> +	size_t global, shared;
> +	size_t active, inactive;
>  };
>  
>  static int per_file_stats(int id, void *ptr, void *data)
> @@ -313,6 +315,9 @@ static int per_file_stats(int id, void *ptr, void *data)
>  	stats->count++;
>  	stats->total += obj->base.size;
>  
> +	if (obj->base.name || obj->base.dma_buf)
> +		stats->shared += obj->base.size;
> +
>  	if (USES_FULL_PPGTT(obj->base.dev)) {
>  		list_for_each_entry(vma, &obj->vma_list, vma_link) {
>  			struct i915_hw_ppgtt *ppgtt;
> @@ -450,13 +455,14 @@ static int i915_gem_object_info(struct seq_file *m, void* data)
>  		 */
>  		rcu_read_lock();
>  		task = pid_task(file->pid, PIDTYPE_PID);
> -		seq_printf(m, "%s: %u objects, %zu bytes (%zu active, %zu inactive, %zu global, %zu unbound)\n",
> +		seq_printf(m, "%s: %u objects, %zu bytes (%zu active, %zu inactive, %zu global, %zu shared, %zu unbound)\n",
>  			   task ? task->comm : "<unknown>",
>  			   stats.count,
>  			   stats.total,
>  			   stats.active,
>  			   stats.inactive,
>  			   stats.global,
> +			   stats.shared,
>  			   stats.unbound);
>  		rcu_read_unlock();
>  	}
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ben Widawsky, Intel Open Source Technology Center

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] drm/i915: Print how many objects are shared in per-process statsg
  2014-03-20  4:42         ` [PATCH 2/2] drm/i915: Print how many objects are shared in per-process statsg Ben Widawsky
@ 2014-03-20  7:40           ` Chris Wilson
  2014-03-20 14:11             ` Daniel Vetter
  0 siblings, 1 reply; 9+ messages in thread
From: Chris Wilson @ 2014-03-20  7:40 UTC (permalink / raw)
  To: Ben Widawsky; +Cc: intel-gfx

On Wed, Mar 19, 2014 at 09:42:49PM -0700, Ben Widawsky wrote:
> On Wed, Mar 19, 2014 at 01:45:46PM +0000, Chris Wilson wrote:
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> Any clue how you intend to use this for a commit message (I'm actually
> curious)? Also, the subject is wrong, you're counting size, not
> quantity. Anyhoo, looks correct.

Knowing how much of the allocated objects are shared between processes
helps gauge whether a process is leaking private objects, or if it
simply a display server suffering memory pressure from lots of clients.

> Reviewed-by: Ben Widawsky <ben@bwidawsk.net>

-- 
Chris Wilson, Intel Open Source Technology Centre

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 2/2] drm/i915: Print how many objects are shared in per-process statsg
  2014-03-20  7:40           ` Chris Wilson
@ 2014-03-20 14:11             ` Daniel Vetter
  0 siblings, 0 replies; 9+ messages in thread
From: Daniel Vetter @ 2014-03-20 14:11 UTC (permalink / raw)
  To: Chris Wilson, Ben Widawsky, intel-gfx

On Thu, Mar 20, 2014 at 07:40:37AM +0000, Chris Wilson wrote:
> On Wed, Mar 19, 2014 at 09:42:49PM -0700, Ben Widawsky wrote:
> > On Wed, Mar 19, 2014 at 01:45:46PM +0000, Chris Wilson wrote:
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > 
> > Any clue how you intend to use this for a commit message (I'm actually
> > curious)? Also, the subject is wrong, you're counting size, not
> > quantity. Anyhoo, looks correct.
> 
> Knowing how much of the allocated objects are shared between processes
> helps gauge whether a process is leaking private objects, or if it
> simply a display server suffering memory pressure from lots of clients.

I've added something along these lines here.
> 
> > Reviewed-by: Ben Widawsky <ben@bwidawsk.net>

Both patches merged, thanks.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2014-03-20 14:11 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-13 14:57 [PATCH 3/4] drm/i915: Per-process stats work better when evaluated per-process Rodrigo Vivi
2014-03-19  0:05 ` Ben Widawsky
2014-03-19  6:55   ` Chris Wilson
2014-03-19 13:45     ` [PATCH 1/2] " Chris Wilson
2014-03-19 13:45       ` [PATCH 2/2] drm/i915: Print how many objects are shared in per-process stats Chris Wilson
2014-03-20  4:42         ` [PATCH 2/2] drm/i915: Print how many objects are shared in per-process statsg Ben Widawsky
2014-03-20  7:40           ` Chris Wilson
2014-03-20 14:11             ` Daniel Vetter
2014-03-19 17:30       ` [PATCH 1/2] drm/i915: Per-process stats work better when evaluated per-process Ben Widawsky

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.