On Mon, Jun 29, 2026 at 6:01 AM Christian König
<[email protected]> wrote:
>
> 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.
I think the proper fix for this is to properly unwind in
amdgpu_device_ip_init(). If sw_init() fails, we should goto a proper
unwind loop which calls sw_fini() for all IPs where status.sw == true,
etc.
Alex
>
> 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*/
>
