Re: [RFC PATCH] nvme-pci: adaptively poll completions on busy queues

> From: "Andy Shevchenko"<[email protected]>
> Date:  Fri, Aug 7, 2026, 05:18
> Subject:  Re: [RFC PATCH] nvme-pci: adaptively poll completions on busy queues
> To: "Fengnan Chang"<[email protected]>
> Cc: <[email protected]>, "Keith Busch"<[email protected]>, "Jens Axboe"<[email protected]>, "Christoph Hellwig"<[email protected]>, "Sagi Grimberg"<[email protected]>, "Bart Van Assche"<[email protected]>, "Thomas Gleixner"<[email protected]>, "Jun Zeng"<[email protected]>, "Gang Cao"<[email protected]>, "Jun I Jin"<[email protected]>, "Liang A Fang"<[email protected]>, "Yong Hu"<[email protected]>, <[email protected]>, "Guzebing"<[email protected]>
> On Thu, Jul 23, 2026 at 08:05:56PM +0800, Fengnan Chang wrote:
> > In high-IOPS scenarios, relying on interrupts to handle I/O operations
> > can limit performance. This issue becomes particularly pronounced in
> > multi-disk environments, where performance is constrained by the CPU’s
> > interrupt-handling capacity.
> > 
> > Each Solidigm SB5PH27X038T device used for testing can deliver about 3.2M
> > 4 KiB random-read IOPS.  Four devices therefore have about 12.8M IOPS of
> > aggregate capability, but interrupt-driven completion topped out at 7.81M
> > IOPS, or about 61% of that capability.
> > 
> > Add optional adaptive polling for busy interrupt-driven I/O queues.  After
> > an interrupt drains the CQ, use the ring distance between last_sq_tail and
> > cq_head as an approximate host-side inflight count.  If the count reaches
> > a per-controller high watermark, let the threaded handler keep draining
> > completions instead of returning to interrupt-driven completion right away.
> > 
> > The thread always checks for pending CQEs first.  When the CQ is empty, it
> > compares the inflight count with a per-controller low watermark and sleeps
> > for a configurable interval before checking again.  It returns to
> > interrupt-driven completion after three consecutive empty-CQ checks below
> > the low watermark or when the loop limit is reached.
> > 
> > The mode is disabled by default. It can be enabled at module load time.
> > The high and low watermarks, empty-CQ interval, and loop limit are
> > available through sysfs.  The defaults are 32 commands, 3 commands,
> > 30 us, and 1000 iterations.
> > 
> > Tested with the default settings and 4 KiB random reads on four Solidigm
> > SB5PH27X038T devices.  With adaptive polling off and on:
> > 
> >   - t/io_uring, depth 1024 and four workers per device:
> >     7.810M and 12.780M aggregate IOPS (+63.64%).
> >   - fio/libaio, iodepth 1024, numjobs 4 and completion batches of 32:
> >     7.699M and 12.785M aggregate IOPS (+66.05%).
> > 
> > In a paired libaio and io_uring boundary sweep, QD8, QD16, QD32, and
> > QD16/numjobs=4 changed by -0.15% to +0.48%.  QD64 improved by 28.61% to
> > 59.75%.
> 
> ...
> 
> > +What:                /sys/class/nvme/nvmeX/adaptive_poll_inflight_high
> > +Date:                July 2026
> 
> Can't be July for v7.3. See crystal ball predictor for the dates (rc1 or release).

Thank you for taking the time to review this patch. I’ve decided to abandon
the approach in this patch, as it has unavoidable regression issues.
The number of in-flight I/O operations does not guarantee that CQEs will be
completed at a high frequency, nor does it indicate that using poll would be
more efficient. Therefore, I’ve proposed an alternative approach:
https://lore.kernel.org/linux-nvme/[email protected]/T/#u

I’d greatly appreciate it if you could review the new approach.

> 
> > +KernelVersion:        7.3
> 
> ...
> 
> > +static void nvme_adaptive_unmask_irq(struct nvme_queue *nvmeq, int irq)
> > +{
> > +        struct nvme_dev *dev = nvmeq->dev;
> > +        struct pci_dev *pdev = to_pci_dev(dev->dev);
> > +
> > +        clear_bit(NVMEQ_ADAPTIVE_POLLING, &nvmeq->flags);
> 
> 
> > +        if (pdev->msi_enabled) {
> 
> Wondering if MSI-X also should be considered here.
> With that in mind, can we use pci_dev_msi_enabled()?
> 
> > +                writel(BIT(nvmeq->cq_vector), dev->bar + NVME_REG_INTMC);
> > +                readl(dev->bar + NVME_REG_INTMS);
> > +        } else {
> > +                enable_irq(irq);
> > +        }
> > +}
> 
> ...
> 
> > +static ssize_t adaptive_poll_interval_us_store(struct device *dev,
> > +                                               struct device_attribute *attr,
> > +                                               const char *buf, size_t count)
> > +{
> > +        struct nvme_dev *ndev = to_nvme_dev(dev_get_drvdata(dev));
> > +        unsigned int value;
> > +        int ret;
> > +
> > +        ret = kstrtouint(buf, 10, &value);
> > +        if (ret)
> > +                return ret;
> > +        if (!value)
> > +                return -EINVAL;
> 
> ERANGE? EDOM?
> 
> Ditto for other similar cases.
> 
> > +        WRITE_ONCE(ndev->adaptive_poll_interval_us, value);
> > +        return count;
> > +}
> 
> -- 
> With Best Regards,
> Andy Shevchenko
>