Re: [PATCH 39/55] drm/i915: Mark up i915_gem_active for locking annotation

Joonas Lahtinen
2016-07-28T07:26:52Z

On ti, 2016-07-26 at 10:06 +0100, Chris Wilson wrote:
> On Tue, Jul 26, 2016 at 11:54:16AM +0300, Joonas Lahtinen wrote:
> >
> > On ma, 2016-07-25 at 18:32 +0100, Chris Wilson wrote:
> > >
> > > The future annotations will track the locking used for access to ensure
> > > that it is always sufficient. We make the preparations now to present
> > > the API ahead and to make sure that GCC can eliminate the unused
> > > parameter.
> > >
> > Is it at some point going to be other than struct_mutex?
> Yes.
>
> >
> > I do not feel
> > the API change intuitive at all as it is.
> The API change here is solely for RCU markup later, i.e. we can access
> the active.request lockless but have to be very careful when we do.
>
> >
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > > index b41561bdfb85..16fa1f527ef5 100644
> > > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > > @@ -155,10 +155,13 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj)
> > >      obj->base.write_domain);
> > >   for_each_engine_id(engine, dev_priv, id)
> > >   seq_printf(m, "%x ",
> > > -    i915_gem_active_get_seqno(&obj->last_read[id]));
> > > +    i915_gem_active_get_seqno(&obj->last_read[id],
> > > +      &obj->base.dev->struct_mutex));
> > In functions where you use plenty of this, maybe make struct_mutex
> > alias. But before that, what's wrong with passing dev_priv?
> What dev_priv? See earlier answer about this should not be struct_mutex
> in the long run.

Ok, then it's acceptable intermediary state.

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

> -Chris
>
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation