| Message ID | 20190130205039.20959-2-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/3] drm/i915: Generalise GPU activity tracking | expand |
On 30/01/2019 20:50, Chris Wilson wrote: > As soon as we detect that the active tracker is idle and we prepare to > call the retire callback, release the storage for our tree of > per-timeline nodes. We expect these to be infrequently usage and quick s/usage/used/ > to allocate, so there is little benefit in keeping the tree cached and > we would prefer to return the pages back to the system in a timely > fashion. > > This also means that when we finalize the struct as a whole, we know as > the activity tracker must be idle, the tree has already been released. > Indeed we can reduce i915_active_fini() just the assertions that there > is nothing to do. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > --- > drivers/gpu/drm/i915/i915_active.c | 33 +++++++++++++++++++++--------- > drivers/gpu/drm/i915/i915_active.h | 4 ++++ > 2 files changed, 27 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c > index 91950d778cab..b1fefe98f9a6 100644 > --- a/drivers/gpu/drm/i915/i915_active.c > +++ b/drivers/gpu/drm/i915/i915_active.c > @@ -16,12 +16,29 @@ struct active_node { > u64 timeline; > }; > > +static void > +__active_park(struct i915_active *ref) > +{ > + struct active_node *it, *n; > + > + rbtree_postorder_for_each_entry_safe(it, n, &ref->tree, node) { > + GEM_BUG_ON(i915_gem_active_isset(&it->base)); > + kfree(it); > + } > + ref->tree = RB_ROOT; > +} > + > static void > __active_retire(struct i915_active *ref) > { > GEM_BUG_ON(!ref->count); > - if (!--ref->count) > - ref->retire(ref); > + if (--ref->count) > + return; > + > + /* return the unused nodes to our slabcache */ > + __active_park(ref); > + > + ref->retire(ref); > } > > static void > @@ -210,18 +227,14 @@ int i915_request_await_active(struct i915_request *rq, struct i915_active *ref) > return 0; > } > > +#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM) > void i915_active_fini(struct i915_active *ref) > { > - struct active_node *it, *n; > - > GEM_BUG_ON(i915_gem_active_isset(&ref->last)); > - > - rbtree_postorder_for_each_entry_safe(it, n, &ref->tree, node) { > - GEM_BUG_ON(i915_gem_active_isset(&it->base)); > - kfree(it); > - } > - ref->tree = RB_ROOT; > + GEM_BUG_ON(!RB_EMPTY_ROOT(&ref->tree)); > + GEM_BUG_ON(ref->count); > } > +#endif > > #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) > #include "selftests/i915_active.c" > diff --git a/drivers/gpu/drm/i915/i915_active.h b/drivers/gpu/drm/i915/i915_active.h > index 8bd7cf757d1a..6130c6770d10 100644 > --- a/drivers/gpu/drm/i915/i915_active.h > +++ b/drivers/gpu/drm/i915/i915_active.h > @@ -57,6 +57,10 @@ i915_active_is_idle(const struct i915_active *ref) > return !ref->count; > } > > +#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM) > void i915_active_fini(struct i915_active *ref); > +#else > +static inline void i915_active_fini(struct i915_active *ref) { } > +#endif > > #endif /* _I915_ACTIVE_H_ */ > Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Regards, Tvrtko
diff --git a/drivers/gpu/drm/i915/i915_active.c b/drivers/gpu/drm/i915/i915_active.c index 91950d778cab..b1fefe98f9a6 100644 --- a/drivers/gpu/drm/i915/i915_active.c +++ b/drivers/gpu/drm/i915/i915_active.c @@ -16,12 +16,29 @@ struct active_node { u64 timeline; }; +static void +__active_park(struct i915_active *ref) +{ + struct active_node *it, *n; + + rbtree_postorder_for_each_entry_safe(it, n, &ref->tree, node) { + GEM_BUG_ON(i915_gem_active_isset(&it->base)); + kfree(it); + } + ref->tree = RB_ROOT; +} + static void __active_retire(struct i915_active *ref) { GEM_BUG_ON(!ref->count); - if (!--ref->count) - ref->retire(ref); + if (--ref->count) + return; + + /* return the unused nodes to our slabcache */ + __active_park(ref); + + ref->retire(ref); } static void @@ -210,18 +227,14 @@ int i915_request_await_active(struct i915_request *rq, struct i915_active *ref) return 0; } +#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM) void i915_active_fini(struct i915_active *ref) { - struct active_node *it, *n; - GEM_BUG_ON(i915_gem_active_isset(&ref->last)); - - rbtree_postorder_for_each_entry_safe(it, n, &ref->tree, node) { - GEM_BUG_ON(i915_gem_active_isset(&it->base)); - kfree(it); - } - ref->tree = RB_ROOT; + GEM_BUG_ON(!RB_EMPTY_ROOT(&ref->tree)); + GEM_BUG_ON(ref->count); } +#endif #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) #include "selftests/i915_active.c" diff --git a/drivers/gpu/drm/i915/i915_active.h b/drivers/gpu/drm/i915/i915_active.h index 8bd7cf757d1a..6130c6770d10 100644 --- a/drivers/gpu/drm/i915/i915_active.h +++ b/drivers/gpu/drm/i915/i915_active.h @@ -57,6 +57,10 @@ i915_active_is_idle(const struct i915_active *ref) return !ref->count; } +#if IS_ENABLED(CONFIG_DRM_I915_DEBUG_GEM) void i915_active_fini(struct i915_active *ref); +#else +static inline void i915_active_fini(struct i915_active *ref) { } +#endif #endif /* _I915_ACTIVE_H_ */
As soon as we detect that the active tracker is idle and we prepare to call the retire callback, release the storage for our tree of per-timeline nodes. We expect these to be infrequently usage and quick to allocate, so there is little benefit in keeping the tree cached and we would prefer to return the pages back to the system in a timely fashion. This also means that when we finalize the struct as a whole, we know as the activity tracker must be idle, the tree has already been released. Indeed we can reduce i915_active_fini() just the assertions that there is nothing to do. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> --- drivers/gpu/drm/i915/i915_active.c | 33 +++++++++++++++++++++--------- drivers/gpu/drm/i915/i915_active.h | 4 ++++ 2 files changed, 27 insertions(+), 10 deletions(-)