Re: [PATCH 4/9 v2] drm/i915: Replace struct_mutex in intel_guc_log.c

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Thu, Aug 07, 2025 at 02:02:03PM -0300, Luiz Otavio Mello wrote:
> Remove the use of struct_mutex from intel_guc_log.c and replace it with
> a dedicated lock, guc_lock, defined within the intel_guc_log struct.
>     
> The struct_mutex was previously used to protect concurrent access and
> modification of intel_guc_log->level in intel_guc_log_set_level().
> However, it was suggested that the lock should reside within the
> intel_guc_log struct itself.
>     
> Initialize the new guc_lock in intel_guc_log_init_early(), alongside the
> existing relay.lock. The lock is initialized using drmm_mutex_init(),
> which also ensures it is properly destroyed when the driver is unloaded.
> 
> Signed-off-by: Luiz Otavio Mello <luiz.mello@estudante.ufscar.br>
> Suggested-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  drivers/gpu/drm/i915/gt/uc/intel_guc_log.c | 5 +++--
>  drivers/gpu/drm/i915/gt/uc/intel_guc_log.h | 8 ++++++++
>  2 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
> index 469173791394..0104fffd5852 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
> @@ -518,6 +518,7 @@ void intel_guc_log_init_early(struct intel_guc_log *log)
>  	struct drm_i915_private *i915 = guc_to_i915(guc);
>  
>  	drmm_mutex_init(&i915->drm, &log->relay.lock);
> +	drmm_mutex_init(&i915->drm, &log->guc_lock);
>  	INIT_WORK(&log->relay.flush_work, copy_debug_logs_work);
>  	log->relay.started = false;
>  }
> @@ -683,7 +684,7 @@ int intel_guc_log_set_level(struct intel_guc_log *log, u32 level)
>  	if (level < GUC_LOG_LEVEL_DISABLED || level > GUC_LOG_LEVEL_MAX)
>  		return -EINVAL;
>  
> -	mutex_lock(&i915->struct_mutex);
> +	mutex_lock(&log->guc_lock);
>  
>  	if (log->level == level)
>  		goto out_unlock;
> @@ -701,7 +702,7 @@ int intel_guc_log_set_level(struct intel_guc_log *log, u32 level)
>  	log->level = level;
>  
>  out_unlock:
> -	mutex_unlock(&i915->struct_mutex);
> +	mutex_unlock(&log->guc_lock);
>  
>  	return ret;
>  }
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.h
> index 02127703be80..13cb93ad0710 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.h
> @@ -42,6 +42,14 @@ enum {
>  struct intel_guc_log {
>  	u32 level;
>  
> +	/*
> +	 * Protects concurrent access and modification of intel_guc_log->level.
> +	 *
> +	 * This lock replaces the legacy struct_mutex usage in
> +	 * intel_guc_log system.
> +	 */
> +	struct mutex guc_lock;
> +
>  	/* Allocation settings */
>  	struct {
>  		s32 bytes;	/* Size in bytes */
> -- 
> 2.50.1
>