[PATCH 2/2] drm/amdgpu: No longer insert ras blocks into ras_list if it already exists in ras_list

[Chai, Thomas]

Hi Felix:
     amdgpu_ras_register_ras_block was called by all IP ras blocks,  and every ip also has different ras versions.  We do common work together, which can reduce the chance of the ras function going wrong.

-----Original Message-----
From: Kuehling, Felix <Felix.Kuehling at amd.com> 
Sent: Thursday, January 13, 2022 12:39 AM
To: Chai, Thomas <YiPeng.Chai at amd.com>; amd-gfx at lists.freedesktop.org
Cc: Zhou1, Tao <Tao.Zhou1 at amd.com>; Zhang, Hawking <Hawking.Zhang at amd.com>; Clements, John <John.Clements at amd.com>; Chai, Thomas <YiPeng.Chai at amd.com>
Subject: Re: [PATCH 2/2] drm/amdgpu: No longer insert ras blocks into ras_list if it already exists in ras_list


Am 2022-01-12 um 2:48 a.m. schrieb yipechai:
> No longer insert ras blocks into ras_list if it already exists in ras_list.
>
> Signed-off-by: yipechai <YiPeng.Chai at amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> index 62be0b4909b3..e6d3bb4b56e4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> @@ -2754,9 +2754,17 @@ int amdgpu_ras_reset_gpu(struct amdgpu_device 
> *adev)  int amdgpu_ras_register_ras_block(struct amdgpu_device *adev,
>  		struct amdgpu_ras_block_object* ras_block_obj)  {
> +	struct amdgpu_ras_block_object *obj, *tmp;
>  	if (!adev || !amdgpu_ras_asic_supported(adev) || !ras_block_obj)
>  		return -EINVAL;
>  
> +	/* If the ras object had been in ras_list, doesn't add it to ras_list again */
> +	list_for_each_entry_safe(obj, tmp, &adev->ras_list, node) {
> +		if (obj == ras_block_obj) {
Instead of a loop, can't this be done more efficiently with "if (!list_empty(&ras_block_obj->node))"?

Of course this would require that you move the INIT_LIST_HEAD to some earlier stage so that list_empty is reliable.

Regards,
  Felix


> +			return 0;
> +		}
> +	}
> +
>  	INIT_LIST_HEAD(&ras_block_obj->node);
>  	list_add_tail(&ras_block_obj->node, &adev->ras_list);
>