Re: [PATCH v3 10/12] drm/amdgpu: Avoid sysfs dirs removal post device unplug

[Daniel Vetter <daniel@ffwll.ch>]

On Sat, Nov 21, 2020 at 12:21:20AM -0500, Andrey Grodzovsky wrote:
> Avoids NULL ptr due to kobj->sd being unset on device removal.
> 
> Signed-off-by: Andrey Grodzovsky <andrey.grodzovsky@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c   | 4 +++-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c | 4 +++-
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> index caf828a..812e592 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ras.c
> @@ -27,6 +27,7 @@
>  #include <linux/uaccess.h>
>  #include <linux/reboot.h>
>  #include <linux/syscalls.h>
> +#include <drm/drm_drv.h>
>  
>  #include "amdgpu.h"
>  #include "amdgpu_ras.h"
> @@ -1043,7 +1044,8 @@ static int amdgpu_ras_sysfs_remove_feature_node(struct amdgpu_device *adev)
>  		.attrs = attrs,
>  	};
>  
> -	sysfs_remove_group(&adev->dev->kobj, &group);
> +	if (!drm_dev_is_unplugged(&adev->ddev))
> +		sysfs_remove_group(&adev->dev->kobj, &group);

This looks wrong. sysfs, like any other interface, should be
unconditionally thrown out when we do the drm_dev_unregister. Whether
hotunplugged or not should matter at all. Either this isn't needed at all,
or something is wrong with the ordering here. But definitely fishy.
-Daniel

>  
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> index 2b7c90b..54331fc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> @@ -24,6 +24,7 @@
>  #include <linux/firmware.h>
>  #include <linux/slab.h>
>  #include <linux/module.h>
> +#include <drm/drm_drv.h>
>  
>  #include "amdgpu.h"
>  #include "amdgpu_ucode.h"
> @@ -464,7 +465,8 @@ int amdgpu_ucode_sysfs_init(struct amdgpu_device *adev)
>  
>  void amdgpu_ucode_sysfs_fini(struct amdgpu_device *adev)
>  {
> -	sysfs_remove_group(&adev->dev->kobj, &fw_attr_group);
> +	if (!drm_dev_is_unplugged(&adev->ddev))
> +		sysfs_remove_group(&adev->dev->kobj, &fw_attr_group);
>  }
>  
>  static int amdgpu_ucode_init_single_fw(struct amdgpu_device *adev,
> -- 
> 2.7.4
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel