On Thu, Aug 07, 2025 at 02:02:04PM -0300, Luiz Otavio Mello wrote: > The struct_mutex will be removed from the DRM subsystem, as it was a > legacy BKL that was only used by i915 driver. After review, it was > concluded that its usage was no longer necessary > > This patch updates various comments in the i915/gem and i915/gt > codebase to either remove or clarify references to struct_mutex, in > order to prevent future misunderstandings. > > * i915_gem_execbuffer.c: Replace reference to struct_mutex with > vm->mutex, as noted in the eb_reserve() function, which states that > vm->mutex handles deadlocks. > * i915_gem_object.c: Change struct_mutex by > drm_i915_gem_object->vma.lock. i915_gem_object_unbind() in i915_gem.c > states that this lock is who actually protects the unbind. > * i915_gem_shrinker.c: The correct lock is actually i915->mm.obj, as > already documented in its declaration. > * i915_gem_wait.c: The existing comment already mentioned that > struct_mutex was no longer necessary. Updated to refer to a generic > global lock instead. > * intel_reset_types.h: Cleaned up the comment text. Updated to refer to > a generic global lock instead. > > Signed-off-by: Luiz Otavio Mello <luiz.me...@estudante.ufscar.br>
Reviewed-by: Rodrigo Vivi <rodrigo.v...@intel.com> > --- > drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 2 +- > drivers/gpu/drm/i915/gem/i915_gem_object.c | 4 ++-- > drivers/gpu/drm/i915/gem/i915_gem_shrinker.c | 4 ++-- > drivers/gpu/drm/i915/gem/i915_gem_wait.c | 8 ++++---- > drivers/gpu/drm/i915/gt/intel_reset_types.h | 2 +- > 5 files changed, 10 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > index f243f8a5215d..39c7c32e1e74 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c > @@ -182,7 +182,7 @@ enum { > * the object. Simple! ... The relocation entries are stored in user memory > * and so to access them we have to copy them into a local buffer. That copy > * has to avoid taking any pagefaults as they may lead back to a GEM object > - * requiring the struct_mutex (i.e. recursive deadlock). So once again we > split > + * requiring the vm->mutex (i.e. recursive deadlock). So once again we split > * the relocation into multiple passes. First we try to do everything within > an > * atomic context (avoid the pagefaults) which requires that we never wait. > If > * we detect that we may wait, or if we need to fault, then we have to > fallback > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_object.c > b/drivers/gpu/drm/i915/gem/i915_gem_object.c > index 1f38e367c60b..478011e5ecb3 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_object.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_object.c > @@ -459,8 +459,8 @@ static void i915_gem_free_object(struct drm_gem_object > *gem_obj) > atomic_inc(&i915->mm.free_count); > > /* > - * Since we require blocking on struct_mutex to unbind the freed > - * object from the GPU before releasing resources back to the > + * Since we require blocking on drm_i915_gem_object->vma.lock to unbind > + * the freed object from the GPU before releasing resources back to the > * system, we can not do that directly from the RCU callback (which may > * be a softirq context), but must instead then defer that work onto a > * kthread. We use the RCU callback rather than move the freed object > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c > b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c > index b81e67504bbe..7a3e74a6676e 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c > @@ -170,7 +170,7 @@ i915_gem_shrink(struct i915_gem_ww_ctx *ww, > * Also note that although these lists do not hold a reference to > * the object we can safely grab one here: The final object > * unreferencing and the bound_list are both protected by the > - * dev->struct_mutex and so we won't ever be able to observe an > + * i915->mm.obj_lock and so we won't ever be able to observe an > * object on the bound_list with a reference count equals 0. > */ > for (phase = phases; phase->list; phase++) { > @@ -185,7 +185,7 @@ i915_gem_shrink(struct i915_gem_ww_ctx *ww, > > /* > * We serialize our access to unreferenced objects through > - * the use of the struct_mutex. While the objects are not > + * the use of the obj_lock. While the objects are not > * yet freed (due to RCU then a workqueue) we still want > * to be able to shrink their pages, so they remain on > * the unbound/bound list until actually freed. > diff --git a/drivers/gpu/drm/i915/gem/i915_gem_wait.c > b/drivers/gpu/drm/i915/gem/i915_gem_wait.c > index 991666fd9f85..54829801d3f7 100644 > --- a/drivers/gpu/drm/i915/gem/i915_gem_wait.c > +++ b/drivers/gpu/drm/i915/gem/i915_gem_wait.c > @@ -217,10 +217,10 @@ static unsigned long to_wait_timeout(s64 timeout_ns) > * > * The wait ioctl with a timeout of 0 reimplements the busy ioctl. With any > * non-zero timeout parameter the wait ioctl will wait for the given number > of > - * nanoseconds on an object becoming unbusy. Since the wait itself does so > - * without holding struct_mutex the object may become re-busied before this > - * function completes. A similar but shorter * race condition exists in the > busy > - * ioctl > + * nanoseconds on an object becoming unbusy. Since the wait occurs without > + * holding a global or exclusive lock the object may become re-busied before > + * this function completes. A similar but shorter * race condition exists > + * in the busy ioctl > */ > int > i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file > *file) > diff --git a/drivers/gpu/drm/i915/gt/intel_reset_types.h > b/drivers/gpu/drm/i915/gt/intel_reset_types.h > index 4f5fd393af6f..ee4eb574a219 100644 > --- a/drivers/gpu/drm/i915/gt/intel_reset_types.h > +++ b/drivers/gpu/drm/i915/gt/intel_reset_types.h > @@ -20,7 +20,7 @@ struct intel_reset { > * FENCE registers). > * > * #I915_RESET_ENGINE[num_engines] - Since the driver doesn't need to > - * acquire the struct_mutex to reset an engine, we need an explicit > + * acquire a global lock to reset an engine, we need an explicit > * flag to prevent two concurrent reset attempts in the same engine. > * As the number of engines continues to grow, allocate the flags from > * the most significant bits. > -- > 2.50.1 >