All of lore.kernel.org
 help / color / mirror / Atom feed
* [patch virtio] virtio: delete vq in vp_find_vqs_msix() when request_irq() fails
@ 2024-04-26 15:08 Jiri Pirko
  2024-04-28  1:16 ` Xuan Zhuo
  2024-05-09 12:03 ` Jiri Pirko
  0 siblings, 2 replies; 5+ messages in thread
From: Jiri Pirko @ 2024-04-26 15:08 UTC (permalink / raw)
  To: virtualization; +Cc: mst, jasowang, xuanzhuo

From: Jiri Pirko <jiri@nvidia.com>

When request_irq() fails, error path calls vp_del_vqs(). There, as vq is
present in the list, free_irq() is called for the same vector. That
causes following splat:

[    0.414355] Trying to free already-free IRQ 27
[    0.414403] WARNING: CPU: 1 PID: 1 at kernel/irq/manage.c:1899 free_irq+0x1a1/0x2d0
[    0.414510] Modules linked in:
[    0.414540] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 6.9.0-rc4+ #27
[    0.414540] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-1.fc39 04/01/2014
[    0.414540] RIP: 0010:free_irq+0x1a1/0x2d0
[    0.414540] Code: 1e 00 48 83 c4 08 48 89 e8 5b 5d 41 5c 41 5d 41 5e 41 5f c3 cc cc cc cc 90 8b 74 24 04 48 c7 c7 98 80 6c b1 e8 00 c9 f7 ff 90 <0f> 0b 90 90 48 89 ee 4c 89 ef e8 e0 20 b8 00 49 8b 47 40 48 8b 40
[    0.414540] RSP: 0000:ffffb71480013ae0 EFLAGS: 00010086
[    0.414540] RAX: 0000000000000000 RBX: ffffa099c2722000 RCX: 0000000000000000
[    0.414540] RDX: 0000000000000000 RSI: ffffb71480013998 RDI: 0000000000000001
[    0.414540] RBP: 0000000000000246 R08: 00000000ffffdfff R09: 0000000000000001
[    0.414540] R10: 00000000ffffdfff R11: ffffffffb18729c0 R12: ffffa099c1c91760
[    0.414540] R13: ffffa099c1c916a4 R14: ffffa099c1d2f200 R15: ffffa099c1c91600
[    0.414540] FS:  0000000000000000(0000) GS:ffffa099fec40000(0000) knlGS:0000000000000000
[    0.414540] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[    0.414540] CR2: 0000000000000000 CR3: 0000000008e3e001 CR4: 0000000000370ef0
[    0.414540] Call Trace:
[    0.414540]  <TASK>
[    0.414540]  ? __warn+0x80/0x120
[    0.414540]  ? free_irq+0x1a1/0x2d0
[    0.414540]  ? report_bug+0x164/0x190
[    0.414540]  ? handle_bug+0x3b/0x70
[    0.414540]  ? exc_invalid_op+0x17/0x70
[    0.414540]  ? asm_exc_invalid_op+0x1a/0x20
[    0.414540]  ? free_irq+0x1a1/0x2d0
[    0.414540]  vp_del_vqs+0xc1/0x220
[    0.414540]  vp_find_vqs_msix+0x305/0x470
[    0.414540]  vp_find_vqs+0x3e/0x1a0
[    0.414540]  vp_modern_find_vqs+0x1b/0x70
[    0.414540]  init_vqs+0x387/0x600
[    0.414540]  virtnet_probe+0x50a/0xc80
[    0.414540]  virtio_dev_probe+0x1e0/0x2b0
[    0.414540]  really_probe+0xc0/0x2c0
[    0.414540]  ? __pfx___driver_attach+0x10/0x10
[    0.414540]  __driver_probe_device+0x73/0x120
[    0.414540]  driver_probe_device+0x1f/0xe0
[    0.414540]  __driver_attach+0x88/0x180
[    0.414540]  bus_for_each_dev+0x85/0xd0
[    0.414540]  bus_add_driver+0xec/0x1f0
[    0.414540]  driver_register+0x59/0x100
[    0.414540]  ? __pfx_virtio_net_driver_init+0x10/0x10
[    0.414540]  virtio_net_driver_init+0x90/0xb0
[    0.414540]  do_one_initcall+0x58/0x230
[    0.414540]  kernel_init_freeable+0x1a3/0x2d0
[    0.414540]  ? __pfx_kernel_init+0x10/0x10
[    0.414540]  kernel_init+0x1a/0x1c0
[    0.414540]  ret_from_fork+0x31/0x50
[    0.414540]  ? __pfx_kernel_init+0x10/0x10
[    0.414540]  ret_from_fork_asm+0x1a/0x30
[    0.414540]  </TASK>

