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

Chris Wilson
2016-07-26T09:06:06Z

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.
-Chris

--
Chris Wilson, Intel Open Source Technology Centre