Re: [PATCH v3 01/14] drm/i915: Give each sw_fence its own lockclass

[Joonas Lahtinen <joonas.lahtinen@linux.intel.com>]

On ma, 2016-11-14 at 08:56 +0000, Chris Wilson wrote:
> Localise the static struct lock_class_key to the caller of
> i915_sw_fence_init() so that we create a lock_class instance for each
> unique sw_fence rather than all sw_fences sharing the same
> lock_class. This eliminate some lockdep false positive when using fences
> from within fence callbacks.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

<SNIP>

> @@ -40,7 +40,16 @@ typedef int (*i915_sw_fence_notify_t)(struct i915_sw_fence *,
>  				      enum i915_sw_fence_notify state);
>  #define __i915_sw_fence_call __aligned(4)
>  
> -void i915_sw_fence_init(struct i915_sw_fence *fence, i915_sw_fence_notify_t fn);
> +void __i915_sw_fence_init(struct i915_sw_fence *fence,
> +			  i915_sw_fence_notify_t fn,
> +			  const char *name,
> +			  struct lock_class_key *key);
> +#define i915_sw_fence_init(fence, fn) do {			\

Gimme a (line) break here.

> +	static struct lock_class_key __key; 			\

When lockdep is disabled, this becomes zero size. We might still get
rid of the #fence strings, with some #ifdef, did you measure the
impact? I remember some for_each_engine_masked cry over bytes.

> +								\
> +	__i915_sw_fence_init((fence), fn, #fence, &__key);	\
> +} while (0)
> +

Above addressed, and assuming we're not compiling in extra;

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

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx