* [PATCH] drm/amdgpu: handle the case of pci_channel_io_frozen only in amdgpu_pci_resume
@ 2021-10-02 15:18 Guchun Chen
2021-10-04 16:04 ` Andrey Grodzovsky
0 siblings, 1 reply; 3+ messages in thread
From: Guchun Chen @ 2021-10-02 15:18 UTC (permalink / raw)
To: amd-gfx, christian.koenig, xinhui.pan, alexander.deucher,
andrey.grodzovsky, monk.liu
Cc: Guchun Chen
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.grodzovsky@amd.com>
Signed-off-by: Guchun Chen <guchun.chen@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;
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];
--
2.17.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH] drm/amdgpu: handle the case of pci_channel_io_frozen only in amdgpu_pci_resume 2021-10-02 15:18 [PATCH] drm/amdgpu: handle the case of pci_channel_io_frozen only in amdgpu_pci_resume Guchun Chen @ 2021-10-04 16:04 ` Andrey Grodzovsky 2021-10-05 11:43 ` Chen, Guchun 0 siblings, 1 reply; 3+ messages in thread From: Andrey Grodzovsky @ 2021-10-04 16:04 UTC (permalink / raw) To: Guchun Chen, amd-gfx, christian.koenig, xinhui.pan, alexander.deucher, monk.liu 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.grodzovsky@amd.com> > Signed-off-by: Guchun Chen <guchun.chen@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.grodzovsky@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]; > ^ permalink raw reply [flat|nested] 3+ messages in thread
* RE: [PATCH] drm/amdgpu: handle the case of pci_channel_io_frozen only in amdgpu_pci_resume 2021-10-04 16:04 ` Andrey Grodzovsky @ 2021-10-05 11:43 ` Chen, Guchun 0 siblings, 0 replies; 3+ messages in thread From: Chen, Guchun @ 2021-10-05 11:43 UTC (permalink / raw) To: Grodzovsky, Andrey, amd-gfx, Koenig, Christian, Pan, Xinhui, Deucher, Alexander, Liu, Monk [Public] Thanks Andrey, I will update the name to be " pci_channel_state " when submitting. Regards, Guchun -----Original Message----- From: Grodzovsky, Andrey <Andrey.Grodzovsky@amd.com> Sent: Tuesday, October 5, 2021 12:04 AM To: Chen, Guchun <Guchun.Chen@amd.com>; amd-gfx@lists.freedesktop.org; Koenig, Christian <Christian.Koenig@amd.com>; Pan, Xinhui <Xinhui.Pan@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com>; Liu, Monk <Monk.Liu@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.grodzovsky@amd.com> > Signed-off-by: Guchun Chen <guchun.chen@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.grodzovsky@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]; > ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-10-05 11:43 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-10-02 15:18 [PATCH] drm/amdgpu: handle the case of pci_channel_io_frozen only in amdgpu_pci_resume Guchun Chen 2021-10-04 16:04 ` Andrey Grodzovsky 2021-10-05 11:43 ` Chen, Guchun
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.