[Intel-gfx] [PATCH 2/5] drm/i915: Track i915_active using debugobjects

Chris Wilson chris at chris-wilson.co.uk
Fri Jun 21 15:30:53 UTC 2019


Quoting Matthew Auld (2019-06-21 16:25:05)
> On 21/06/2019 14:05, Chris Wilson wrote:
> > Provide runtime asserts and tracking of i915_active via debugobjects.
> > For example, this should allow us to check that the i915_active is only
> > active when we expect it to be and is never freed too early.
> > 
> > One consequence is that, for simplicity, we no longer allow i915_active
> > to be on-stack which only affected the selftests.
> > 
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > ---
> >   drivers/gpu/drm/i915/i915_active.c           | 66 ++++++++++++++++-
> >   drivers/gpu/drm/i915/selftests/i915_active.c | 78 +++++++++++++++-----
> >   2 files changed, 123 insertions(+), 21 deletions(-)
> > 
> 
> [snip]
> 
> > +
> > +static struct live_active *
> > +__live_active_setup(struct drm_i915_private *i915)
> >   {
> >       struct intel_engine_cs *engine;
> >       struct i915_sw_fence *submit;
> > +     struct live_active *active;
> >       enum intel_engine_id id;
> >       unsigned int count = 0;
> >       int err = 0;
> >   
> > -     submit = heap_fence_create(GFP_KERNEL);
> > -     if (!submit)
> > -             return -ENOMEM;
> > +     active = __live_alloc(i915);
> > +     if (!active)
> > +             return ERR_PTR(-ENOMEM);
> >   
> > -     i915_active_init(i915, &active->base, __live_active_retire);
> > -     active->retired = false;
> > +     submit = heap_fence_create(GFP_KERNEL);
> > +     if (!submit) {
> > +             kfree(active);
> > +             return ERR_PTR(-ENOMEM);
> > +     }
> >   
> >       if (!i915_active_acquire(&active->base)) {
> >               pr_err("First i915_active_acquire should report being idle\n");
> > @@ -84,64 +107,79 @@ static int __live_active_setup(struct drm_i915_private *i915,
> >       i915_sw_fence_commit(submit);
> >       heap_fence_put(submit);
> >   
> > -     return err;
> > +     /* XXX leaks live_active on error */
> > +     return err ? ERR_PTR(err) : active
> 
> Too much of a pain?

In the next patch (with an active callback), it grew a ref count and
gets cleaned up automatically. Could have pulled it into this patch, but
too much effort for a hopefully never seen result.
-Chris


More information about the Intel-gfx mailing list