Re: [Intel-xe] [PATCH v5 1/7] drm: fix drmm_mutex_init()

[Stanislaw Gruszka <stanislaw.gruszka@linux.intel.com>]

On Wed, May 17, 2023 at 04:22:38PM +0100, Matthew Auld wrote:
> In mutex_init() lockdep seems to identify a lock by defining a static
> key for each lock class. However if we wrap the whole thing in a
> function the static key will be the same for everything calling that
> function, which looks to be the case for drmm_mutex_init(). This then
> results in impossible lockdep splats since lockdep thinks completely
> unrelated locks are the same lock class. The other issue is that when
> looking at splats we lose the actual lock name, where instead of seeing
> something like xe->mem_access.lock for the name, we just see something
> generic like lock#8.
> 
> Attempt to fix this by converting drmm_mutex_init() into a macro, which
> should ensure that mutex_init() behaves as expected.

Nice catch :-) we observed lockdep deadlock false alarms too, but I could
not spot it out and were adding lockdep_set_class(key) to avoid those.


> +static inline void __drmm_mutex_release(struct drm_device *dev, void *res)

Can this be inline if used in drmm_add_action_or_reset() ? 


> +{
> +	struct mutex *lock = res;
> +
> +	mutex_destroy(lock);
> +}
> +
> +/**
> + * drmm_mutex_init - &drm_device-managed mutex_init()
> + * @dev: DRM device
> + * @lock: lock to be initialized
> + *
> + * Returns:
> + * 0 on success, or a negative errno code otherwise.
> + *
> + * This is a &drm_device-managed version of mutex_init(). The initialized
> + * lock is automatically destroyed on the final drm_dev_put().
> + */
> +#define drmm_mutex_init(dev, lock) ({					     \
> +	mutex_init(lock);						     \
> +	drmm_add_action_or_reset(dev, __drmm_mutex_release, lock);	     \
> +})									     \

Regards
Stanislaw