[Public] Thanks Andrey, I will update the name to be " pci_channel_state " when submitting.
Regards, Guchun -----Original Message----- From: Grodzovsky, Andrey <andrey.grodzov...@amd.com> Sent: Tuesday, October 5, 2021 12:04 AM To: Chen, Guchun <guchun.c...@amd.com>; amd-gfx@lists.freedesktop.org; Koenig, Christian <christian.koe...@amd.com>; Pan, Xinhui <xinhui....@amd.com>; Deucher, Alexander <alexander.deuc...@amd.com>; Liu, Monk <monk....@amd.com> Subject: Re: [PATCH] drm/amdgpu: handle the case of pci_channel_io_frozen only in amdgpu_pci_resume On 2021-10-02 11:18 a.m., Guchun Chen wrote: > In current code, when a PCI error state pci_channel_io_normal is > detectd, it will report PCI_ERS_RESULT_CAN_RECOVER status to PCI > driver, and PCI driver will continue the execution of PCI resume > callback report_resume by pci_walk_bridge, and the callback will go > into amdgpu_pci_resume finally, where write lock is releasd > unconditionally without acquiring such lock first. In this case, a > deadlock will happen when other threads start to acquire the read lock. > > To fix this, add a member in amdgpu_device strucutre to cache > pci_channel_state, and only continue the execution in > amdgpu_pci_resume when it's pci_channel_io_frozen. > > Fixes: c9a6b82f45e2("drm/amdgpu: Implement DPC recovery") > Suggested-by: Andrey Grodzovsky <andrey.grodzov...@amd.com> > Signed-off-by: Guchun Chen <guchun.c...@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 + > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 6 ++++++ > 2 files changed, 7 insertions(+) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h > b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > index f4bceb2624fb..720d0ccecfe0 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h > @@ -1094,6 +1094,7 @@ struct amdgpu_device { > > bool no_hw_access; > struct pci_saved_state *pci_state; > + pci_channel_state_t cached_state; I would give a more descriptive name to this (e.g. pci_channel_state) Other then that Reviewed-by: Andrey Grodzovsky <andrey.grodzov...@amd.com> Andrey > > struct amdgpu_reset_control *reset_cntl; > uint32_t > ip_versions[HW_ID_MAX][HWIP_MAX_INSTANCE]; > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > index bb5ad2b6ca13..1aaeb4b30edc 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c > @@ -5368,6 +5368,8 @@ pci_ers_result_t amdgpu_pci_error_detected(struct > pci_dev *pdev, pci_channel_sta > return PCI_ERS_RESULT_DISCONNECT; > } > > + adev->cached_state = state; > + > switch (state) { > case pci_channel_io_normal: > return PCI_ERS_RESULT_CAN_RECOVER; @@ -5510,6 +5512,10 @@ void > amdgpu_pci_resume(struct pci_dev *pdev) > > DRM_INFO("PCI error: resume callback!!\n"); > > + /* Only continue execution for the case of pci_channel_io_frozen */ > + if (adev->cached_state != pci_channel_io_frozen) > + return; > + > for (i = 0; i < AMDGPU_MAX_RINGS; ++i) { > struct amdgpu_ring *ring = adev->rings[i]; >