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

[sashiko-bot@kernel.org]

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