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

[David Matlack]

On Fri, Oct 16, 2026 at 1:45 PM Michał Cłapiński <mclapinski@xxxxxxxxxx> wrote:
>
> On Fri, Oct 16, 2026 at 10:17 PM David Matlack <dmatlack@xxxxxxxxxx> wrote:
> >
> > 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.
>
> Yes, but I'm not sure if the poll logic can be shared without making
> the code worse rather than better. I'll look into that.
>
> Also, I'm planning on skipping pci_doe_abort for preserved devices so
> it was less of a priority for me.

I don't think that will be effective. A Live Update may have any
number of preserved and non-preserved devices. We should try to find
solutions that work for both types whenever possible. The less places
we need to sprinkle pci_liveupdate_is_incoming() the better.

>
> > >
> > > Signed-off-by: Michal Clapinski <mclapinski@xxxxxxxxxx>
> > > ---
> > > 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.
>
> Okay, will do in v2.
>
> > > +#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?
>
> I guess. I can do more precise measurements and tweak this value;
> however, I can only speak for the device I have on hand, so generally
> I prefer overkilling 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.56.0.385.gd3acb90ef8-goog
> > >