| Message ID | 1443698309-28038-2-git-send-email-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Thu, Oct 01, 2015 at 12:18:26PM +0100, Chris Wilson wrote: > Often it is very useful to know why we suddenly purge vast tracts of > memory and surprisingly up until now we didn't even have a tracepoint > for when we shrink our memory. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/i915_gem_shrinker.c | 2 ++ > drivers/gpu/drm/i915/i915_trace.h | 20 ++++++++++++++++++++ > 2 files changed, 22 insertions(+) > > diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c > index b627d07fad29..88f66a2586ec 100644 > --- a/drivers/gpu/drm/i915/i915_gem_shrinker.c > +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c > @@ -85,6 +85,8 @@ i915_gem_shrink(struct drm_i915_private *dev_priv, > }, *phase; > unsigned long count = 0; > > + trace_i915_gem_shrink(dev_priv, target, flags); Shouldn't we also dump how many pages we actually managed to shrink, i.e. count (at the end of the functions). Also we have a slab_start/end tracepoint already, but that one obviously doesn't cover the internal calls to i915_gem_shrink. Should imo be mentioned in the commit message. -Daniel > + > /* > * As we may completely rewrite the (un)bound list whilst unbinding > * (due to retiring requests) we have to strictly process only > diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h > index e6b5c7470ba0..ed7f42f2e740 100644 > --- a/drivers/gpu/drm/i915/i915_trace.h > +++ b/drivers/gpu/drm/i915/i915_trace.h > @@ -107,6 +107,26 @@ TRACE_EVENT(i915_gem_object_create, > TP_printk("obj=%p, size=%u", __entry->obj, __entry->size) > ); > > +TRACE_EVENT(i915_gem_shrink, > + TP_PROTO(struct drm_i915_private *i915, unsigned long target, unsigned flags), > + TP_ARGS(i915, target, flags), > + > + TP_STRUCT__entry( > + __field(int, dev) > + __field(unsigned long, target) > + __field(unsigned, flags) > + ), > + > + TP_fast_assign( > + __entry->dev = i915->dev->primary->index; > + __entry->target = target; > + __entry->flags = flags; > + ), > + > + TP_printk("dev=%d, target=%lu, flags=%x", > + __entry->dev, __entry->target, __entry->flags) > +); > + > TRACE_EVENT(i915_vma_bind, > TP_PROTO(struct i915_vma *vma, unsigned flags), > TP_ARGS(vma, flags), > -- > 2.6.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, Oct 06, 2015 at 02:54:25PM +0200, Daniel Vetter wrote: > On Thu, Oct 01, 2015 at 12:18:26PM +0100, Chris Wilson wrote: > > Often it is very useful to know why we suddenly purge vast tracts of > > memory and surprisingly up until now we didn't even have a tracepoint > > for when we shrink our memory. > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > --- > > drivers/gpu/drm/i915/i915_gem_shrinker.c | 2 ++ > > drivers/gpu/drm/i915/i915_trace.h | 20 ++++++++++++++++++++ > > 2 files changed, 22 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c > > index b627d07fad29..88f66a2586ec 100644 > > --- a/drivers/gpu/drm/i915/i915_gem_shrinker.c > > +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c > > @@ -85,6 +85,8 @@ i915_gem_shrink(struct drm_i915_private *dev_priv, > > }, *phase; > > unsigned long count = 0; > > > > + trace_i915_gem_shrink(dev_priv, target, flags); > > Shouldn't we also dump how many pages we actually managed to shrink, i.e. > count (at the end of the functions). I didn't because I find the double tracepoints annoying, and you already have the unbinds following. I guess shrink_begin, shrink_end (to be consistent with wait_begin/_end or shrink_start/_end to be consistent with slab). > Also we have a slab_start/end tracepoint already, but that one obviously > doesn't cover the internal calls to i915_gem_shrink. Should imo be > mentioned in the commit message. Sure, I don't usually watch slab, so I don't have a marker for the thousand unbinds as to what caused them. -Chris
On Tue, Oct 06, 2015 at 02:16:56PM +0100, Chris Wilson wrote: > On Tue, Oct 06, 2015 at 02:54:25PM +0200, Daniel Vetter wrote: > > On Thu, Oct 01, 2015 at 12:18:26PM +0100, Chris Wilson wrote: > > > Often it is very useful to know why we suddenly purge vast tracts of > > > memory and surprisingly up until now we didn't even have a tracepoint > > > for when we shrink our memory. > > > > > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > > --- > > > drivers/gpu/drm/i915/i915_gem_shrinker.c | 2 ++ > > > drivers/gpu/drm/i915/i915_trace.h | 20 ++++++++++++++++++++ > > > 2 files changed, 22 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c > > > index b627d07fad29..88f66a2586ec 100644 > > > --- a/drivers/gpu/drm/i915/i915_gem_shrinker.c > > > +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c > > > @@ -85,6 +85,8 @@ i915_gem_shrink(struct drm_i915_private *dev_priv, > > > }, *phase; > > > unsigned long count = 0; > > > > > > + trace_i915_gem_shrink(dev_priv, target, flags); > > > > Shouldn't we also dump how many pages we actually managed to shrink, i.e. > > count (at the end of the functions). > > I didn't because I find the double tracepoints annoying, and you already > have the unbinds following. > > I guess shrink_begin, shrink_end (to be consistent with wait_begin/_end > or shrink_start/_end to be consistent with slab). I meant moving the tracepoint to the end of the function where we both know how much core mm asked us to shrink and how much we actually managed to unshrink. But watching the unbind tracepoints is good enough for that too. > > Also we have a slab_start/end tracepoint already, but that one obviously > > doesn't cover the internal calls to i915_gem_shrink. Should imo be > > mentioned in the commit message. > > Sure, I don't usually watch slab, so I don't have a marker for the > thousand unbinds as to what caused them. I've captured the above two items in a note in the commit message and applied the patch. Thanks, Daniel
diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c index b627d07fad29..88f66a2586ec 100644 --- a/drivers/gpu/drm/i915/i915_gem_shrinker.c +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c @@ -85,6 +85,8 @@ i915_gem_shrink(struct drm_i915_private *dev_priv, }, *phase; unsigned long count = 0; + trace_i915_gem_shrink(dev_priv, target, flags); + /* * As we may completely rewrite the (un)bound list whilst unbinding * (due to retiring requests) we have to strictly process only diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h index e6b5c7470ba0..ed7f42f2e740 100644 --- a/drivers/gpu/drm/i915/i915_trace.h +++ b/drivers/gpu/drm/i915/i915_trace.h @@ -107,6 +107,26 @@ TRACE_EVENT(i915_gem_object_create, TP_printk("obj=%p, size=%u", __entry->obj, __entry->size) ); +TRACE_EVENT(i915_gem_shrink, + TP_PROTO(struct drm_i915_private *i915, unsigned long target, unsigned flags), + TP_ARGS(i915, target, flags), + + TP_STRUCT__entry( + __field(int, dev) + __field(unsigned long, target) + __field(unsigned, flags) + ), + + TP_fast_assign( + __entry->dev = i915->dev->primary->index; + __entry->target = target; + __entry->flags = flags; + ), + + TP_printk("dev=%d, target=%lu, flags=%x", + __entry->dev, __entry->target, __entry->flags) +); + TRACE_EVENT(i915_vma_bind, TP_PROTO(struct i915_vma *vma, unsigned flags), TP_ARGS(vma, flags),
Often it is very useful to know why we suddenly purge vast tracts of memory and surprisingly up until now we didn't even have a tracepoint for when we shrink our memory. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/i915_gem_shrinker.c | 2 ++ drivers/gpu/drm/i915/i915_trace.h | 20 ++++++++++++++++++++ 2 files changed, 22 insertions(+)