[PATCH] PCI/DPC: Increase recovery timeout to 60 seconds

[PATCH] PCI/DPC: Increase recovery timeout to 60 seconds

[Ziyou Wang (Lenovo)]

Some NVMe devices need more than 5 seconds to recover after DPC
containment, but pci_dpc_recovered() waits only 4 seconds.  If the device
hasn't recovered by then, pciehp reports "device recovery failed" and
the device is re-enumerated.

Use PCIE_RESET_READY_POLL_MS (60 seconds) as the timeout so pciehp
waits as long as pci_dev_wait() does, which is sufficient for all known
devices.

Tested with INNOGRIT NVMe SSD [1dbe:5668] with ACS Fatal error
injection:

Upstream kernel (7.3.0-master+, 4s timeout):
  [123.018691] nvme: not ready 1023ms after DPC; waiting
  [124.106690] nvme: not ready 2047ms after DPC; waiting
  [125.642703] pciehp: Slot(64): Link Down
  [126.218696] AER: subordinate device reset failed
  [126.218700] AER: device recovery failed
  Device re-enumerated as nvme1

With this patch (60s timeout):
  [74.894811] nvme: not ready 1023ms after DPC; waiting
  [75.982813] nvme: not ready 2047ms after DPC; waiting
  [78.094810] nvme: ready 4095ms after DPC
  [78.094816] nvme: restart after slot reset
  [78.111620] AER: device recovery successful
  Device works normally

Fixes: a97396c6eb13 ("PCI: pciehp: Ignore Link Down/Up caused by DPC")
Cc: stable@vger.kernel.org
Signed-off-by: Ziyou Wang (Lenovo) <ziyou.dev@foxmail.com>
---
 drivers/pci/pcie/dpc.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c
index 2b779bd1d861..df1c469fa24c 100644
--- a/drivers/pci/pcie/dpc.c
+++ b/drivers/pci/pcie/dpc.c
@@ -116,12 +116,13 @@ bool pci_dpc_recovered(struct pci_dev *pdev)
 		return false;
 
 	/*
-	 * Need a timeout in case DPC never completes due to failure of
-	 * dpc_wait_rp_inactive().  The spec doesn't mandate a time limit,
-	 * but reports indicate that DPC completes within 4 seconds.
+	 * Wait for DPC recovery completion.  Use PCIE_RESET_READY_POLL_MS
+	 * (60s) to match pci_dev_wait() timeout.  Some devices need >5 seconds
+	 * to recover after DPC containment.  The 4 second timeout is too short.
 	 */
-	wait_event_timeout(dpc_completed_waitqueue, dpc_completed(pdev),
-			   msecs_to_jiffies(4000));
+	wait_event_timeout(dpc_completed_waitqueue,
+			   dpc_completed(pdev),
+			   msecs_to_jiffies(PCIE_RESET_READY_POLL_MS));
 
 	return test_and_clear_bit(PCI_DPC_RECOVERED, &pdev->priv_flags);
 }
-- 
2.53.0

Re: [PATCH] PCI/DPC: Increase recovery timeout to 60 seconds

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [High] PCI/DPC: undeclared PCIE_RESET_READY_POLL_MS in
  pci_dpc_recovered()
--

commit 9107bdc3c03dade9f834df49cdbc6c47d0a15a44
Author: Ziyou Wang (Lenovo) <ziyou.dev@foxmail.com>

PCI/DPC: Increase recovery timeout to 60 seconds

This commit increases the DPC recovery wait timeout in pci_dpc_recovered()
from 4 seconds to PCIE_RESET_READY_POLL_MS (60 seconds).

> diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c
> index 2b779bd1d861b..df1c469fa24cd 100644
> --- a/drivers/pci/pcie/dpc.c
> +++ b/drivers/pci/pcie/dpc.c
> @@ -116,12 +116,13 @@ bool pci_dpc_recovered(struct pci_dev *pdev)
>  		return false;
>  
>  	/*
> -	 * Need a timeout in case DPC never completes due to failure of
> -	 * dpc_wait_rp_inactive().  The spec doesn't mandate a time limit,
> -	 * but reports indicate that DPC completes within 4 seconds.
> +	 * Wait for DPC recovery completion.  Use PCIE_RESET_READY_POLL_MS
> +	 * (60s) to match pci_dev_wait() timeout.  Some devices need >5 seconds
> +	 * to recover after DPC containment.  The 4 second timeout is too short.
>  	 */
> -	wait_event_timeout(dpc_completed_waitqueue, dpc_completed(pdev),
> -			   msecs_to_jiffies(4000));
> +	wait_event_timeout(dpc_completed_waitqueue,
> +			   dpc_completed(pdev),
> +			   msecs_to_jiffies(PCIE_RESET_READY_POLL_MS));

[Severity: High]
Does this fail to build when CONFIG_PCIE_DPC and CONFIG_HOTPLUG_PCI_PCIE
are enabled?  PCIE_RESET_READY_POLL_MS is defined locally in
drivers/pci/pci.c rather than in drivers/pci/pci.h, so it is undeclared in
drivers/pci/pcie/dpc.c.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/tencent_0EA784F33CA15095806009C26E9DD624B809@qq.com?part=1