Re: [RFC PATCH 03/13] drm/amdgpu: add a flag to track ras debugfs creation status

[Mario Limonciello <mario.limonciello@amd.com>]

On 1/8/2025 07:59, Jiang Liu wrote:
> Add a flag to track ras debugfs creation status, to avoid possible
> incorrect reference count management for ras block object  in function
> amdgpu_ras_aca_is_supported().

Rather than taking a marker position, why not just check for
obj->fs_data.debugfs_name to be non NULL in amdgpu_ras_fs_fini()?

> 
> Signed-off-by: Jiang Liu <gerry@linux.alibaba.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h     | 2 +-
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 9 +++++++--
>   2 files changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index 32941f29507c..2ef7d3102be3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -378,7 +378,7 @@ enum amdgpu_marker {
>   	AMDGPU_MARKER_IRQ6		= 6,
>   	AMDGPU_MARKER_IRQ7		= 7,
>   	AMDGPU_MARKER_IRQ_MAX		= 47,
> -	AMDGPU_MARKER_DEBUGFS		= 63,
> +	AMDGPU_MARKER_RAS_DEBUGFS	= 63,

Any particular reason you jumped to 63 in this patch and then counted 
down in the next one?  IE why not throw it at 48 (and then 49 for next one)?

>   };
>   
>   #define AMDGPU_MARKER_INDEX_IRQ(idx)		(AMDGPU_MARKER_INDEX_IRQ0 + (idx))
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> index 6d52e22691f7..efd72b07a185 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> @@ -1996,7 +1996,8 @@ static void amdgpu_ras_debugfs_create(struct amdgpu_device *adev,
>   {
>   	struct ras_manager *obj = amdgpu_ras_find_obj(adev, &head->head);
>   
> -	if (!obj || !dir)
> +	if (!obj || !dir ||
> +	    amdgpu_ras_test_marker(adev, &head->head, AMDGPU_MARKER_RAS_DEBUGFS))
>   		return;
>   
>   	get_obj(obj);
> @@ -2007,6 +2008,8 @@ static void amdgpu_ras_debugfs_create(struct amdgpu_device *adev,
>   
>   	debugfs_create_file(obj->fs_data.debugfs_name, S_IWUGO | S_IRUGO, dir,
>   			    obj, &amdgpu_ras_debugfs_ops);
> +
> +	amdgpu_ras_set_marker(adev, &head->head, AMDGPU_MARKER_RAS_DEBUGFS);
>   }
>   
>   static bool amdgpu_ras_aca_is_supported(struct amdgpu_device *adev)
> @@ -2134,7 +2137,9 @@ static int amdgpu_ras_fs_fini(struct amdgpu_device *adev)
>   	if (IS_ENABLED(CONFIG_DEBUG_FS)) {
>   		list_for_each_entry_safe(con_obj, tmp, &con->head, node) {
>   			ip_obj = amdgpu_ras_find_obj(adev, &con_obj->head);
> -			if (ip_obj)
> +			if (ip_obj &&
> +			    amdgpu_ras_test_and_clear_marker(adev, &ip_obj->head,
> +			    				     AMDGPU_MARKER_RAS_DEBUGFS))
>   				put_obj(ip_obj);
>   		}
>   	}