[PATCH 1/3] Revert "drm/xe: Add mutex locking to devcoredump"

[Thomas Hellström]

On Wed, 2024-11-27 at 14:00 +0530, Himal Prasad Ghimiray wrote:
> This reverts commit b9428d0ac54fcece5ad8fc86c5d3e7aacc6e2c76. The
> commit
> was accidentally and unintentionally pushed.
> 
> Cc: John Harrison <John.C.Harrison at Intel.com>
> Cc: Matthew Brost <matthew.brost at intel.com>
> Cc: Thomas Hellström <thomas.hellstrom at linux.intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> Signed-off-by: Himal Prasad Ghimiray
> <himal.prasad.ghimiray at intel.com>

Reviewed-by: Thomas Hellström <thomas.hellstrom at linux.intel.com>


> ---
>  drivers/gpu/drm/xe/xe_devcoredump.c       | 33 ++-------------------
> --
>  drivers/gpu/drm/xe/xe_devcoredump_types.h |  4 +--
>  2 files changed, 3 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_devcoredump.c
> b/drivers/gpu/drm/xe/xe_devcoredump.c
> index dd6a4f591ee0..5d19a4e3d5af 100644
> --- a/drivers/gpu/drm/xe/xe_devcoredump.c
> +++ b/drivers/gpu/drm/xe/xe_devcoredump.c
> @@ -182,24 +182,16 @@ static ssize_t xe_devcoredump_read(char
> *buffer, loff_t offset,
>  	/* Ensure delayed work is captured before continuing */
>  	flush_work(&ss->work);
>  
> -	mutex_lock(&coredump->lock);
> -
> -	if (!ss->read.buffer) {
> -		mutex_unlock(&coredump->lock);
> +	if (!ss->read.buffer)
>  		return -ENODEV;
> -	}
>  
> -	if (offset >= ss->read.size) {
> -		mutex_unlock(&coredump->lock);
> +	if (offset >= ss->read.size)
>  		return 0;
> -	}
>  
>  	byte_copied = count < ss->read.size - offset ? count :
>  		ss->read.size - offset;
>  	memcpy(buffer, ss->read.buffer + offset, byte_copied);
>  
> -	mutex_unlock(&coredump->lock);
> -
>  	return byte_copied;
>  }
>  
> @@ -213,8 +205,6 @@ static void xe_devcoredump_free(void *data)
>  
>  	cancel_work_sync(&coredump->snapshot.work);
>  
> -	mutex_lock(&coredump->lock);
> -
>  	xe_devcoredump_snapshot_free(&coredump->snapshot);
>  	kvfree(coredump->snapshot.read.buffer);
>  
> @@ -223,8 +213,6 @@ static void xe_devcoredump_free(void *data)
>  	coredump->captured = false;
>  	drm_info(&coredump_to_xe(coredump)->drm,
>  		 "Xe device coredump has been deleted.\n");
> -
> -	mutex_unlock(&coredump->lock);
>  }
>  
>  static void xe_devcoredump_deferred_snap_work(struct work_struct
> *work)
> @@ -333,11 +321,8 @@ void xe_devcoredump(struct xe_exec_queue *q,
> struct xe_sched_job *job, const cha
>  	struct xe_devcoredump *coredump = &xe->devcoredump;
>  	va_list varg;
>  
> -	mutex_lock(&coredump->lock);
> -
>  	if (coredump->captured) {
>  		drm_dbg(&xe->drm, "Multiple hangs are occurring, but
> only the first snapshot was taken\n");
> -		mutex_unlock(&coredump->lock);
>  		return;
>  	}
>  
> @@ -352,8 +337,6 @@ void xe_devcoredump(struct xe_exec_queue *q,
> struct xe_sched_job *job, const cha
>  	drm_info(&xe->drm, "Xe device coredump has been created\n");
>  	drm_info(&xe->drm, "Check your
> /sys/class/drm/card%d/device/devcoredump/data\n",
>  		 xe->drm.primary->index);
> -
> -	mutex_unlock(&coredump->lock);
>  }
>  
>  static void xe_driver_devcoredump_fini(void *arg)
> @@ -365,18 +348,6 @@ static void xe_driver_devcoredump_fini(void
> *arg)
>  
>  int xe_devcoredump_init(struct xe_device *xe)
>  {
> -	int err;
> -
> -	err = drmm_mutex_init(&xe->drm, &xe->devcoredump.lock);
> -	if (err)
> -		return err;
> -
> -	if (IS_ENABLED(CONFIG_LOCKDEP)) {
> -		fs_reclaim_acquire(GFP_KERNEL);
> -		might_lock(&xe->devcoredump.lock);
> -		fs_reclaim_release(GFP_KERNEL);
> -	}
> -
>  	return devm_add_action_or_reset(xe->drm.dev,
> xe_driver_devcoredump_fini, &xe->drm);
>  }
>  
> diff --git a/drivers/gpu/drm/xe/xe_devcoredump_types.h
> b/drivers/gpu/drm/xe/xe_devcoredump_types.h
> index 1a1d16a96b2d..e6234e887102 100644
> --- a/drivers/gpu/drm/xe/xe_devcoredump_types.h
> +++ b/drivers/gpu/drm/xe/xe_devcoredump_types.h
> @@ -80,9 +80,7 @@ struct xe_devcoredump_snapshot {
>   * for reading the information.
>   */
>  struct xe_devcoredump {
> -	/** @lock: protects access to entire structure */
> -	struct mutex lock;
> -	/** @captured: The snapshot of the first hang has already
> been taken */
> +	/** @captured: The snapshot of the first hang has already
> been taken. */
>  	bool captured;
>  	/** @snapshot: Snapshot is captured at time of the first
> crash */
>  	struct xe_devcoredump_snapshot snapshot;