linux-kernel - Re: [PATCH] PCI: dpc: Increase pciehp waiting time for DPC recovery

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20260123202140.GA84703@bhelgaas>
Date: Fri, 23 Jan 2026 14:21:40 -0600
From: Bjorn Helgaas <helgaas@...nel.org>
To: LeoLiu-oc <LeoLiu-oc@...oxin.com>
Cc: mahesh@...ux.ibm.com, oohall@...il.com, bhelgaas@...gle.com,
	linuxppc-dev@...ts.ozlabs.org, linux-pci@...r.kernel.org,
	linux-kernel@...r.kernel.org, CobeChen@...oxin.com,
	TonyWWang@...oxin.com, ErosZhang@...oxin.com,
	Lukas Wunner <lukas@...ner.de>
Subject: Re: [PATCH] PCI: dpc: Increase pciehp waiting time for DPC recovery

[+cc Lukas, pciehp expert and author of a97396c6eb13]

On Fri, Jan 23, 2026 at 06:40:34PM +0800, LeoLiu-oc wrote:
> Commit a97396c6eb13 ("PCI: pciehp: Ignore Link Down/Up caused by DPC")
> amended PCIe hotplug to not bring down the slot upon Data Link Layer State
> Changed events caused by Downstream Port Containment.
> 
> However, PCIe hotplug (pciehp) waits up to 4 seconds before assuming that
> DPC recovery has failed and disabling the slot. This timeout period is
> insufficient for some PCIe devices.
> For example, the E810 dual-port network card driver needs to take over
> 10 seconds to execute its err_detected() callback.
> Since this exceeds the maximum wait time allowed for DPC recovery by the
> hotplug IRQ threads, a race condition occurs between the hotplug thread and
> the dpc_handler() thread.

Add blank lines between paragraphs.

Include the name of the E810 driver so we can easily find the
.err_detected() callback in question.  Actually, including the *name*
of that callback would be a very direct way of doing this :)

I guess the problem this fixes is that there was a PCIe error that
triggered DPC, and the E810 .err_detected() works but takes longer
than expected, which results in pciehp disabling the slot when it
doesn't need to?  So the user basically sees a dead E810 device?

It seems unfortunate that we have this dependency on the time allowed
for .err_detected() to execute.  It's nice if adding arbitrary delay
doesn't break things, but maybe we can't always achieve that.

I see that pci_dpc_recovered() is called from pciehp_ist().  Are we
prepared for long delays there?

> Signed-off-by: LeoLiu-oc <LeoLiu-oc@...oxin.com>
> ---
>  drivers/pci/pcie/dpc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/pcie/dpc.c b/drivers/pci/pcie/dpc.c
> index fc18349614d7..08b5f275699a 100644
> --- a/drivers/pci/pcie/dpc.c
> +++ b/drivers/pci/pcie/dpc.c
> @@ -121,7 +121,7 @@ bool pci_dpc_recovered(struct pci_dev *pdev)
>  	 * but reports indicate that DPC completes within 4 seconds.
>  	 */
>  	wait_event_timeout(dpc_completed_waitqueue, dpc_completed(pdev),
> -			   msecs_to_jiffies(4000));
> +			   msecs_to_jiffies(16000));

It looks like this breaks the connection between the "completes within
4 seconds" comment and the 4000ms wait_event timeout.

>  	return test_and_clear_bit(PCI_DPC_RECOVERED, &pdev->priv_flags);
>  }
> -- 
> 2.43.0
>