Re: [PATCH 1/6] amdgpu: add flags to track sysfs initialization status

[Chen, Xiaogang]

On 1/1/2025 11:36 PM, Jiang Liu wrote:
Add flags to track sysfs initialization status, so we can correctly
clean them up on error recover paths.

Signed-off-by: Jiang Liu <ge...@linux.alibaba.com>
---
  drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  3 ++
  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 34 +++++++++++++++++-----
  2 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 22c7e9669162..e4b13e729770 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1157,6 +1157,9 @@ struct amdgpu_device {
        bool                            in_runpm;
        bool                            has_pr3;
  
+	bool				sysfs_en;
+       bool                            fru_sysfs_en;
+       bool                            reg_state_sysfs_en;
        bool                            ucode_sysfs_en;
  
  	struct amdgpu_fru_info		*fru_info;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index d1bb9e85b6d7..3244966b0c39 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -4533,8 +4533,13 @@ int amdgpu_device_init(struct amdgpu_device *adev,
                adev->ucode_sysfs_en = true;
  
  	r = sysfs_create_files(&adev->dev->kobj, amdgpu_dev_attributes);
-       if (r)
+       if (r) {
                dev_err(adev->dev, "Could not create amdgpu device attr\n");
+               adev->sysfs_en = false;
+       } else {
+               adev->sysfs_en = true;
+       }
+
not need use {}
  #ifdef HAVE_PCI_DRIVER_DEV_GROUPS
        r = devm_device_add_group(adev->dev, &amdgpu_board_attrs_group);
        if (r)
@@ -4542,8 +4547,21 @@ int amdgpu_device_init(struct amdgpu_device *adev,
                        "Could not create amdgpu board attributes\n");
  #endif
  
-	amdgpu_fru_sysfs_init(adev);
-       amdgpu_reg_state_sysfs_init(adev);
+       r = amdgpu_fru_sysfs_init(adev);
+       if (r) {
+               dev_err(adev->dev, "Could not create amdgpu fru attr\n");
+               adev->fru_sysfs_en = false;
+       } else {
+               adev->fru_sysfs_en = true;
+       }
not need use {}
+
+       r = amdgpu_reg_state_sysfs_init(adev);
+       if (r) {
+               dev_err(adev->dev, "Could not create amdgpu reg state attr\n");
+               adev->reg_state_sysfs_en = false;
+       } else {
+               adev->reg_state_sysfs_en = true;
+       }
same as above
  
  	if (IS_ENABLED(CONFIG_PERF_EVENTS))
                r = amdgpu_pmu_init(adev);
@@ -4666,10 +4684,12 @@ void amdgpu_device_fini_hw(struct amdgpu_device *adev)
                amdgpu_pm_sysfs_fini(adev);
        if (adev->ucode_sysfs_en)
                amdgpu_ucode_sysfs_fini(adev);
-       sysfs_remove_files(&adev->dev->kobj, amdgpu_dev_attributes);
-       amdgpu_fru_sysfs_fini(adev);
-
-       amdgpu_reg_state_sysfs_fini(adev);
+       if (adev->sysfs_en)
+               sysfs_remove_files(&adev->dev->kobj, amdgpu_dev_attributes);
+       if (adev->fru_sysfs_en)
+               amdgpu_fru_sysfs_fini(adev);
+       if (adev->reg_state_sysfs_en)
+               amdgpu_reg_state_sysfs_fini(adev);

If creations of these sys entries fail does kernel send warnings or just 
ignore that when delete these entries? Seems kernel keeps silence for 
this case.

Regards

Xiaogang


  
  	/* disable ras feature must before hw fini */
        amdgpu_ras_pre_fini(adev);