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

Joonas Lahtinen
2016-07-26T08:54:18Z

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? I do not feel
the API change intuitive at all as it is.

> Before: 6298417 3619610  696320 10614347         a1f64b vmlinux
> After: 6298417 3619610  696320 10614347         a1f64b vmlinux
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c     | 12 +++++---
>  drivers/gpu/drm/i915/i915_gem.c         | 49 ++++++++++++++++++++++-----------
>  drivers/gpu/drm/i915/i915_gem_fence.c   |  3 +-
>  drivers/gpu/drm/i915/i915_gem_request.h | 38 +++++++++++++++----------
>  drivers/gpu/drm/i915/i915_gem_tiling.c  |  3 +-
>  drivers/gpu/drm/i915/i915_gem_userptr.c |  3 +-
>  drivers/gpu/drm/i915/i915_gpu_error.c   | 29 +++++++++++++++----
>  drivers/gpu/drm/i915/intel_display.c    | 12 +++++---
>  8 files changed, 102 insertions(+), 47 deletions(-)
>
> 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?

Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation