Re: [PATCH 13/20] drm/xe/coredump: move over to devm

[Andrzej Hajda <andrzej.hajda@intel.com>]

On 10.05.2024 20:12, Matthew Auld wrote:
> Here we are using drmm to ensure we release the coredump when unloading
> the module, however the coredump is very much tied to the struct device
> underneath. We can see this when we hotunplug the device, for which we
> have already got a coredump attached. In such a case the coredump still
> remains and adding another is not possible. However we still register
> the release action via xe_driver_devcoredump_fini(), so in effect two or
> more releases for one dump.  The other consideration is that the
> coredump state is embedded in the xe_driver instance, so technically
> once the drmm release action fires we might free the coredumpe state
> from a different driver instance, assuming we have two release actions
> and they can race. Rather use devm here to remove the coredump when the
> device is released.
> 
> References: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/1679
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Andrzej Hajda <andrzej.hajda@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>

Reviewed-by: Andrzej Hajda <andrzej.hajda@intel.com>

Regards
Andrzej
> ---
>   drivers/gpu/drm/xe/xe_devcoredump.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_devcoredump.c b/drivers/gpu/drm/xe/xe_devcoredump.c
> index 3d7980232be1..e70aef797193 100644
> --- a/drivers/gpu/drm/xe/xe_devcoredump.c
> +++ b/drivers/gpu/drm/xe/xe_devcoredump.c
> @@ -238,13 +238,15 @@ void xe_devcoredump(struct xe_sched_job *job)
>   		      xe_devcoredump_read, xe_devcoredump_free);
>   }
>   
> -static void xe_driver_devcoredump_fini(struct drm_device *drm, void *arg)
> +static void xe_driver_devcoredump_fini(void *arg)
>   {
> +	struct drm_device *drm = arg;
> +
>   	dev_coredump_put(drm->dev);
>   }
>   
>   int xe_devcoredump_init(struct xe_device *xe)
>   {
> -	return drmm_add_action_or_reset(&xe->drm, xe_driver_devcoredump_fini, xe);
> +	return devm_add_action_or_reset(xe->drm.dev, xe_driver_devcoredump_fini, &xe->drm);
>   }
>   #endif