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