[PATCH] PCI/DOE: Poll briefly for a response before sleeping

Michal Clapinski posted 1 patch 1 week ago
drivers/pci/doe.c | 28 +++++++++++++++++++++++-----
1 file changed, 23 insertions(+), 5 deletions(-)
[PATCH] PCI/DOE: Poll briefly for a response before sleeping
Posted by Michal Clapinski 1 week ago
doe_statemachine_work() waits for a response by sleeping
PCI_DOE_POLL_INTERVAL (PCI_DOE_TIMEOUT / 128) between status reads,
which is 7 ms at HZ=1000 and 4 ms at HZ=250. Exchanges are served from
the device's local mailbox logic and typically complete within tens of
microseconds, so nearly all of that sleep is wasted. Every DOE-capable
endpoint pays it once per supported feature during enumeration.

Poll with usleep_range(20, 50) for the first ~2 ms before falling back
to the jiffies-based wait. On an Intel Emerald Rapids system, time spent
in pci_device_add() for a DOE-capable endpoint drops from 41.8 ms to
14.0 ms.

Signed-off-by: Michal Clapinski <mclapinski@google.com>
---
 drivers/pci/doe.c | 28 +++++++++++++++++++++++-----
 1 file changed, 23 insertions(+), 5 deletions(-)

diff --git a/drivers/pci/doe.c b/drivers/pci/doe.c
index ac95b1d2d999..6dc85133a88e 100644
--- a/drivers/pci/doe.c
+++ b/drivers/pci/doe.c
@@ -27,6 +27,9 @@
 /* Timeout of 1 second from 6.30.2 Operation, PCI Spec r6.0 */
 #define PCI_DOE_TIMEOUT HZ
 #define PCI_DOE_POLL_INTERVAL	(PCI_DOE_TIMEOUT / 128)
+#define PCI_DOE_POLL_FAST_MIN_US	20
+#define PCI_DOE_POLL_FAST_MAX_US	50
+#define PCI_DOE_POLL_FAST_TRIES		40
 
 #define PCI_DOE_FLAG_CANCEL	0
 #define PCI_DOE_FLAG_DEAD	1
@@ -486,7 +489,7 @@ static void doe_statemachine_work(struct work_struct *work)
 	int offset = doe_mb->cap_offset;
 	unsigned long timeout_jiffies;
 	u32 val;
-	int rc;
+	int rc, tries = 0;
 
 	if (test_bit(PCI_DOE_FLAG_DEAD, &doe_mb->flags)) {
 		signal_task_complete(task, -EIO);
@@ -524,10 +527,25 @@ static void doe_statemachine_work(struct work_struct *work)
 			signal_task_abort(task, -EIO);
 			return;
 		}
-		rc = pci_doe_wait(doe_mb, PCI_DOE_POLL_INTERVAL);
-		if (rc) {
-			signal_task_abort(task, rc);
-			return;
+		/*
+		 * Exchanges are served from the device's local mailbox
+		 * logic and usually complete in tens of microseconds, far
+		 * below the jiffies granularity of PCI_DOE_POLL_INTERVAL.
+		 * Poll for up to ~2 ms before falling back to it.
+		 */
+		if (tries++ < PCI_DOE_POLL_FAST_TRIES) {
+			if (test_bit(PCI_DOE_FLAG_CANCEL, &doe_mb->flags)) {
+				signal_task_abort(task, -EIO);
+				return;
+			}
+			usleep_range(PCI_DOE_POLL_FAST_MIN_US,
+				     PCI_DOE_POLL_FAST_MAX_US);
+		} else {
+			rc = pci_doe_wait(doe_mb, PCI_DOE_POLL_INTERVAL);
+			if (rc) {
+				signal_task_abort(task, rc);
+				return;
+			}
 		}
 		goto retry_resp;
 	}
-- 
2.55.0.1082.g2b9226bbc0-goog
Re: [PATCH] PCI/DOE: Poll briefly for a response before sleeping
Posted by Lukas Wunner 6 days, 10 hours ago
On Fri, Sep 18, 2026 at 01:03:23AM +0200, Michal Clapinski wrote:
> doe_statemachine_work() waits for a response by sleeping
> PCI_DOE_POLL_INTERVAL (PCI_DOE_TIMEOUT / 128) between status reads,
> which is 7 ms at HZ=1000 and 4 ms at HZ=250. Exchanges are served from
> the device's local mailbox logic and typically complete within tens of
> microseconds, so nearly all of that sleep is wasted. Every DOE-capable
> endpoint pays it once per supported feature during enumeration.
> 
> Poll with usleep_range(20, 50) for the first ~2 ms before falling back
> to the jiffies-based wait. On an Intel Emerald Rapids system, time spent
> in pci_device_add() for a DOE-capable endpoint drops from 41.8 ms to
> 14.0 ms.

Polling is just a poor man's replacement for interrupts.  DOE supports
interrupt-driven transfers but we're not taking advantage of it so far.

Perhaps you could look into adding that?  It would seem more worthwhile
than optimizing polling.

Thanks,

Lukas
Re: [PATCH] PCI/DOE: Poll briefly for a response before sleeping
Posted by Michał Cłapiński 6 days, 5 hours ago
On Fri, Sep 18, 2026 at 7:27 AM Lukas Wunner <lukas@wunner.de> wrote:
>
> On Fri, Sep 18, 2026 at 01:03:23AM +0200, Michal Clapinski wrote:
> > doe_statemachine_work() waits for a response by sleeping
> > PCI_DOE_POLL_INTERVAL (PCI_DOE_TIMEOUT / 128) between status reads,
> > which is 7 ms at HZ=1000 and 4 ms at HZ=250. Exchanges are served from
> > the device's local mailbox logic and typically complete within tens of
> > microseconds, so nearly all of that sleep is wasted. Every DOE-capable
> > endpoint pays it once per supported feature during enumeration.
> >
> > Poll with usleep_range(20, 50) for the first ~2 ms before falling back
> > to the jiffies-based wait. On an Intel Emerald Rapids system, time spent
> > in pci_device_add() for a DOE-capable endpoint drops from 41.8 ms to
> > 14.0 ms.
>
> Polling is just a poor man's replacement for interrupts.  DOE supports
> interrupt-driven transfers but we're not taking advantage of it so far.
>
> Perhaps you could look into adding that?  It would seem more worthwhile
> than optimizing polling.

Thanks for the quick response.

This code runs during PCI device enumeration, before a driver is bound, so
the core doesn't really have an interrupt to work with: MSI/MSI-X vectors
belong to the driver, and enabling them in the core would collide with its
later pci_alloc_irq_vectors(); INTx isn't routed yet either, since dev->irq
only becomes valid via acpi_pci_irq_enable() at pci_enable_device() time.