1. Patchew
  2. linux
  3. View series

[PATCH] virtio_pci: do not mutate caller irq_affinity.pre_vectors

Xiong Weimin posted 1 patch 1 month, 4 weeks ago
There is a newer version of this series
drivers/virtio/virtio_pci_common.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
[PATCH] virtio_pci: do not mutate caller irq_affinity.pre_vectors
Posted by Xiong Weimin 1 month, 4 weeks ago 
vp_request_msix_vectors() bumps desc->pre_vectors in place to reserve
the virtio config vector. vp_find_vqs() may call it more than once while
retrying MSI-X with different per-vq policies, so the caller's
irq_affinity keeps accumulating and later attempts get the wrong
affinity layout.

Copy the descriptor to a stack local, adjust pre_vectors there, and
pass that to pci_alloc_irq_vectors_affinity().

Fixes: ba74b6f7fcc0 ("virtio_pci: fix cpu affinity support")
Cc: stable@vger.kernel.org
Signed-off-by: Xiong Weimin <xiongweimin@kylinos.cn>
---
 drivers/virtio/virtio_pci_common.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index 164f480b1..9e75fd03e 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -128,6 +128,7 @@ static int vp_request_msix_vectors(struct virtio_device *vdev, int nvectors,
 {
 	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
 	const char *name = dev_name(&vp_dev->vdev.dev);
+	struct irq_affinity tmp_aff, *aff = NULL;
 	unsigned int flags = PCI_IRQ_MSIX;
 	unsigned int i, v;
 	int err = -ENOMEM;
@@ -146,16 +147,19 @@ static int vp_request_msix_vectors(struct virtio_device *vdev, int nvectors,
 					GFP_KERNEL))
 			goto error;
 
-	if (!per_vq_vectors)
-		desc = NULL;
-
-	if (desc) {
+	if (per_vq_vectors && desc) {
+		/*
+		 * Do not mutate the caller's irq_affinity across MSI-X
+		 * fallback retries in vp_find_vqs().
+		 */
+		tmp_aff = *desc;
+		tmp_aff.pre_vectors++; /* virtio config vector */
+		aff = &tmp_aff;
 		flags |= PCI_IRQ_AFFINITY;
-		desc->pre_vectors++; /* virtio config vector */
 	}
 
 	err = pci_alloc_irq_vectors_affinity(vp_dev->pci_dev, nvectors,
-					     nvectors, flags, desc);
+					     nvectors, flags, aff);
 	if (err < 0)
 		goto error;
 	vp_dev->msix_enabled = 1;
-- 
2.43.0
Re: [PATCH] virtio_pci: do not mutate caller irq_affinity.pre_vectors
Posted by Xiong Weimin 1 month, 3 weeks ago 
On the virtio_input thread, Michael asked whether each of these patches
is a real or a theoretical issue for stable. For this virtio_pci patch:

Real, when affinity is requested and MSI-X policy falls back.

vp_find_vqs() retries EACH -> SHARED_SLOW -> SHARED. Each failed
vp_request_msix_vectors() currently does desc->pre_vectors++ in place,
so a later successful attempt sees a polluted pre_vectors and gets the
wrong affinity layout. That fallback path is unconditional in the
driver.

I have not collected a userspace/IRQ-affinity failure log yet. Please
let me know if you still want Cc: stable on this one.

Thanks,
Xiong
Re: [PATCH] virtio_pci: do not mutate caller irq_affinity.pre_vectors
Posted by Michael S. Tsirkin 1 month, 3 weeks ago 
On Wed, Aug 05, 2026 at 11:29:34AM +0800, Xiong Weimin wrote:
> vp_request_msix_vectors() bumps desc->pre_vectors in place to reserve
> the virtio config vector. vp_find_vqs() may call it more than once while
> retrying MSI-X with different per-vq policies, so the caller's
> irq_affinity keeps accumulating and later attempts get the wrong
> affinity layout.
> 
> Copy the descriptor to a stack local, adjust pre_vectors there, and
> pass that to pci_alloc_irq_vectors_affinity().
> 
> Fixes: ba74b6f7fcc0 ("virtio_pci: fix cpu affinity support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Xiong Weimin <xiongweimin@kylinos.cn>
> ---
>  drivers/virtio/virtio_pci_common.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> index 164f480b1..9e75fd03e 100644
> --- a/drivers/virtio/virtio_pci_common.c
> +++ b/drivers/virtio/virtio_pci_common.c
> @@ -128,6 +128,7 @@ static int vp_request_msix_vectors(struct virtio_device *vdev, int nvectors,
>  {
>  	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
>  	const char *name = dev_name(&vp_dev->vdev.dev);
> +	struct irq_affinity tmp_aff, *aff = NULL;


"tmp" in what sense? if we can come up with a sensible name, just
"affinity" will do.


>  	unsigned int flags = PCI_IRQ_MSIX;
>  	unsigned int i, v;
>  	int err = -ENOMEM;
> @@ -146,16 +147,19 @@ static int vp_request_msix_vectors(struct virtio_device *vdev, int nvectors,
>  					GFP_KERNEL))
>  			goto error;
>  
> -	if (!per_vq_vectors)
> -		desc = NULL;
> -
> -	if (desc) {
> +	if (per_vq_vectors && desc) {
> +		/*
> +		 * Do not mutate the caller's irq_affinity across MSI-X
> +		 * fallback retries in vp_find_vqs().
> +		 */


We do not need a comment arguing with previous version of the code.

> +		tmp_aff = *desc;
> +		tmp_aff.pre_vectors++; /* virtio config vector */
> +		aff = &tmp_aff;


just assign to desc, instead?

>  		flags |= PCI_IRQ_AFFINITY;
> -		desc->pre_vectors++; /* virtio config vector */
>  	}
>  
>  	err = pci_alloc_irq_vectors_affinity(vp_dev->pci_dev, nvectors,
> -					     nvectors, flags, desc);
> +					     nvectors, flags, aff);
>  	if (err < 0)
>  		goto error;
>  	vp_dev->msix_enabled = 1;
> -- 
> 2.43.0
Re: [PATCH] virtio_pci: do not mutate caller irq_affinity.pre_vectors
Posted by Xiong Weimin 1 month, 3 weeks ago 
On Wed, Aug 05, 2026 at 01:37:01AM -0400, Michael S. Tsirkin wrote:
> "tmp" in what sense? if we can come up with a sensible name, just
> "affinity" will do.
>
> We do not need a comment arguing with previous version of the code.
>
> just assign to desc, instead?

Thanks, will do. I will post a fresh [PATCH v2] as a new standalone
mail (not threaded under this one) with:
- a local "affinity" copy
- that comment dropped
- desc = &affinity, and keep passing desc

On the virtio_input thread you also asked whether this is real or
theoretical for stable. For this virtio_pci patch: real when the caller
passes irq_affinity and MSI-X policy falls back.

vp_find_vqs() retries EACH -> SHARED_SLOW -> SHARED. A failed
vp_request_msix_vectors() currently does desc->pre_vectors++ in place,
so a later successful attempt sees a polluted pre_vectors and gets the
wrong affinity layout. That fallback path is unconditional in the
driver.

I have not collected a userspace failure log yet. Please let me know if
you still want Cc: stable on the v2.

Thanks,
Xiong
Re: [PATCH] virtio_pci: do not mutate caller irq_affinity.pre_vectors
Posted by Michael S. Tsirkin 1 month, 3 weeks ago 
On Wed, Aug 05, 2026 at 02:47:06PM +0800, Xiong Weimin wrote:
> On Wed, Aug 05, 2026 at 01:37:01AM -0400, Michael S. Tsirkin wrote:
> > "tmp" in what sense? if we can come up with a sensible name, just
> > "affinity" will do.
> >
> > We do not need a comment arguing with previous version of the code.
> >
> > just assign to desc, instead?
> 
> Thanks, will do. I will post a fresh [PATCH v2] as a new standalone
> mail (not threaded under this one) with:
> - a local "affinity" copy
> - that comment dropped
> - desc = &affinity, and keep passing desc
> 
> On the virtio_input thread you also asked whether this is real or
> theoretical for stable. For this virtio_pci patch: real when the caller
> passes irq_affinity and MSI-X policy falls back.
> 
> vp_find_vqs() retries EACH -> SHARED_SLOW -> SHARED. A failed
> vp_request_msix_vectors() currently does desc->pre_vectors++ in place,
> so a later successful attempt sees a polluted pre_vectors and gets the
> wrong affinity layout. That fallback path is unconditional in the
> driver.
> 
> I have not collected a userspace failure log yet. Please let me know if
> you still want Cc: stable on the v2.
> 
> Thanks,
> Xiong

do not cc stable on theoretical issue.

there is a bit of a language issue.

if the issue is real describe it:

"this was observed: ... "

if theoretical say so

-- 
MST

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.