Re: [PATCH v1 3/3] drm/xe/drm_ras: Add per node cleanup action

["Tauro, Riana" <riana.tauro@intel.com>]

On 5/15/2026 1:58 AM, Raag Jadav wrote:
> cleanup_node_param() is not registered in case of counter allocation
> failure, which results in stale memory of previous node that isn't
> cleaned up on unwind.

It is registered.

ret = assign_node_params(xe, node, i);
if (ret)
     cleanup_node_param(ras, i);



>   Add per node cleanup action which guarantees
> cleanup on unwind and also simplifies the cleanup logic.
>
> Fixes: b40db12b542f ("drm/xe/xe_drm_ras: Add support for XE DRM RAS")
> Signed-off-by: Raag Jadav <raag.jadav@intel.com>
> ---
>   drivers/gpu/drm/xe/xe_drm_ras.c | 42 +++++++++++++--------------------
>   1 file changed, 17 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_drm_ras.c b/drivers/gpu/drm/xe/xe_drm_ras.c
> index 89640ffb1c33..40abde29a26f 100644
> --- a/drivers/gpu/drm/xe/xe_drm_ras.c
> +++ b/drivers/gpu/drm/xe/xe_drm_ras.c
> @@ -131,14 +131,20 @@ static int assign_node_params(struct xe_device *xe, struct drm_ras_node *node,
>   	return 0;
>   }
>   
> -static void cleanup_node_param(struct xe_drm_ras *ras, const enum drm_xe_ras_error_severity severity)
> +static void cleanup_node_param(struct drm_ras_node *node)
>   {
> -	struct drm_ras_node *node = &ras->node[severity];
> -
>   	kfree(node->device_name);
>   	node->device_name = NULL;
>   }
>   
> +static void cleanup_node(struct drm_device *drm, void *arg)
> +{
> +	struct drm_ras_node *node = arg;
> +
> +	drm_ras_node_unregister(node);
> +	cleanup_node_param(node);
> +}
> +
>   static int register_nodes(struct xe_device *xe)
>   {
>   	struct xe_drm_ras *ras = &xe->ras;
> @@ -150,13 +156,19 @@ static int register_nodes(struct xe_device *xe)
>   
>   		ret = assign_node_params(xe, node, i);
>   		if (ret) {
> -			cleanup_node_param(ras, i);
> +			cleanup_node_param(node);

At this point drm_ras node is not registered.

>   			return ret;
>   		}
>   
>   		ret = drm_ras_node_register(node);
>   		if (ret) {
> -			cleanup_node_param(ras, i);
> +			cleanup_node_param(node);

Ditto

Thanks
Riana

> +			return ret;
> +		}
> +
> +		ret = drmm_add_action_or_reset(&xe->drm, cleanup_node, node);
> +		if (ret) {
> +			cleanup_node(&xe->drm, node);
>   			return ret;
>   		}
>   	}
> @@ -164,20 +176,6 @@ static int register_nodes(struct xe_device *xe)
>   	return 0;
>   }
>   
> -static void xe_drm_ras_unregister_nodes(struct drm_device *device, void *arg)
> -{
> -	struct xe_device *xe = arg;
> -	struct xe_drm_ras *ras = &xe->ras;
> -	int i;
> -
> -	for_each_error_severity(i) {
> -		struct drm_ras_node *node = &ras->node[i];
> -
> -		drm_ras_node_unregister(node);
> -		cleanup_node_param(ras, i);
> -	}
> -}
> -
>   /**
>    * xe_drm_ras_init() - Initialize DRM RAS
>    * @xe: xe device instance
> @@ -204,11 +202,5 @@ int xe_drm_ras_init(struct xe_device *xe)
>   		return err;
>   	}
>   
> -	err = drmm_add_action_or_reset(&xe->drm, xe_drm_ras_unregister_nodes, xe);
> -	if (err) {
> -		drm_err(&xe->drm, "Failed to add action for Xe DRM RAS (%pe)\n", ERR_PTR(err));
> -		return err;
> -	}
> -
>   	return 0;
>   }