Fix this by calling deleting the current vq when request_irq() fails.

Fixes: 0b0f9dc52ed0 ("Revert "virtio_pci: use shared interrupts for virtqueues"")
Signed-off-by: Jiri Pirko <jiri@nvidia.com>
---
 drivers/virtio/virtio_pci_common.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index b655fccaf773..584af7816532 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -348,8 +348,10 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned int nvqs,
 				  vring_interrupt, 0,
 				  vp_dev->msix_names[msix_vec],
 				  vqs[i]);
-		if (err)
+		if (err) {
+			vp_del_vq(vqs[i]);
 			goto error_find;
+		}
 	}
 	return 0;
 
-- 
2.44.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [patch virtio] virtio: delete vq in vp_find_vqs_msix() when request_irq() fails
  2024-04-26 15:08 [patch virtio] virtio: delete vq in vp_find_vqs_msix() when request_irq() fails Jiri Pirko
@ 2024-04-28  1:16 ` Xuan Zhuo
  2024-05-09 12:03 ` Jiri Pirko
  1 sibling, 0 replies; 5+ messages in thread
From: Xuan Zhuo @ 2024-04-28  1:16 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: mst, jasowang, virtualization

On Fri, 26 Apr 2024 17:08:45 +0200, Jiri Pirko <jiri@resnulli.us> wrote:
> From: Jiri Pirko <jiri@nvidia.com>
>
> When request_irq() fails, error path calls vp_del_vqs(). There, as vq is
> present in the list, free_irq() is called for the same vector. That
> causes following splat:
>
> [    0.414355] Trying to free already-free IRQ 27
> [    0.414403] WARNING: CPU: 1 PID: 1 at kernel/irq/manage.c:1899 free_irq+0x1a1/0x2d0
> [    0.414510] Modules linked in:
> [    0.414540] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 6.9.0-rc4+ #27
> [    0.414540] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-1.fc39 04/01/2014
> [    0.414540] RIP: 0010:free_irq+0x1a1/0x2d0
> [    0.414540] Code: 1e 00 48 83 c4 08 48 89 e8 5b 5d 41 5c 41 5d 41 5e 41 5f c3 cc cc cc cc 90 8b 74 24 04 48 c7 c7 98 80 6c b1 e8 00 c9 f7 ff 90 <0f> 0b 90 90 48 89 ee 4c 89 ef e8 e0 20 b8 00 49 8b 47 40 48 8b 40
> [    0.414540] RSP: 0000:ffffb71480013ae0 EFLAGS: 00010086
> [    0.414540] RAX: 0000000000000000 RBX: ffffa099c2722000 RCX: 0000000000000000
> [    0.414540] RDX: 0000000000000000 RSI: ffffb71480013998 RDI: 0000000000000001
> [    0.414540] RBP: 0000000000000246 R08: 00000000ffffdfff R09: 0000000000000001
> [    0.414540] R10: 00000000ffffdfff R11: ffffffffb18729c0 R12: ffffa099c1c91760
> [    0.414540] R13: ffffa099c1c916a4 R14: ffffa099c1d2f200 R15: ffffa099c1c91600
> [    0.414540] FS:  0000000000000000(0000) GS:ffffa099fec40000(0000) knlGS:0000000000000000
> [    0.414540] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [    0.414540] CR2: 0000000000000000 CR3: 0000000008e3e001 CR4: 0000000000370ef0
> [    0.414540] Call Trace:
> [    0.414540]  <TASK>
> [    0.414540]  ? __warn+0x80/0x120
> [    0.414540]  ? free_irq+0x1a1/0x2d0
> [    0.414540]  ? report_bug+0x164/0x190
> [    0.414540]  ? handle_bug+0x3b/0x70
> [    0.414540]  ? exc_invalid_op+0x17/0x70
> [    0.414540]  ? asm_exc_invalid_op+0x1a/0x20
> [    0.414540]  ? free_irq+0x1a1/0x2d0
> [    0.414540]  vp_del_vqs+0xc1/0x220
> [    0.414540]  vp_find_vqs_msix+0x305/0x470
> [    0.414540]  vp_find_vqs+0x3e/0x1a0
> [    0.414540]  vp_modern_find_vqs+0x1b/0x70
> [    0.414540]  init_vqs+0x387/0x600
> [    0.414540]  virtnet_probe+0x50a/0xc80
> [    0.414540]  virtio_dev_probe+0x1e0/0x2b0
> [    0.414540]  really_probe+0xc0/0x2c0
> [    0.414540]  ? __pfx___driver_attach+0x10/0x10
> [    0.414540]  __driver_probe_device+0x73/0x120
> [    0.414540]  driver_probe_device+0x1f/0xe0
> [    0.414540]  __driver_attach+0x88/0x180
> [    0.414540]  bus_for_each_dev+0x85/0xd0
> [    0.414540]  bus_add_driver+0xec/0x1f0
> [    0.414540]  driver_register+0x59/0x100
> [    0.414540]  ? __pfx_virtio_net_driver_init+0x10/0x10
> [    0.414540]  virtio_net_driver_init+0x90/0xb0
> [    0.414540]  do_one_initcall+0x58/0x230
> [    0.414540]  kernel_init_freeable+0x1a3/0x2d0
> [    0.414540]  ? __pfx_kernel_init+0x10/0x10
> [    0.414540]  kernel_init+0x1a/0x1c0
> [    0.414540]  ret_from_fork+0x31/0x50
> [    0.414540]  ? __pfx_kernel_init+0x10/0x10
> [    0.414540]  ret_from_fork_asm+0x1a/0x30
> [    0.414540]  </TASK>
>
> Fix this by calling deleting the current vq when request_irq() fails.
>
> Fixes: 0b0f9dc52ed0 ("Revert "virtio_pci: use shared interrupts for virtqueues"")
> Signed-off-by: Jiri Pirko <jiri@nvidia.com>



