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

[David Matlack <dmatlack@google.com>]

On 2026-10-12 01:42 PM, 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.

Does any of this remaining time come from pci_doe_abort()? I see that is
also called during mailbox creation and has the same call to
pci_doe_wait(). If so, maybe the poll logic can be shared.

> 
> Signed-off-by: Michal Clapinski <mclapinski@google.com>
> ---
> I was asked if I can do this with interrupts instead. I don't see a way
> to do that during device enumeration that won't collide with the
> drivers.
> ---
>  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 6b5d9ba8e969..5e93d78a1a97 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)

It would be nice to also fix the HZ=100 issue, in a separate patch
within the same series, since you are updating this code.

> +#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
> @@ -487,7 +490,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);
> @@ -525,10 +528,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 tens of microsecond is the norm then is polling for up to 2ms
overkill?

> +		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.56.0.385.gd3acb90ef8-goog
>