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

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

On 5/23/2026 10:30 AM, Raag Jadav wrote:
> cleanup_node_param() is not registered for previous node in case of counter
> allocation failure, which results in stale memory of previous node that
> isn't cleaned up on unwind. 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 | 59 +++++++++++++--------------------
>   1 file changed, 23 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_drm_ras.c b/drivers/gpu/drm/xe/xe_drm_ras.c
> index c1d5ac198a7c..b901976203fc 100644
> --- a/drivers/gpu/drm/xe/xe_drm_ras.c
> +++ b/drivers/gpu/drm/xe/xe_drm_ras.c
> @@ -131,53 +131,46 @@ 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];
> -
> -	ras->info[severity] = NULL;
> -
>   	kfree(node->device_name);
>   	node->device_name = NULL;
>   }
>   
> +static void cleanup_node(struct drm_device *drm, void *node)
> +{
> +	drm_ras_node_unregister(node);
> +	cleanup_node_param(node);
> +}
> +
>   static int register_nodes(struct xe_device *xe)
>   {
>   	struct xe_drm_ras *ras = &xe->ras;
> -	int i;
> +	struct drm_ras_node *node;
> +	int i, ret;
>   
>   	for_each_error_severity(i) {
> -		struct drm_ras_node *node = &ras->node[i];
> -		int ret;
> +		node = &ras->node[i];
>   
>   		ret = assign_node_params(xe, node, i);
> -		if (ret) {
> -			cleanup_node_param(ras, i);
> -			return ret;
> -		}
> +		if (ret)
> +			break;
>   
>   		ret = drm_ras_node_register(node);
> -		if (ret) {
> -			cleanup_node_param(ras, i);
> -			return ret;
> -		}
> -	}
> -
> -	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;
> +		if (ret)
> +			break;
>   
> -	for_each_error_severity(i) {
> -		struct drm_ras_node *node = &ras->node[i];
> +		ret = drmm_add_action_or_reset(&xe->drm, cleanup_node, node);
> +		if (ret)
> +			break;
On failure of drmm_add_action_or_reset , action is called. We don't need 
to cleanup_node_param again Thanks Riana
> +	}
>   
> -		drm_ras_node_unregister(node);
> -		cleanup_node_param(ras, i);
> +	if (ret) {
> +		cleanup_node_param(node);
> +		ras->info[i] = NULL;
>   	}
> +
> +	return ret;
>   }
>   
>   /**
> @@ -206,11 +199,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;
>   }