[Public]
Hi Andrey,
Do you mean to move the code of drm_sched_resubmit_jobs and drm_sched_start in
amdgpu_pci_resume to amdgpu_pci_error_detected, under the case
pci_channel_io_frozen?
Then leave amdgpu_pci_resume as a null function, and in this way, we can drop
the acquire/lock write lock for case of pci_channel_io_normal as well?
Regards,
Guchun
-----Original Message-----
From: Grodzovsky, Andrey <andrey.grodzov...@amd.com>
Sent: Friday, October 1, 2021 10:22 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>
Subject: Re: [PATCH] drm/amdgpu: add missed write lock for pci detected state
pci_channel_io_normal
On 2021-09-30 10:00 p.m., Guchun Chen wrote:
> 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.
Good catch but, the issue is even wider in scope, what about
drm_sched_resubmit_jobs and drm_sched_start called without being stopped before
? Better to put the entire scope of code in this function under flag that set
only in pci_channel_io_frozen. As far as i remember we don't need to do
anything in case of pci_channel_io_normal.
Andrey
>
> Fixes: c9a6b82f45e2("drm/amdgpu: Implement DPC recovery")
> Signed-off-by: Guchun Chen <guchun.c...@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index bb5ad2b6ca13..12f822d51de2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -5370,6 +5370,7 @@ pci_ers_result_t
> amdgpu_pci_error_detected(struct pci_dev *pdev, pci_channel_sta
>
> switch (state) {
> case pci_channel_io_normal:
> + amdgpu_device_lock_adev(adev, NULL);
> return PCI_ERS_RESULT_CAN_RECOVER;
> /* Fatal error, prepare for slot reset */
> case pci_channel_io_frozen:
