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

[Tauro, Riana]

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.ja...@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;
  }