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