Reviewed-by: Xuan Zhuo <xuanzhuo@linux.alibaba.com>




> ---
>  drivers/virtio/virtio_pci_common.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> index b655fccaf773..584af7816532 100644
> --- a/drivers/virtio/virtio_pci_common.c
> +++ b/drivers/virtio/virtio_pci_common.c
> @@ -348,8 +348,10 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned int nvqs,
>  				  vring_interrupt, 0,
>  				  vp_dev->msix_names[msix_vec],
>  				  vqs[i]);
> -		if (err)
> +		if (err) {
> +			vp_del_vq(vqs[i]);
>  			goto error_find;
> +		}
>  	}
>  	return 0;
>
> --
> 2.44.0
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [patch virtio] virtio: delete vq in vp_find_vqs_msix() when request_irq() fails
  2024-04-26 15:08 [patch virtio] virtio: delete vq in vp_find_vqs_msix() when request_irq() fails Jiri Pirko
  2024-04-28  1:16 ` Xuan Zhuo
@ 2024-05-09 12:03 ` Jiri Pirko
  2024-05-09 12:40   ` Michael S. Tsirkin
  1 sibling, 1 reply; 5+ messages in thread
From: Jiri Pirko @ 2024-05-09 12:03 UTC (permalink / raw)
  To: virtualization; +Cc: mst, jasowang, xuanzhuo

Hi, I'm new in sending patches to virtio tree directly. Do I need to do
anything more in order to get this fix applied?

Thanks!

Fri, Apr 26, 2024 at 05:08:45PM CEST, jiri@resnulli.us wrote:
>From: Jiri Pirko <jiri@nvidia.com>
>
>When request_irq() fails, error path calls vp_del_vqs(). There, as vq is
>present in the list, free_irq() is called for the same vector. That
>causes following splat:
>
>[    0.414355] Trying to free already-free IRQ 27
>[    0.414403] WARNING: CPU: 1 PID: 1 at kernel/irq/manage.c:1899 free_irq+0x1a1/0x2d0
>[    0.414510] Modules linked in:
>[    0.414540] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 6.9.0-rc4+ #27
>[    0.414540] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.3-1.fc39 04/01/2014
>[    0.414540] RIP: 0010:free_irq+0x1a1/0x2d0
>[    0.414540] Code: 1e 00 48 83 c4 08 48 89 e8 5b 5d 41 5c 41 5d 41 5e 41 5f c3 cc cc cc cc 90 8b 74 24 04 48 c7 c7 98 80 6c b1 e8 00 c9 f7 ff 90 <0f> 0b 90 90 48 89 ee 4c 89 ef e8 e0 20 b8 00 49 8b 47 40 48 8b 40
>[    0.414540] RSP: 0000:ffffb71480013ae0 EFLAGS: 00010086
>[    0.414540] RAX: 0000000000000000 RBX: ffffa099c2722000 RCX: 0000000000000000
>[    0.414540] RDX: 0000000000000000 RSI: ffffb71480013998 RDI: 0000000000000001
>[    0.414540] RBP: 0000000000000246 R08: 00000000ffffdfff R09: 0000000000000001
>[    0.414540] R10: 00000000ffffdfff R11: ffffffffb18729c0 R12: ffffa099c1c91760
>[    0.414540] R13: ffffa099c1c916a4 R14: ffffa099c1d2f200 R15: ffffa099c1c91600
>[    0.414540] FS:  0000000000000000(0000) GS:ffffa099fec40000(0000) knlGS:0000000000000000
>[    0.414540] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>[    0.414540] CR2: 0000000000000000 CR3: 0000000008e3e001 CR4: 0000000000370ef0
>[    0.414540] Call Trace:
>[    0.414540]  <TASK>
>[    0.414540]  ? __warn+0x80/0x120
>[    0.414540]  ? free_irq+0x1a1/0x2d0
>[    0.414540]  ? report_bug+0x164/0x190
>[    0.414540]  ? handle_bug+0x3b/0x70
>[    0.414540]  ? exc_invalid_op+0x17/0x70
>[    0.414540]  ? asm_exc_invalid_op+0x1a/0x20
>[    0.414540]  ? free_irq+0x1a1/0x2d0
>[    0.414540]  vp_del_vqs+0xc1/0x220
>[    0.414540]  vp_find_vqs_msix+0x305/0x470
>[    0.414540]  vp_find_vqs+0x3e/0x1a0
>[    0.414540]  vp_modern_find_vqs+0x1b/0x70
>[    0.414540]  init_vqs+0x387/0x600
>[    0.414540]  virtnet_probe+0x50a/0xc80
>[    0.414540]  virtio_dev_probe+0x1e0/0x2b0
>[    0.414540]  really_probe+0xc0/0x2c0
>[    0.414540]  ? __pfx___driver_attach+0x10/0x10
>[    0.414540]  __driver_probe_device+0x73/0x120
>[    0.414540]  driver_probe_device+0x1f/0xe0
>[    0.414540]  __driver_attach+0x88/0x180
>[    0.414540]  bus_for_each_dev+0x85/0xd0
>[    0.414540]  bus_add_driver+0xec/0x1f0
>[    0.414540]  driver_register+0x59/0x100
>[    0.414540]  ? __pfx_virtio_net_driver_init+0x10/0x10
>[    0.414540]  virtio_net_driver_init+0x90/0xb0
>[    0.414540]  do_one_initcall+0x58/0x230
>[    0.414540]  kernel_init_freeable+0x1a3/0x2d0
>[    0.414540]  ? __pfx_kernel_init+0x10/0x10
>[    0.414540]  kernel_init+0x1a/0x1c0
>[    0.414540]  ret_from_fork+0x31/0x50
>[    0.414540]  ? __pfx_kernel_init+0x10/0x10
>[    0.414540]  ret_from_fork_asm+0x1a/0x30
>[    0.414540]  </TASK>
>
>Fix this by calling deleting the current vq when request_irq() fails.
>
>Fixes: 0b0f9dc52ed0 ("Revert "virtio_pci: use shared interrupts for virtqueues"")
>Signed-off-by: Jiri Pirko <jiri@nvidia.com>
>---
> drivers/virtio/virtio_pci_common.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
>index b655fccaf773..584af7816532 100644
>--- a/drivers/virtio/virtio_pci_common.c
>+++ b/drivers/virtio/virtio_pci_common.c
>@@ -348,8 +348,10 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned int nvqs,
> 				  vring_interrupt, 0,
> 				  vp_dev->msix_names[msix_vec],
> 				  vqs[i]);
>-		if (err)
>+		if (err) {
>+			vp_del_vq(vqs[i]);
> 			goto error_find;
>+		}
> 	}
> 	return 0;
> 
>-- 
>2.44.0
>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [patch virtio] virtio: delete vq in vp_find_vqs_msix() when request_irq() fails
  2024-05-09 12:03 ` Jiri Pirko
