On 6/27/26 07:57, Geoffrey McRae wrote:
> Fix more sysfs file duplication errors when reinitializing the device:
>
> sysfs: cannot create duplicate filename '.../enforce_isolation'
> sysfs: cannot create duplicate filename '.../sdma_reset_mask'
> sysfs: cannot create duplicate filename '.../vcn_reset_mask'
> sysfs: cannot create duplicate filename '.../jpeg_reset_mask'
> sysfs: cannot create duplicate filename '.../vpe_reset_mask'
> sysfs: cannot create duplicate filename '.../current_memory_partition'
> sysfs: cannot create duplicate filename '.../available_memory_partition'
>
> Fix this by:
> - improve amdgpu_gfx_sysfs_init error unwinding so partially-created
> gfx sysfs files are removed on registration failure.
> - added adev->gfx.sysfs_registered tracking so gfx sysfs cleanup is
> idempotent when harware teardown runs before the existing software
> teardown cleanup path.
> - calling amdgpu_gfx_sysfs_fini, amdgpu_sdma_sysfs_reset_mask_fini,
> amdgpu_vcn_sysfs_reset_mask_fini, amdgpu_jpeg_sysfs_reset_mask_fini,
> amdgpu_vpe_sysfs_reset_mask_fini and amdgpu_gmc_sysfs_fini from
> amdgpu_device_sys_interface_fini
>
> Signed-off-by: Geoffrey McRae <[email protected]>
> Cc: Alex Deucher <[email protected]>
> Cc: Christian König <[email protected]>
Alex can you take a look at this?
In general the coding seems correct, but Greg usually insists that we use the
managed sysfs functions which remove stuff automatically on driver unbind.
But if I'm not completely mistaken that doesn't work here because the device
structure is not released before all userspace refs drop.
If the device is re-created before that happens we run into the error above.
Regards,
Christian.
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 8 ++++++
> drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c | 32 +++++++++++++++++-----
> drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h | 2 ++
> 3 files changed, 35 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index 70d07ca187a3..c0a22d2411f2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -3674,6 +3674,14 @@ static int amdgpu_device_sys_interface_init(struct
> amdgpu_device *adev)
>
> static void amdgpu_device_sys_interface_fini(struct amdgpu_device *adev)
> {
> + amdgpu_gfx_sysfs_fini(adev);
> + amdgpu_sdma_sysfs_reset_mask_fini(adev);
> + amdgpu_vcn_sysfs_reset_mask_fini(adev);
> + amdgpu_jpeg_sysfs_reset_mask_fini(adev);
> + amdgpu_vpe_sysfs_reset_mask_fini(adev);
> + if (amdgpu_is_multi_aid(adev))
> + amdgpu_gmc_sysfs_fini(adev);
> +
> if (adev->pm.sysfs_initialized)
> amdgpu_pm_sysfs_fini(adev);
> if (adev->ucode_sysfs_en)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> index 982b41606d48..ab37c4a9f415 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.c
> @@ -2175,6 +2175,8 @@ int amdgpu_gfx_sysfs_init(struct amdgpu_device *adev)
> {
> int r;
>
> + adev->gfx.sysfs_registered = false;
> +
> r = amdgpu_gfx_sysfs_xcp_init(adev);
> if (r) {
> dev_err(adev->dev, "failed to create xcp sysfs files");
> @@ -2182,23 +2184,39 @@ int amdgpu_gfx_sysfs_init(struct amdgpu_device *adev)
> }
>
> r = amdgpu_gfx_sysfs_isolation_shader_init(adev);
> - if (r)
> + if (r) {
> dev_err(adev->dev, "failed to create isolation sysfs files");
> + goto err_isolation_shader;
> + }
>
> r = amdgpu_gfx_sysfs_reset_mask_init(adev);
> - if (r)
> + if (r) {
> dev_err(adev->dev, "failed to create reset mask sysfs files");
> + goto err_reset_mask;
> + }
>
> + adev->gfx.sysfs_registered = true;
> +
> + return 0;
> +
> +err_reset_mask:
> + amdgpu_gfx_sysfs_reset_mask_fini(adev);
> + amdgpu_gfx_sysfs_isolation_shader_fini(adev);
> +err_isolation_shader:
> + amdgpu_gfx_sysfs_xcp_fini(adev);
> return r;
> }
>
> void amdgpu_gfx_sysfs_fini(struct amdgpu_device *adev)
> {
> - if (adev->dev->kobj.sd) {
> - amdgpu_gfx_sysfs_xcp_fini(adev);
> - amdgpu_gfx_sysfs_isolation_shader_fini(adev);
> - amdgpu_gfx_sysfs_reset_mask_fini(adev);
> - }
> + if (!adev->gfx.sysfs_registered)
> + return;
> +
> + amdgpu_gfx_sysfs_xcp_fini(adev);
> + amdgpu_gfx_sysfs_isolation_shader_fini(adev);
> + amdgpu_gfx_sysfs_reset_mask_fini(adev);
> +
> + adev->gfx.sysfs_registered = false;
> }
>
> static void amdgpu_gfx_reset_start_compute_scheds(struct amdgpu_device *adev,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> index aefd4f03b443..df7ce026bfa9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
> @@ -477,6 +477,8 @@ struct amdgpu_gfx {
> struct amdgpu_irq_src rlc_poison_irq;
> struct sq_work sq_work;
>
> + bool sysfs_registered;
> +
> /* gfx status */
> uint32_t gfx_current_status;
> /* ce ram size*/
