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

"changfengnan" <[email protected]> Fri, 07 Aug 2026 10:18:48 +0800
Newsgroups org.infradead.lists.linux-nvme,org.kernel.vger.linux-kernel
Message-ID <d9210bcdf73fbe1ac8b6ec132865609a3ed68688.52232ae5.c88d.441c.9d7a.294537a77e7c@bytedance.com>
> From: "Andy Shevchenko"<[email protected]>
> Date:=C2=A0 Fri, Aug 7, 2026, 05:18
> Subject:=C2=A0 Re: [RFC PATCH] nvme-pci: adaptively poll completions on b=
usy queues
> To: "Fengnan Chang"<[email protected]>
> Cc: <[email protected]>, "Keith Busch"<[email protected]>, "=
Jens Axboe"<[email protected]>, "Christoph Hellwig"<[email protected]>, "Sagi Grimbe=
rg"<[email protected]>, "Bart Van Assche"<[email protected]>, "Thomas Gleix=
ner"<[email protected]>, "Jun Zeng"<[email protected]>, "Gang Cao"<gang.cao=
@intel.com>, "Jun I Jin"<[email protected]>, "Liang A Fang"<liang.a.fang@=
intel.com>, "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=E2=
=80=99s
> > interrupt-handling capacity.
> >=C2=A0
> > Each Solidigm SB5PH27X038T device used for testing can deliver about 3.=
2M
> > 4 KiB random-read IOPS. =C2=A0Four devices therefore have about 12.8M I=
OPS of
> > aggregate capability, but interrupt-driven completion topped out at 7.8=
1M
> > IOPS, or about 61% of that capability.
> >=C2=A0
> > Add optional adaptive polling for busy interrupt-driven I/O queues. =C2=
=A0After
> > an interrupt drains the CQ, use the ring distance between last_sq_tail =
and
> > cq_head as an approximate host-side inflight count. =C2=A0If the count =
reaches
> > a per-controller high watermark, let the threaded handler keep draining
> > completions instead of returning to interrupt-driven completion right a=
way.
> >=C2=A0
> > The thread always checks for pending CQEs first. =C2=A0When the CQ is e=
mpty, it
> > compares the inflight count with a per-controller low watermark and sle=
eps
> > for a configurable interval before checking again. =C2=A0It returns to
> > interrupt-driven completion after three consecutive empty-CQ checks bel=
ow
> > the low watermark or when the loop limit is reached.
> >=C2=A0
> > 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. =C2=A0The defaults are 32 commands, 3 commands=
,
> > 30 us, and 1000 iterations.
> >=C2=A0
> > Tested with the default settings and 4 KiB random reads on four Solidig=
m
> > SB5PH27X038T devices. =C2=A0With adaptive polling off and on:
> >=C2=A0
> > =C2=A0 - t/io_uring, depth 1024 and four workers per device:
> > =C2=A0 =C2=A0 7.810M and 12.780M aggregate IOPS (+63.64%).
> > =C2=A0 - fio/libaio, iodepth 1024, numjobs 4 and completion batches of =
32:
> > =C2=A0 =C2=A0 7.699M and 12.785M aggregate IOPS (+66.05%).
> >=C2=A0
> > In a paired libaio and io_uring boundary sweep, QD8, QD16, QD32, and
> > QD16/numjobs=3D4 changed by -0.15% to +0.48%. =C2=A0QD64 improved by 28=
.61% to
> > 59.75%.
>=C2=A0
> ...
>=C2=A0
> > +What: =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/sys/clas=
s/nvme/nvmeX/adaptive_poll_inflight_high
> > +Date: =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0July 2026
>=C2=A0
> 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=E2=80=99ve 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 b=
e
more efficient. Therefore, I=E2=80=99ve proposed an alternative approach:
https://lore.kernel.org/linux-nvme/20260806031058.40176-1-changfengnan@byte=
dance.com/T/#u

I=E2=80=99d greatly appreciate it if you could review the new approach.

>=C2=A0
> > +KernelVersion: =C2=A0 =C2=A0 =C2=A0 =C2=A07.3
>=C2=A0
> ...
>=C2=A0
> > +static void nvme_adaptive_unmask_irq(struct nvme_queue *nvmeq, int irq=
)
> > +{
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0struct nvme_dev *dev =3D nvmeq->dev;
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0struct pci_dev *pdev =3D to_pci_dev(dev->d=
ev);
> > +
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0clear_bit(NVMEQ_ADAPTIVE_POLLING, &nvmeq->=
flags);
>=C2=A0
>=C2=A0
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0if (pdev->msi_enabled) {
>=C2=A0
> Wondering if MSI-X also should be considered here.
> With that in mind, can we use pci_dev_msi_enabled()?
>=C2=A0
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0writel(BIT(nvm=
eq->cq_vector), dev->bar + NVME_REG_INTMC);
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0readl(dev->bar=
 + NVME_REG_INTMS);
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0} else {
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0enable_irq(irq=
);
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0}
> > +}
>=C2=A0
> ...
>=C2=A0
> > +static ssize_t adaptive_poll_interval_us_store(struct device *dev,
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=
 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 struct device_attribute *attr,
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0=
 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 =C2=A0 const char *buf, size_t count)
> > +{
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0struct nvme_dev *ndev =3D to_nvme_dev(dev_=
get_drvdata(dev));
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0unsigned int value;
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0int ret;
> > +
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0ret =3D kstrtouint(buf, 10, &value);
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0if (ret)
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return ret;
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0if (!value)
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0return -EINVAL=
;
>=C2=A0
> ERANGE? EDOM?
>=C2=A0
> Ditto for other similar cases.
>=C2=A0
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0WRITE_ONCE(ndev->adaptive_poll_interval_us=
, value);
> > + =C2=A0 =C2=A0 =C2=A0 =C2=A0return count;
> > +}
>=C2=A0
> --=C2=A0
> With Best Regards,
> Andy Shevchenko
>=C2=A0