@ 2024-05-09 12:40   ` Michael S. Tsirkin
  2024-05-09 13:29     ` Jiri Pirko
  0 siblings, 1 reply; 5+ messages in thread
From: Michael S. Tsirkin @ 2024-05-09 12:40 UTC (permalink / raw)
  To: Jiri Pirko; +Cc: virtualization, jasowang, xuanzhuo

On Thu, May 09, 2024 at 02:03:15PM +0200, Jiri Pirko wrote:
> Hi, I'm new in sending patches to virtio tree directly. Do I need to do
> anything more in order to get this fix applied?
> 
> Thanks!

I never had patches with "patch" in lowercase so missed it ;)
Queued now and I'd prefer [PATCH virtio] in the future.
Thanks!


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [patch virtio] virtio: delete vq in vp_find_vqs_msix() when request_irq() fails
  2024-05-09 12:40   ` Michael S. Tsirkin
@ 2024-05-09 13:29     ` Jiri Pirko
  0 siblings, 0 replies; 5+ messages in thread
From: Jiri Pirko @ 2024-05-09 13:29 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: virtualization, jasowang, xuanzhuo

Thu, May 09, 2024 at 02:40:19PM CEST, mst@redhat.com wrote:
>On Thu, May 09, 2024 at 02:03:15PM +0200, Jiri Pirko wrote:
>> Hi, I'm new in sending patches to virtio tree directly. Do I need to do
>> anything more in order to get this fix applied?
>> 
>> Thanks!
>
>I never had patches with "patch" in lowercase so missed it ;)

Ha, interesting. Will do it next time.

>Queued now and I'd prefer [PATCH virtio] in the future.
>Thanks!

Thanks!

>

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-05-09 13:29 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-26 15:08 [patch virtio] virtio: delete vq in vp_find_vqs_msix() when request_irq() fails Jiri Pirko
2024-04-28  1:16 ` Xuan Zhuo
2024-05-09 12:03 ` Jiri Pirko
2024-05-09 12:40   ` Michael S. Tsirkin
2024-05-09 13:29     ` Jiri Pirko

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.