[PATCH] nvme-pci: Fix performance regression with completion batching for Hygon processors

[PATCH] nvme-pci: Fix performance regression with completion batching for Hygon processors

[Liao Xuan]

Commit 4f5022453acd ("nvme: wire up completion batching for the IRQ path")
introduced completion batching to the NVMe IRQ handling path. While this
change brings performance gains on mainstream platforms, it causes a
severe random read performance regression on Hygon platforms.

The batching logic defers CQE processing until after the doorbell is
written. On Hygon microarchitectures, this specific execution order
accelerates the NVMe controller's interrupt generation. This leads to an
interrupt storm: the overall interrupt count increases significantly,
and each interrupt handles fewer CQEs. Consequently, a massive amount of
CPU cycles are consumed by the fixed overhead of interrupt entry/exit
rather than useful I/O processing, drastically degrading random read
efficiency.

Interrupt statistics were collected on a Hygon platform using a 10-second
workload of 16 jobs with 4 KiB random reads:
With batching: average IOPS ~2.29M; interrupts per queue ~1.82M
Without batching: average IOPS ~2.89M; interrupts per queue ~1.52M

By restoring the single-completion fast path, the interrupt rate is
reduced by ~16%, eliminating the interrupt storm.

Performance results (fio 4k randread, 16 jobs) show
significant improvements across various CPU pinning topologies:

CPU Pinning     | With Batching (kops) | This Patch (kops) | Improvement
0-15,128-143    | 1,747.9              | 2,053.9           | +17.5%
0-11,128-139    | 1,939.8              | 2,437.6           | +25.7%
0-7,128-135     | 2,341.9              | 2,891.3           | +23.5%
0-15            | 1,754.1              | 2,018.6           | +15.1%
0-11            | 2,051.2              | 2,510.5           | +22.4%

It is worth noting that this issue is not necessarily exclusive to Hygon
CPUs. Architectures with higher interrupt latency or lower overall
performance characteristics may also suffer from similar regressions,
where the overhead of the batching fast path outweighs the benefit of
amortizing the doorbell write.

Instead of hardcoding a permanent revert, introduce a flag 'use_comp_batch'
and disable IRQ completion batching if running on a Hygon CPU. This
preserves the performance benefits for platforms that favor batching
while restoring the optimal single-completion fast path for Hygon, and
provides a scalable mechanism to disable it on other affected platforms
in the future.

Fixes: 4f5022453acd ("nvme: wire up completion batching for the IRQ path")
Signed-off-by: Liao Xuan <liaoxuan@open-hieco.net>
---
 drivers/nvme/host/pci.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
index 69932d640..a3ecbac40 100644
--- a/drivers/nvme/host/pci.c
+++ b/drivers/nvme/host/pci.c
@@ -276,6 +276,15 @@ static bool noacpi;
 module_param(noacpi, bool, 0444);
 MODULE_PARM_DESC(noacpi, "disable acpi bios quirks");
 
+static bool use_comp_batch __ro_after_init = true;
+static void nvme_setup_comp_batch(void)
+{
+#ifdef CONFIG_X86
+	if (boot_cpu_data.x86_vendor == X86_VENDOR_HYGON)
+		use_comp_batch = false;
+#endif
+}
+
 struct nvme_dev;
 struct nvme_queue;
 
@@ -1630,8 +1639,11 @@ static inline bool nvme_poll_cq(struct nvme_queue *nvmeq,
 static irqreturn_t nvme_irq(int irq, void *data)
 {
 	struct nvme_queue *nvmeq = data;
-	DEFINE_IO_COMP_BATCH(iob);
 
+	if (!use_comp_batch)
+		return nvme_poll_cq(nvmeq, NULL) ? IRQ_HANDLED : IRQ_NONE;
+
+	DEFINE_IO_COMP_BATCH(iob);
 	if (nvme_poll_cq(nvmeq, &iob)) {
 		if (!rq_list_empty(&iob.req_list))
 			nvme_pci_complete_batch(&iob);
@@ -3767,6 +3779,8 @@ static int nvme_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 
 	dev_info(dev->ctrl.device, "pci function %s\n", dev_name(&pdev->dev));
 
+	nvme_setup_comp_batch();
+
 	result = nvme_pci_enable(dev);
 	if (result)
 		goto out_release_iod_mempool;
-- 
2.43.0

Re: [PATCH] nvme-pci: Fix performance regression with completion batching for Hygon processors

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] nvme-pci: write to __ro_after_init use_comp_batch in nvme_probe()
--

commit f5d6be55aa6c877692b8138ca3509c22f309ca86
Author: Liao Xuan <liaoxuan@open-hieco.net>

nvme-pci: Fix performance regression with completion batching for Hygon processors

This commit introduces a use_comp_batch flag in drivers/nvme/host/pci.c to
disable IRQ completion batching in nvme_irq() when running on Hygon x86
processors.

> diff --git a/drivers/nvme/host/pci.c b/drivers/nvme/host/pci.c
> index 019a1e3cc0a2d..635755a886523 100644
> --- a/drivers/nvme/host/pci.c
> +++ b/drivers/nvme/host/pci.c
> @@ -278,6 +278,15 @@ static bool noacpi;
>  module_param(noacpi, bool, 0444);
>  MODULE_PARM_DESC(noacpi, "disable acpi bios quirks");
>  
> +static bool use_comp_batch __ro_after_init = true;
> +static void nvme_setup_comp_batch(void)
> +{
> +#ifdef CONFIG_X86
> +	if (boot_cpu_data.x86_vendor == X86_VENDOR_HYGON)
> +		use_comp_batch = false;
> +#endif
> +}
> +
>  struct nvme_dev;
>  struct nvme_queue;
>  

[ ... ]

> @@ -4014,6 +4026,8 @@ static int nvme_probe(struct pci_dev *pdev, const struct pci_device_id *id)
>  
>  	dev_info(dev->ctrl.device, "pci function %s\n", dev_name(&pdev->dev));
>  
> +	nvme_setup_comp_batch();

[Severity: High]
Can calling nvme_setup_comp_batch() from nvme_probe() cause a page fault by
writing to read-only memory?

In drivers/nvme/host/pci.c, use_comp_batch is annotated with __ro_after_init,
which places it in the .data..ro_after_init section that becomes read-only
after kernel or module init finishes.

However, nvme_probe() can be invoked after init has completed, such as during
PCIe hotplug, SR-IOV VF creation, PCI bus rescan, sysfs driver bind, or
asynchronous probing (since nvme_driver sets PROBE_PREFER_ASYNCHRONOUS). When
that happens on a Hygon CPU, nvme_probe() -> nvme_setup_comp_batch() writes
to use_comp_batch after .data..ro_after_init has been marked read-only. In
addition, concurrent nvme_probe() calls can write to use_comp_batch while
already active controllers read it in nvme_irq().

Since boot_cpu_data.x86_vendor is a system-wide property, should
nvme_setup_comp_batch() be marked __init and called once from nvme_init()
before pci_register_driver(&nvme_driver) instead of inside nvme_probe()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103063249.3212198-1-liaoxuan@open-hieco.net?part=1