Re: [RFC v2 05/15] drm/amdgpu: enhance amdgpu_ras_block_late_fini()

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

On 1/12/2025 19:42, Jiang Liu wrote:
> Enhance amdgpu_ras_block_late_fini() to revert what has been done
> by amdgpu_ras_block_late_init(), and fix a possible resource leakage
> in function amdgpu_ras_block_late_init().
> 
> Signed-off-by: Jiang Liu <gerry@linux.alibaba.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 16 ++++++++++------
>   1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> index f0cd14ff78a7..7bbab7297c97 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> @@ -4104,13 +4104,13 @@ int amdgpu_ras_block_late_init(struct amdgpu_device *adev,
>   	     ras_obj->hw_ops->query_ras_error_status)) {
>   		r = amdgpu_ras_sysfs_create(adev, ras_block);
>   		if (r)
> -			goto interrupt;
> +			goto cleanup;
>   
>   		/* Those are the cached values at init.
>   		 */
>   		query_info = kzalloc(sizeof(*query_info), GFP_KERNEL);
>   		if (!query_info)
> -			return -ENOMEM;
> +			goto cleanup;

AFAICT  you still need to set "r = -ENOMEM" here for this error flow.

>   		memcpy(&query_info->head, ras_block, sizeof(struct ras_common_if));
>   
>   		if (amdgpu_ras_query_error_count(adev, &ce_count, &ue_count, query_info) == 0) {
> @@ -4123,11 +4123,8 @@ int amdgpu_ras_block_late_init(struct amdgpu_device *adev,
>   
>   	return 0;
>   
> -interrupt:
> -	if (ras_obj->ras_cb)
> -		amdgpu_ras_interrupt_remove_handler(adev, ras_block);
>   cleanup:
> -	amdgpu_ras_feature_enable(adev, ras_block, 0);
> +	amdgpu_ras_block_late_fini(adev, ras_block);
>   	return r;
>   }
>   
> @@ -4142,9 +4139,16 @@ void amdgpu_ras_block_late_fini(struct amdgpu_device *adev,
>   			  struct ras_common_if *ras_block)
>   {
>   	struct amdgpu_ras_block_object *ras_obj;
> +
>   	if (!ras_block)
>   		return;
>   
> +	amdgpu_ras_feature_enable(adev, ras_block, 0);
> +
> +	/* in resume/reset phase, no need to delete ras fs node */
> +	if (adev->in_suspend || amdgpu_in_reset(adev))
> +		return;
> +
>   	amdgpu_ras_sysfs_remove(adev, ras_block);
>   
>   	ras_obj = container_of(ras_block, struct amdgpu_ras_block_object, ras_comm);