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
>
