Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1610050 > unrolled thread

Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use shared interrupts for virtqueues")

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use  shared interrupts for virtqueues") Mike Galbraith <efault@gmx.de> - 2017-03-27 20:30 +0200
    Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use  shared interrupts for virtqueues") Mike Galbraith <efault@gmx.de> - 2017-03-29 08:30 +0200
      Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use shared  interrupts for virtqueues") "Michael S. Tsirkin" <mst@redhat.com> - 2017-03-29 22:20 +0200
        Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use  shared interrupts for virtqueues") Mike Galbraith <efault@gmx.de> - 2017-03-30 09:30 +0200
          Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use shared  interrupts for virtqueues") "Michael S. Tsirkin" <mst@redhat.com> - 2017-03-31 05:30 +0200
            Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use         shared interrupts for virtqueues") Christoph Hellwig <hch@lst.de> - 2017-03-31 10:30 +0200
              Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use shared  interrupts for virtqueues") "Michael S. Tsirkin" <mst@redhat.com> - 2017-03-31 18:50 +0200
                Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use         shared interrupts for virtqueues") Christoph Hellwig <hch@lst.de> - 2017-04-03 16:20 +0200
                  Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use shared  interrupts for virtqueues") "Michael S. Tsirkin" <mst@redhat.com> - 2017-04-03 17:50 +0200
                  Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use shared  interrupts for virtqueues") "Michael S. Tsirkin" <mst@redhat.com> - 2017-04-03 18:20 +0200
                    Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use         shared interrupts for virtqueues") Christoph Hellwig <hch@lst.de> - 2017-04-05 08:40 +0200
                  Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use  shared interrupts for virtqueues") Mike Galbraith <efault@gmx.de> - 2017-04-03 20:00 +0200
                    Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use shared  interrupts for virtqueues") "Michael S. Tsirkin" <mst@redhat.com> - 2017-04-03 20:20 +0200
                      Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use  shared interrupts for virtqueues") Mike Galbraith <efault@gmx.de> - 2017-04-04 06:10 +0200
                        Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use shared  interrupts for virtqueues") "Michael S. Tsirkin" <mst@redhat.com> - 2017-04-04 15:40 +0200
                          Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use  shared interrupts for virtqueues") Mike Galbraith <efault@gmx.de> - 2017-04-04 16:20 +0200
                            Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use shared  interrupts for virtqueues") "Michael S. Tsirkin" <mst@redhat.com> - 2017-04-04 16:30 +0200
                            Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use shared  interrupts for virtqueues") "Michael S. Tsirkin" <mst@redhat.com> - 2017-04-04 17:40 +0200
                              Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use  shared interrupts for virtqueues") Mike Galbraith <efault@gmx.de> - 2017-04-04 19:50 +0200
                                Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use  shared interrupts for virtqueues") Mike Galbraith <efault@gmx.de> - 2017-04-04 20:00 +0200
                                  Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use shared  interrupts for virtqueues") "Michael S. Tsirkin" <mst@redhat.com> - 2017-04-04 20:10 +0200
                                    Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use  shared interrupts for virtqueues") Mike Galbraith <efault@gmx.de> - 2017-04-04 20:40 +0200
                                      Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use shared  interrupts for virtqueues") "Michael S. Tsirkin" <mst@redhat.com> - 2017-04-04 23:40 +0200
                                        Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use  shared interrupts for virtqueues") Mike Galbraith <efault@gmx.de> - 2017-04-05 05:00 +0200
                                  Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use shared  interrupts for virtqueues") "Michael S. Tsirkin" <mst@redhat.com> - 2017-04-04 21:10 +0200
                                    Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use  shared interrupts for virtqueues") Mike Galbraith <efault@gmx.de> - 2017-04-05 05:10 +0200
                                      Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use shared  interrupts for virtqueues") "Michael S. Tsirkin" <mst@redhat.com> - 2017-04-05 05:20 +0200
                                        Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use  shared interrupts for virtqueues") Mike Galbraith <efault@gmx.de> - 2017-04-05 05:30 +0200
                                          Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use  shared interrupts for virtqueues") Mike Galbraith <efault@gmx.de> - 2017-04-05 05:50 +0200
                                            Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use shared  interrupts for virtqueues") "Michael S. Tsirkin" <mst@redhat.com> - 2017-04-05 06:00 +0200
                                              Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use  shared interrupts for virtqueues") Mike Galbraith <efault@gmx.de> - 2017-04-05 06:30 +0200
                                                Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use         shared interrupts for virtqueues") Christoph Hellwig <hch@lst.de> - 2017-04-05 08:30 +0200
                                                  Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use  shared interrupts for virtqueues") Mike Galbraith <efault@gmx.de> - 2017-04-05 09:00 +0200
                                                  Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use shared  interrupts for virtqueues") "Michael S. Tsirkin" <mst@redhat.com> - 2017-04-05 23:40 +0200
                                                    Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use  shared interrupts for virtqueues") Mike Galbraith <efault@gmx.de> - 2017-04-07 08:10 +0200
                                                      Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use shared  interrupts for virtqueues") "Michael S. Tsirkin" <mst@redhat.com> - 2017-04-07 08:30 +0200
                                                        Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use  shared interrupts for virtqueues") Mike Galbraith <efault@gmx.de> - 2017-04-07 08:50 +0200
                                                          Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use  shared interrupts for virtqueues") Mike Galbraith <efault@gmx.de> - 2017-04-07 09:10 +0200
                                                            Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use  shared interrupts for virtqueues") Mike Galbraith <efault@gmx.de> - 2017-04-07 09:30 +0200
                                                              Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use  shared interrupts for virtqueues") Mike Galbraith <efault@gmx.de> - 2017-04-07 09:30 +0200
                                                              Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use shared  interrupts for virtqueues") "Michael S. Tsirkin" <mst@redhat.com> - 2017-04-07 15:30 +0200
                                                                Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use shared  interrupts for virtqueues") "Michael S. Tsirkin" <mst@redhat.com> - 2017-04-07 15:40 +0200
                                                                  Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use  shared interrupts for virtqueues") Mike Galbraith <efault@gmx.de> - 2017-04-07 16:40 +0200
                                                                    Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use shared  interrupts for virtqueues") "Michael S. Tsirkin" <mst@redhat.com> - 2017-04-07 21:00 +0200
                                                                      Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use  shared interrupts for virtqueues") Mike Galbraith <efault@gmx.de> - 2017-04-08 07:10 +0200
                                          Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use shared  interrupts for virtqueues") "Michael S. Tsirkin" <mst@redhat.com> - 2017-04-05 06:00 +0200
      Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use shared  interrupts for virtqueues") "Michael S. Tsirkin" <mst@redhat.com> - 2017-03-29 22:30 +0200
        Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use  shared interrupts for virtqueues") Mike Galbraith <efault@gmx.de> - 2017-03-30 06:00 +0200

Page 1 of 3  [1] 2 3  Next page →

#1610050 — Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use shared interrupts for virtqueues")

On Mon, 2017-03-27 at 19:05 +0200, Christoph Hellwig wrote:
> Hi Mike,
> 
> does the patch below fix that issue for you?

Thanks, I'll give it a go in the A.M.

BTW, WRT RT woes with $subject, I tried booting a generic kernel with
threadirqs, and bingo, same deal, just a bit more painful than for RT,
where there's no watchdog moaning accompanying the (preemptible) spin.

[   28.346311] NMI watchdog: BUG: soft lockup - CPU#7 stuck for 22s! [kworker/7:1:108]
[   28.347536] Modules linked in: virtio_rng(E) virtio_blk(E) virtio_console(E) ata_piix(E) qxl(E) drm_kms_helper(E) syscopyarea(E) sysfillrect(E) sysimgblt(E) fb_sys_fops(E) ttm(E) ahci(E) libahci(E) drm(E) ehci_pci(E) uhci_hcd(E) ehci_hcd(E) usbcore(E) libata(E) virtio_pci(E) virtio_ring(E) virtio(E) 8139cp(E) floppy(E) mii(E) sg(E) scsi_mod(E) autofs4(E)
[   28.351160] CPU: 7 PID: 108 Comm: kworker/7:1 Tainted: G            E   4.11.0-default #30
[   28.352085] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.8.1-0-g4adadbd-20161202_174313-build11a 04/01/2014
[   28.353547] Workqueue: events control_work_handler [virtio_console]
[   28.354450] task: ffff8802370d4440 task.stack: ffffc900010d8000
[   28.355281] RIP: 0010:__send_control_msg+0xbd/0xd0 [virtio_console]
[   28.356005] RSP: 0018:ffffc900010dbd20 EFLAGS: 00000246 ORIG_RAX: ffffffffffffff10
[   28.356987] RAX: 0000000000000000 RBX: ffff880231c31ec8 RCX: ffff880231cb1000
[   28.357866] RDX: 0000000000000001 RSI: ffffc900010dbd2c RDI: ffff880234f87400
[   28.358738] RBP: ffffc900010dbd78 R08: 0000000001080020 R09: ffffc900010dbd30
[   28.359718] R10: ffff88023fdddc00 R11: ffffffffffffffc8 R12: ffff880234f87400
[   28.360653] R13: ffff880231c31ea8 R14: 0000000000000001 R15: 0000000000000003
[   28.361510] FS:  0000000000000000(0000) GS:ffff88023fdc0000(0000) knlGS:0000000000000000
[   28.362433] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   28.363177] CR2: 00007f4da0f40000 CR3: 0000000001c09000 CR4: 00000000001406e0
[   28.363994] Call Trace:
[   28.364420]  add_port+0x23f/0x3d0 [virtio_console]
[   28.365094]  ? _raw_spin_unlock_irqrestore+0x24/0x40
[   28.365765]  handle_control_message.constprop.32+0x2c2/0x2e0 [virtio_console]
[   28.366622]  control_work_handler+0x52/0xb7 [virtio_console]
[   28.367291]  process_one_work+0x15c/0x440
[   28.367869]  worker_thread+0x137/0x4b0
[   28.368426]  kthread+0x10c/0x140
[   28.368921]  ? process_one_work+0x440/0x440
[   28.369477]  ? kthread_create_on_node+0x40/0x40
[   28.370067]  ret_from_fork+0x2c/0x40
[   28.370611] Code: 57 e1 48 83 c4 30 31 c0 5b 41 5c 41 5d 41 5e 41 5f 5d c3 4c 89 e7 e8 03 93 f7 ff eb 0e 4c 89 e7 e8 89 84 f7 ff 84 c0 75 d1 f3 90 <48> 8d 75 b4 4c 89 e7 e8 57 91 f7 ff 48 85 c0 74 e1 eb bc 0f 1f 

[toc] | [next] | [standalone]

#1611612

On Mon, 2017-03-27 at 20:18 +0200, Mike Galbraith wrote:

> BTW, WRT RT woes with $subject, I tried booting a generic kernel with
> threadirqs, and bingo, same deal, just a bit more painful than for RT,
> where there's no watchdog moaning accompanying the (preemptible) spin.

BTW++: the last hunk of this bandaid may be a bug fix.  With only the
first two, box tried to use uninitialized stuff on hibernate, went
boom.  Looks like that may be possible without help from me.

--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -2058,7 +2058,7 @@ static int virtcons_probe(struct virtio_
 	portdev->max_nr_ports = 1;
 
 	/* Don't test MULTIPORT at all if we're rproc: not a valid feature! */
-	if (!is_rproc_serial(vdev) &&
+	if (!is_rproc_serial(vdev) && !IS_ENABLED(CONFIG_IRQ_FORCED_THREADING) &&
 	    virtio_cread_feature(vdev, VIRTIO_CONSOLE_F_MULTIPORT,
 				 struct virtio_console_config, max_nr_ports,
 				 &portdev->max_nr_ports) == 0) {
@@ -2179,7 +2179,9 @@ static struct virtio_device_id id_table[
 
 static unsigned int features[] = {
 	VIRTIO_CONSOLE_F_SIZE,
+#ifndef CONFIG_IRQ_FORCED_THREADING
 	VIRTIO_CONSOLE_F_MULTIPORT,
+#endif
 };
 
 static struct virtio_device_id rproc_serial_id_table[] = {
@@ -2202,14 +2204,16 @@ static int virtcons_freeze(struct virtio
 
 	vdev->config->reset(vdev);
 
-	virtqueue_disable_cb(portdev->c_ivq);
+	if (use_multiport(portdev))
+		virtqueue_disable_cb(portdev->c_ivq);
 	cancel_work_sync(&portdev->control_work);
 	cancel_work_sync(&portdev->config_work);
 	/*
 	 * Once more: if control_work_handler() was running, it would
 	 * enable the cb as the last step.
 	 */
-	virtqueue_disable_cb(portdev->c_ivq);
+	if (use_multiport(portdev))
+		virtqueue_disable_cb(portdev->c_ivq);
 	remove_controlq_data(portdev);
 
 	list_for_each_entry(port, &portdev->ports, list) {

[toc] | [prev] | [next] | [standalone]

#1612294 — Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use shared interrupts for virtqueues")

On Wed, Mar 29, 2017 at 08:23:22AM +0200, Mike Galbraith wrote:
> On Mon, 2017-03-27 at 20:18 +0200, Mike Galbraith wrote:
> 
> > BTW, WRT RT woes with $subject, I tried booting a generic kernel with
> > threadirqs, and bingo, same deal, just a bit more painful than for RT,
> > where there's no watchdog moaning accompanying the (preemptible) spin.
> 
> BTW++: the last hunk of this bandaid may be a bug fix.  With only the
> first two, box tried to use uninitialized stuff on hibernate, went
> boom.  Looks like that may be possible without help from me.
> 
> --- a/drivers/char/virtio_console.c
> +++ b/drivers/char/virtio_console.c
> @@ -2058,7 +2058,7 @@ static int virtcons_probe(struct virtio_
>  	portdev->max_nr_ports = 1;
>  
>  	/* Don't test MULTIPORT at all if we're rproc: not a valid feature! */
> -	if (!is_rproc_serial(vdev) &&
> +	if (!is_rproc_serial(vdev) && !IS_ENABLED(CONFIG_IRQ_FORCED_THREADING) &&
>  	    virtio_cread_feature(vdev, VIRTIO_CONSOLE_F_MULTIPORT,
>  				 struct virtio_console_config, max_nr_ports,
>  				 &portdev->max_nr_ports) == 0) {
> @@ -2179,7 +2179,9 @@ static struct virtio_device_id id_table[
>  
>  static unsigned int features[] = {
>  	VIRTIO_CONSOLE_F_SIZE,
> +#ifndef CONFIG_IRQ_FORCED_THREADING
>  	VIRTIO_CONSOLE_F_MULTIPORT,
> +#endif
>  };
>  
>  static struct virtio_device_id rproc_serial_id_table[] = {
> @@ -2202,14 +2204,16 @@ static int virtcons_freeze(struct virtio
>  
>  	vdev->config->reset(vdev);
>  
> -	virtqueue_disable_cb(portdev->c_ivq);
> +	if (use_multiport(portdev))
> +		virtqueue_disable_cb(portdev->c_ivq);
>  	cancel_work_sync(&portdev->control_work);
>  	cancel_work_sync(&portdev->config_work);
>  	/*
>  	 * Once more: if control_work_handler() was running, it would
>  	 * enable the cb as the last step.
>  	 */
> -	virtqueue_disable_cb(portdev->c_ivq);
> +	if (use_multiport(portdev))
> +		virtqueue_disable_cb(portdev->c_ivq);
>  	remove_controlq_data(portdev);
>  
>  	list_for_each_entry(port, &portdev->ports, list) {


Poking at this some more, I was able to reproduce at
least some warnings. I still do not see a spin
but is there a chance this helps your case too?

commit 85039ca3162295759cf986aa753778043a90012c
Author: Michael S. Tsirkin <mst@redhat.com>
Date:   Wed Mar 29 23:02:28 2017 +0300

    virtio_pci: fix msix vector tracking on cleanup
    
    virtio pci tracks allocated vectors in a variable: msix_vectors. This
    isn't reset on del_vqs, as a result if reset is called after vqs are
    deleted we try to synchronize non-existing irqs producing a (probably
    harmless) warning.
    
    Fixes: 07ec51480b5e ("virtio_pci: use shared interrupts for virtqueues")
    Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index baae423..a70bed6 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -151,6 +151,7 @@ void vp_del_vqs(struct virtio_device *vdev)
 	}
 
 	free_irq(pci_irq_vector(vp_dev->pci_dev, 0), vp_dev);
+	vp_dev->msix_vectors = 0;
 	pci_free_irq_vectors(vp_dev->pci_dev);
 }
 
@@ -294,6 +295,7 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
 out_free_msix_names:
 	kfree(vp_dev->msix_names);
 out_free_irq_vectors:
+	vp_dev->msix_vectors = 0;
 	pci_free_irq_vectors(vp_dev->pci_dev);
 	return err;
 }

[toc] | [prev] | [next] | [standalone]

#1612651

On Thu, 2017-03-30 at 05:10 +0200, Mike Galbraith wrote:

> WRT spin, you should need do nothing more than boot with threadirqs,
> that's 100% repeatable here in absolutely virgin source.

No idea why virtqueue_get_buf() in __send_control_msg() fails forever
with threadirqs, but marking that vq as being busted (it clearly is)
results in one gripe, and a vbox that seemingly cares not one whit that
something went missing.  CONFIG_DEBUG_SHIRQ OTOH notices, mutters
something that sounds like "idiot" when I hibernate the thing ;-)

diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index e9b7e0b3cabe..831406dae1cb 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -567,6 +567,7 @@ static ssize_t __send_control_msg(struct ports_device *portdev, u32 port_id,
 	struct scatterlist sg[1];
 	struct virtqueue *vq;
 	unsigned int len;
+	unsigned long deadline = jiffies+1;
 
 	if (!use_multiport(portdev))
 		return 0;
@@ -583,9 +584,13 @@ static ssize_t __send_control_msg(struct ports_device *portdev, u32 port_id,
 
 	if (virtqueue_add_outbuf(vq, sg, 1, &portdev->cpkt, GFP_ATOMIC) == 0) {
 		virtqueue_kick(vq);
-		while (!virtqueue_get_buf(vq, &len)
-			&& !virtqueue_is_broken(vq))
+		while (!virtqueue_get_buf(vq, &len) && !virtqueue_is_broken(vq)) {
 			cpu_relax();
+			if (time_after(jiffies, deadline)) {
+				trace_printk("Aw crap, I'm stuck.. breaking device\n");
+				virtio_break_device(portdev->vdev);
+			}
+		}
 	}
 
 	spin_unlock(&portdev->c_ovq_lock);

[toc] | [prev] | [next] | [standalone]

#1613595 — Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use shared interrupts for virtqueues")

On Fri, Mar 31, 2017 at 04:23:35AM +0300, Michael S. Tsirkin wrote:
> On Thu, Mar 30, 2017 at 09:20:35AM +0200, Mike Galbraith wrote:
> > On Thu, 2017-03-30 at 05:10 +0200, Mike Galbraith wrote:
> > 
> > > WRT spin, you should need do nothing more than boot with threadirqs,
> > > that's 100% repeatable here in absolutely virgin source.
> > 
> > No idea why virtqueue_get_buf() in __send_control_msg() fails forever
> > with threadirqs, but marking that vq as being busted (it clearly is)
> > results in one gripe, and a vbox that seemingly cares not one whit that
> > something went missing.  CONFIG_DEBUG_SHIRQ OTOH notices, mutters
> > something that sounds like "idiot" when I hibernate the thing ;-)
> > 
> > diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
> > index e9b7e0b3cabe..831406dae1cb 100644
> > --- a/drivers/char/virtio_console.c
> > +++ b/drivers/char/virtio_console.c
> > @@ -567,6 +567,7 @@ static ssize_t __send_control_msg(struct ports_device *portdev, u32 port_id,
> >  	struct scatterlist sg[1];
> >  	struct virtqueue *vq;
> >  	unsigned int len;
> > +	unsigned long deadline = jiffies+1;
> >  
> >  	if (!use_multiport(portdev))
> >  		return 0;
> > @@ -583,9 +584,13 @@ static ssize_t __send_control_msg(struct ports_device *portdev, u32 port_id,
> >  
> >  	if (virtqueue_add_outbuf(vq, sg, 1, &portdev->cpkt, GFP_ATOMIC) == 0) {
> >  		virtqueue_kick(vq);
> > -		while (!virtqueue_get_buf(vq, &len)
> > -			&& !virtqueue_is_broken(vq))
> > +		while (!virtqueue_get_buf(vq, &len) && !virtqueue_is_broken(vq)) {
> >  			cpu_relax();
> > +			if (time_after(jiffies, deadline)) {
> > +				trace_printk("Aw crap, I'm stuck.. breaking device\n");
> > +				virtio_break_device(portdev->vdev);
> > +			}
> > +		}
> >  	}
> >  
> >  	spin_unlock(&portdev->c_ovq_lock);
> 
> 
> OK so with your help I was able to reproduce. Surprisingly easy:
> 
> 1. add threadirqs
> 2. add to qemu -device virtio-serial-pci -no-shutdown
> 3. within guest, do echo disk > /sys/power/state
> 
> This produces a warning. Looking deeper into it, I find:
> the device has 64 vqs. This line
> 
>                err = request_irq(pci_irq_vector(vp_dev->pci_dev, msix_vec),
>                                   vring_interrupt, IRQF_SHARED,
>                                   vp_dev->msix_names[j], vqs[i]);
> 
> fails after assigning interrupts to 33 vqs.
> Is there a limit to how many threaded irqs can share a line?

In fact it fails on the 33'rd one, and I see this:

/*
 * Unlikely to have 32 resp 64 irqs sharing one line,
 * but who knows.
 */
if (thread_mask == ~0UL) {
	printk(KERN_ERR "%s +%d\n", __FILE__, __LINE__);
	ret = -EBUSY;
	goto out_mask;
}


I'm not sure why does it fail after 32 on 64 bit, but as
virtio devices aren't limited to 32 vqs it looks like we
should go back to requesting the irq only once for all vqs.

Christoph, should I just revert for now, or do you
want to look into a smaller patch for this?

Another question is looking into intx support - that
should work but it seems to be broken at the moment.


> 
> If so we need to rethink the whole approach.
> 
> Still looking into it.
> 
> Christoph, any idea?
> 
> 
> -- 
> MST

[toc] | [prev] | [next] | [standalone]

#1613731 — Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use shared interrupts for virtqueues")

On Fri, Mar 31, 2017 at 06:22:31AM +0300, Michael S. Tsirkin wrote:
> I'm not sure why does it fail after 32 on 64 bit, but as
> virtio devices aren't limited to 32 vqs it looks like we
> should go back to requesting the irq only once for all vqs.

Meh.

> 
> Christoph, should I just revert for now, or do you
> want to look into a smaller patch for this?

I think we'll need to do a different patch than just a simple revert,
mostly because so much infrastructure depends on the patch.

I'll take a look over the weekend.

> Another question is looking into intx support - that
> should work but it seems to be broken at the moment.

Does it?  I'm pretty sure I tested it back when I came up with the
series by artifically disabling MSI-X in the kernel.  I can try this
again, though.

[toc] | [prev] | [next] | [standalone]

#1614145 — Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use shared interrupts for virtqueues")

On Fri, Mar 31, 2017 at 10:20:49AM +0200, Christoph Hellwig wrote:
> On Fri, Mar 31, 2017 at 06:22:31AM +0300, Michael S. Tsirkin wrote:
> > I'm not sure why does it fail after 32 on 64 bit, but as
> > virtio devices aren't limited to 32 vqs it looks like we
> > should go back to requesting the irq only once for all vqs.
> 
> Meh.
> 
> > 
> > Christoph, should I just revert for now, or do you
> > want to look into a smaller patch for this?
> 
> I think we'll need to do a different patch than just a simple revert,
> mostly because so much infrastructure depends on the patch.
> 
> I'll take a look over the weekend.
> 
> > Another question is looking into intx support - that
> > should work but it seems to be broken at the moment.
> 
> Does it?  I'm pretty sure I tested it back when I came up with the
> series by artifically disabling MSI-X in the kernel.  I can try this
> again, though.

I'm not 100% sure - what I see is that we do not handle failure to
request irqs correctly, we seem to fall back on intx but
the following freeze then blows up trying to free non-existing
vectors.

Does not seem to trigger with just msix off so maybe that is
simply failure to recover from an error correctly.


-- 
MST

[toc] | [prev] | [next] | [standalone]

#1615230 — Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use shared interrupts for virtqueues")

Mike,

can you try the patch below?

---
From fe41a30b54878cc631623b7511267125e0da4b15 Mon Sep 17 00:00:00 2001
From: Christoph Hellwig <hch@lst.de>
Date: Mon, 3 Apr 2017 14:51:35 +0200
Subject: virtio_pci: don't use shared irq for virtqueues

Reimplement the shared irq feature manually, as we might have a larger
number of virtqueues than the core shared interrupt code can handle
in threaded interrupt mode.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/virtio/virtio_pci_common.c | 142 +++++++++++++++++++++----------------
 drivers/virtio/virtio_pci_common.h |   1 +
 2 files changed, 83 insertions(+), 60 deletions(-)

diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
index 590534910dc6..6dd719543410 100644
--- a/drivers/virtio/virtio_pci_common.c
+++ b/drivers/virtio/virtio_pci_common.c
@@ -137,6 +137,9 @@ void vp_del_vqs(struct virtio_device *vdev)
 		kfree(vp_dev->msix_vector_map);
 	}
 
+	/* free the shared virtuqueue irq if we don't use per-vq irqs */
+	if (vp_dev->shared_vq_vec)
+		free_irq(pci_irq_vector(vp_dev->pci_dev, 1), vp_dev);
 	free_irq(pci_irq_vector(vp_dev->pci_dev, 0), vp_dev);
 	pci_free_irq_vectors(vp_dev->pci_dev);
 }
@@ -147,10 +150,10 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
 {
 	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
 	const char *name = dev_name(&vp_dev->vdev.dev);
-	int i, j, err = -ENOMEM, allocated_vectors, nvectors;
+	struct pci_dev *pdev = vp_dev->pci_dev;
+	int i, err = -ENOMEM, nvectors;
 	unsigned flags = PCI_IRQ_MSIX;
-	bool shared = false;
-	u16 msix_vec;
+	u16 msix_vec = 0;
 
 	if (desc) {
 		flags |= PCI_IRQ_AFFINITY;
@@ -162,19 +165,18 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
 		if (callbacks[i])
 			nvectors++;
 
-	/* Try one vector per queue first. */
-	err = pci_alloc_irq_vectors_affinity(vp_dev->pci_dev, nvectors,
-			nvectors, flags, desc);
+	/* Try one vector for config and one per queue first. */
+	err = pci_alloc_irq_vectors_affinity(pdev, nvectors, nvectors, flags,
+			desc);
 	if (err < 0) {
 		/* Fallback to one vector for config, one shared for queues. */
-		shared = true;
-		err = pci_alloc_irq_vectors(vp_dev->pci_dev, 2, 2,
+		nvectors = 2;
+		vp_dev->shared_vq_vec = true;
+		err = pci_alloc_irq_vectors(pdev, nvectors, nvectors,
 				PCI_IRQ_MSIX);
 		if (err < 0)
 			return err;
 	}
-	if (err < 0)
-		return err;
 
 	vp_dev->msix_vectors = nvectors;
 	vp_dev->msix_names = kmalloc_array(nvectors,
@@ -194,79 +196,99 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
 	}
 
 	/* Set the vector used for configuration */
-	snprintf(vp_dev->msix_names[0], sizeof(*vp_dev->msix_names),
+	snprintf(vp_dev->msix_names[msix_vec], sizeof(*vp_dev->msix_names),
 		 "%s-config", name);
-	err = request_irq(pci_irq_vector(vp_dev->pci_dev, 0), vp_config_changed,
-			0, vp_dev->msix_names[0], vp_dev);
+	err = request_irq(pci_irq_vector(pdev, msix_vec), vp_config_changed, 0,
+			  vp_dev->msix_names[msix_vec], vp_dev);
 	if (err)
 		goto out_free_msix_affinity_masks;
 
 	/* Verify we had enough resources to assign the vector */
-	if (vp_dev->config_vector(vp_dev, 0) == VIRTIO_MSI_NO_VECTOR) {
+	if (vp_dev->config_vector(vp_dev, msix_vec) == VIRTIO_MSI_NO_VECTOR) {
 		err = -EBUSY;
 		goto out_free_config_irq;
 	}
 
-	vp_dev->msix_vector_map = kmalloc_array(nvqs,
-			sizeof(*vp_dev->msix_vector_map), GFP_KERNEL);
-	if (!vp_dev->msix_vector_map)
-		goto out_disable_config_irq;
-
-	allocated_vectors = j = 1; /* vector 0 is the config interrupt */
-	for (i = 0; i < nvqs; ++i) {
-		if (!names[i]) {
-			vqs[i] = NULL;
-			continue;
-		}
-
-		if (callbacks[i])
-			msix_vec = allocated_vectors;
-		else
-			msix_vec = VIRTIO_MSI_NO_VECTOR;
-
-		vqs[i] = vp_dev->setup_vq(vp_dev, i, callbacks[i], names[i],
-				msix_vec);
-		if (IS_ERR(vqs[i])) {
-			err = PTR_ERR(vqs[i]);
-			goto out_remove_vqs;
+	msix_vec++;
+
+	/*
+	 * Use a different vector for each queue if they are available,
+	 * else share the same vector for all VQs.
+	 */
+	if (vp_dev->shared_vq_vec) {
+		snprintf(vp_dev->msix_names[msix_vec],
+			 sizeof(vp_dev->msix_names[msix_vec]),
+			 "%s-virtqueues", name);
+		err = request_irq(pci_irq_vector(pdev, msix_vec),
+				vp_vring_interrupt, 0,
+				vp_dev->msix_names[msix_vec], vp_dev);
+		if (err)
+			goto out_disable_config_irq;
+
+		for (i = 0; i < nvqs; ++i) {
+			if (!names[i]) {
+				vqs[i] = NULL;
+				continue;
+			}
+
+			vqs[i] = vp_dev->setup_vq(vp_dev, i, callbacks[i],
+					names[i], callbacks[i] ?
+					msix_vec : VIRTIO_MSI_NO_VECTOR);
+			if (IS_ERR(vqs[i])) {
+				err = PTR_ERR(vqs[i]);
+				goto out_remove_vqs;
+			}
 		}
+	} else {
+		vp_dev->msix_vector_map = kmalloc_array(nvqs,
+				sizeof(*vp_dev->msix_vector_map), GFP_KERNEL);
+		if (!vp_dev->msix_vector_map)
+			goto out_disable_config_irq;
 
-		if (msix_vec == VIRTIO_MSI_NO_VECTOR) {
-			vp_dev->msix_vector_map[i] = VIRTIO_MSI_NO_VECTOR;
-			continue;
-		}
+		for (i = 0; i < nvqs; ++i) {
+			if (!names[i]) {
+				vqs[i] = NULL;
+				continue;
+			}
 
-		snprintf(vp_dev->msix_names[j],
-			 sizeof(*vp_dev->msix_names), "%s-%s",
-			 dev_name(&vp_dev->vdev.dev), names[i]);
-		err = request_irq(pci_irq_vector(vp_dev->pci_dev, msix_vec),
-				  vring_interrupt, IRQF_SHARED,
-				  vp_dev->msix_names[j], vqs[i]);
-		if (err) {
 			/* don't free this irq on error */
 			vp_dev->msix_vector_map[i] = VIRTIO_MSI_NO_VECTOR;
-			goto out_remove_vqs;
+
+			vqs[i] = vp_dev->setup_vq(vp_dev, i, callbacks[i],
+					names[i], callbacks[i] ?
+					msix_vec : VIRTIO_MSI_NO_VECTOR);
+			if (IS_ERR(vqs[i])) {
+				err = PTR_ERR(vqs[i]);
+				goto out_remove_vqs;
+			}
+
+			if (!callbacks[i])
+				continue;
+
+			snprintf(vp_dev->msix_names[msix_vec],
+				 sizeof(*vp_dev->msix_names), "%s-%s",
+				 dev_name(&vp_dev->vdev.dev), names[i]);
+			err = request_irq(pci_irq_vector(pdev, msix_vec),
+					  vring_interrupt, IRQF_SHARED,
+					  vp_dev->msix_names[msix_vec],
+					  vqs[i]);
+			if (err)
+				goto out_remove_vqs;
+			vp_dev->msix_vector_map[i] = msix_vec++;
 		}
-		vp_dev->msix_vector_map[i] = msix_vec;
-		j++;
-
-		/*
-		 * Use a different vector for each queue if they are available,
-		 * else share the same vector for all VQs.
-		 */
-		if (!shared)
-			allocated_vectors++;
 	}
 
 	return 0;
 
 out_remove_vqs:
 	vp_remove_vqs(vdev);
+	if (vp_dev->shared_vq_vec)
+		free_irq(pci_irq_vector(pdev, 1), vp_dev);
 	kfree(vp_dev->msix_vector_map);
 out_disable_config_irq:
 	vp_dev->config_vector(vp_dev, VIRTIO_MSI_NO_VECTOR);
 out_free_config_irq:
-	free_irq(pci_irq_vector(vp_dev->pci_dev, 0), vp_dev);
+	free_irq(pci_irq_vector(pdev, 0), vp_dev);
 out_free_msix_affinity_masks:
 	for (i = 0; i < nvectors; i++) {
 		if (vp_dev->msix_affinity_masks[i])
@@ -276,7 +298,7 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
 out_free_msix_names:
 	kfree(vp_dev->msix_names);
 out_free_irq_vectors:
-	pci_free_irq_vectors(vp_dev->pci_dev);
+	pci_free_irq_vectors(pdev);
 	return err;
 }
 
@@ -346,7 +368,7 @@ int vp_set_vq_affinity(struct virtqueue *vq, int cpu)
 	if (!vq->callback)
 		return -EINVAL;
 
-	if (vp_dev->pci_dev->msix_enabled) {
+	if (vp_dev->msix_vector_map) {
 		int vec = vp_dev->msix_vector_map[vq->index];
 		struct cpumask *mask = vp_dev->msix_affinity_masks[vec];
 		unsigned int irq = pci_irq_vector(vp_dev->pci_dev, vec);
diff --git a/drivers/virtio/virtio_pci_common.h b/drivers/virtio/virtio_pci_common.h
index ac8c9d788964..d6d7fb99e47f 100644
--- a/drivers/virtio/virtio_pci_common.h
+++ b/drivers/virtio/virtio_pci_common.h
@@ -72,6 +72,7 @@ struct virtio_pci_device {
 	int msix_vectors;
 	/* Map of per-VQ MSI-X vectors, may be NULL */
 	unsigned *msix_vector_map;
+	bool shared_vq_vec;
 
 	struct virtqueue *(*setup_vq)(struct virtio_pci_device *vp_dev,
 				      unsigned idx,
-- 
2.11.0

[toc] | [prev] | [next] | [standalone]

#1615356 — Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use shared interrupts for virtqueues")

On Mon, Apr 03, 2017 at 04:18:23PM +0200, Christoph Hellwig wrote:
> Mike,
> 
> can you try the patch below?

It's really easy to test on qemu so I will - just add a dummy
virtio-serial-pci device with -device virtio-serial-pci and
add threadirqs to kernel command line.

However it doesn't look like this will fix the error recovery
for when request irq fails - it will just make the error less likely.

So we still need to look into that - failure should recover
and use the intx path, ATM it causes hybernation to hang.

> ---
> >From fe41a30b54878cc631623b7511267125e0da4b15 Mon Sep 17 00:00:00 2001
> From: Christoph Hellwig <hch@lst.de>
> Date: Mon, 3 Apr 2017 14:51:35 +0200
> Subject: virtio_pci: don't use shared irq for virtqueues
> 
> Reimplement the shared irq feature manually, as we might have a larger
> number of virtqueues than the core shared interrupt code can handle
> in threaded interrupt mode.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/virtio/virtio_pci_common.c | 142 +++++++++++++++++++++----------------
>  drivers/virtio/virtio_pci_common.h |   1 +
>  2 files changed, 83 insertions(+), 60 deletions(-)
> 
> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> index 590534910dc6..6dd719543410 100644
> --- a/drivers/virtio/virtio_pci_common.c
> +++ b/drivers/virtio/virtio_pci_common.c
> @@ -137,6 +137,9 @@ void vp_del_vqs(struct virtio_device *vdev)
>  		kfree(vp_dev->msix_vector_map);
>  	}
>  
> +	/* free the shared virtuqueue irq if we don't use per-vq irqs */
> +	if (vp_dev->shared_vq_vec)
> +		free_irq(pci_irq_vector(vp_dev->pci_dev, 1), vp_dev);
>  	free_irq(pci_irq_vector(vp_dev->pci_dev, 0), vp_dev);
>  	pci_free_irq_vectors(vp_dev->pci_dev);
>  }
> @@ -147,10 +150,10 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
>  {
>  	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
>  	const char *name = dev_name(&vp_dev->vdev.dev);
> -	int i, j, err = -ENOMEM, allocated_vectors, nvectors;
> +	struct pci_dev *pdev = vp_dev->pci_dev;
> +	int i, err = -ENOMEM, nvectors;
>  	unsigned flags = PCI_IRQ_MSIX;
> -	bool shared = false;
> -	u16 msix_vec;
> +	u16 msix_vec = 0;
>  
>  	if (desc) {
>  		flags |= PCI_IRQ_AFFINITY;
> @@ -162,19 +165,18 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
>  		if (callbacks[i])
>  			nvectors++;
>  
> -	/* Try one vector per queue first. */
> -	err = pci_alloc_irq_vectors_affinity(vp_dev->pci_dev, nvectors,
> -			nvectors, flags, desc);
> +	/* Try one vector for config and one per queue first. */
> +	err = pci_alloc_irq_vectors_affinity(pdev, nvectors, nvectors, flags,
> +			desc);
>  	if (err < 0) {
>  		/* Fallback to one vector for config, one shared for queues. */
> -		shared = true;
> -		err = pci_alloc_irq_vectors(vp_dev->pci_dev, 2, 2,
> +		nvectors = 2;
> +		vp_dev->shared_vq_vec = true;
> +		err = pci_alloc_irq_vectors(pdev, nvectors, nvectors,
>  				PCI_IRQ_MSIX);
>  		if (err < 0)
>  			return err;
>  	}
> -	if (err < 0)
> -		return err;
>  
>  	vp_dev->msix_vectors = nvectors;
>  	vp_dev->msix_names = kmalloc_array(nvectors,
> @@ -194,79 +196,99 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
>  	}
>  
>  	/* Set the vector used for configuration */
> -	snprintf(vp_dev->msix_names[0], sizeof(*vp_dev->msix_names),
> +	snprintf(vp_dev->msix_names[msix_vec], sizeof(*vp_dev->msix_names),
>  		 "%s-config", name);
> -	err = request_irq(pci_irq_vector(vp_dev->pci_dev, 0), vp_config_changed,
> -			0, vp_dev->msix_names[0], vp_dev);
> +	err = request_irq(pci_irq_vector(pdev, msix_vec), vp_config_changed, 0,
> +			  vp_dev->msix_names[msix_vec], vp_dev);
>  	if (err)
>  		goto out_free_msix_affinity_masks;
>  
>  	/* Verify we had enough resources to assign the vector */
> -	if (vp_dev->config_vector(vp_dev, 0) == VIRTIO_MSI_NO_VECTOR) {
> +	if (vp_dev->config_vector(vp_dev, msix_vec) == VIRTIO_MSI_NO_VECTOR) {
>  		err = -EBUSY;
>  		goto out_free_config_irq;
>  	}
>  
> -	vp_dev->msix_vector_map = kmalloc_array(nvqs,
> -			sizeof(*vp_dev->msix_vector_map), GFP_KERNEL);
> -	if (!vp_dev->msix_vector_map)
> -		goto out_disable_config_irq;
> -
> -	allocated_vectors = j = 1; /* vector 0 is the config interrupt */
> -	for (i = 0; i < nvqs; ++i) {
> -		if (!names[i]) {
> -			vqs[i] = NULL;
> -			continue;
> -		}
> -
> -		if (callbacks[i])
> -			msix_vec = allocated_vectors;
> -		else
> -			msix_vec = VIRTIO_MSI_NO_VECTOR;
> -
> -		vqs[i] = vp_dev->setup_vq(vp_dev, i, callbacks[i], names[i],
> -				msix_vec);
> -		if (IS_ERR(vqs[i])) {
> -			err = PTR_ERR(vqs[i]);
> -			goto out_remove_vqs;
> +	msix_vec++;
> +
> +	/*
> +	 * Use a different vector for each queue if they are available,
> +	 * else share the same vector for all VQs.
> +	 */
> +	if (vp_dev->shared_vq_vec) {
> +		snprintf(vp_dev->msix_names[msix_vec],
> +			 sizeof(vp_dev->msix_names[msix_vec]),
> +			 "%s-virtqueues", name);
> +		err = request_irq(pci_irq_vector(pdev, msix_vec),
> +				vp_vring_interrupt, 0,
> +				vp_dev->msix_names[msix_vec], vp_dev);
> +		if (err)
> +			goto out_disable_config_irq;
> +
> +		for (i = 0; i < nvqs; ++i) {
> +			if (!names[i]) {
> +				vqs[i] = NULL;
> +				continue;
> +			}
> +
> +			vqs[i] = vp_dev->setup_vq(vp_dev, i, callbacks[i],
> +					names[i], callbacks[i] ?
> +					msix_vec : VIRTIO_MSI_NO_VECTOR);
> +			if (IS_ERR(vqs[i])) {
> +				err = PTR_ERR(vqs[i]);
> +				goto out_remove_vqs;
> +			}
>  		}
> +	} else {
> +		vp_dev->msix_vector_map = kmalloc_array(nvqs,
> +				sizeof(*vp_dev->msix_vector_map), GFP_KERNEL);
> +		if (!vp_dev->msix_vector_map)
> +			goto out_disable_config_irq;
>  
> -		if (msix_vec == VIRTIO_MSI_NO_VECTOR) {
> -			vp_dev->msix_vector_map[i] = VIRTIO_MSI_NO_VECTOR;
> -			continue;
> -		}
> +		for (i = 0; i < nvqs; ++i) {
> +			if (!names[i]) {
> +				vqs[i] = NULL;
> +				continue;
> +			}
>  
> -		snprintf(vp_dev->msix_names[j],
> -			 sizeof(*vp_dev->msix_names), "%s-%s",
> -			 dev_name(&vp_dev->vdev.dev), names[i]);
> -		err = request_irq(pci_irq_vector(vp_dev->pci_dev, msix_vec),
> -				  vring_interrupt, IRQF_SHARED,
> -				  vp_dev->msix_names[j], vqs[i]);
> -		if (err) {
>  			/* don't free this irq on error */
>  			vp_dev->msix_vector_map[i] = VIRTIO_MSI_NO_VECTOR;
> -			goto out_remove_vqs;
> +
> +			vqs[i] = vp_dev->setup_vq(vp_dev, i, callbacks[i],
> +					names[i], callbacks[i] ?
> +					msix_vec : VIRTIO_MSI_NO_VECTOR);
> +			if (IS_ERR(vqs[i])) {
> +				err = PTR_ERR(vqs[i]);
> +				goto out_remove_vqs;
> +			}
> +
> +			if (!callbacks[i])
> +				continue;
> +
> +			snprintf(vp_dev->msix_names[msix_vec],
> +				 sizeof(*vp_dev->msix_names), "%s-%s",
> +				 dev_name(&vp_dev->vdev.dev), names[i]);
> +			err = request_irq(pci_irq_vector(pdev, msix_vec),
> +					  vring_interrupt, IRQF_SHARED,
> +					  vp_dev->msix_names[msix_vec],
> +					  vqs[i]);
> +			if (err)
> +				goto out_remove_vqs;
> +			vp_dev->msix_vector_map[i] = msix_vec++;
>  		}
> -		vp_dev->msix_vector_map[i] = msix_vec;
> -		j++;
> -
> -		/*
> -		 * Use a different vector for each queue if they are available,
> -		 * else share the same vector for all VQs.
> -		 */
> -		if (!shared)
> -			allocated_vectors++;
>  	}
>  
>  	return 0;
>  
>  out_remove_vqs:
>  	vp_remove_vqs(vdev);
> +	if (vp_dev->shared_vq_vec)
> +		free_irq(pci_irq_vector(pdev, 1), vp_dev);
>  	kfree(vp_dev->msix_vector_map);
>  out_disable_config_irq:
>  	vp_dev->config_vector(vp_dev, VIRTIO_MSI_NO_VECTOR);
>  out_free_config_irq:
> -	free_irq(pci_irq_vector(vp_dev->pci_dev, 0), vp_dev);
> +	free_irq(pci_irq_vector(pdev, 0), vp_dev);
>  out_free_msix_affinity_masks:
>  	for (i = 0; i < nvectors; i++) {
>  		if (vp_dev->msix_affinity_masks[i])
> @@ -276,7 +298,7 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
>  out_free_msix_names:
>  	kfree(vp_dev->msix_names);
>  out_free_irq_vectors:
> -	pci_free_irq_vectors(vp_dev->pci_dev);
> +	pci_free_irq_vectors(pdev);
>  	return err;
>  }
>  
> @@ -346,7 +368,7 @@ int vp_set_vq_affinity(struct virtqueue *vq, int cpu)
>  	if (!vq->callback)
>  		return -EINVAL;
>  
> -	if (vp_dev->pci_dev->msix_enabled) {
> +	if (vp_dev->msix_vector_map) {
>  		int vec = vp_dev->msix_vector_map[vq->index];
>  		struct cpumask *mask = vp_dev->msix_affinity_masks[vec];
>  		unsigned int irq = pci_irq_vector(vp_dev->pci_dev, vec);
> diff --git a/drivers/virtio/virtio_pci_common.h b/drivers/virtio/virtio_pci_common.h
> index ac8c9d788964..d6d7fb99e47f 100644
> --- a/drivers/virtio/virtio_pci_common.h
> +++ b/drivers/virtio/virtio_pci_common.h
> @@ -72,6 +72,7 @@ struct virtio_pci_device {
>  	int msix_vectors;
>  	/* Map of per-VQ MSI-X vectors, may be NULL */
>  	unsigned *msix_vector_map;
> +	bool shared_vq_vec;
>  
>  	struct virtqueue *(*setup_vq)(struct virtio_pci_device *vp_dev,
>  				      unsigned idx,
> -- 
> 2.11.0

[toc] | [prev] | [next] | [standalone]

#1615381 — Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use shared interrupts for virtqueues")

On Mon, Apr 03, 2017 at 04:18:23PM +0200, Christoph Hellwig wrote:
> Mike,
> 
> can you try the patch below?
> 
> ---
> >From fe41a30b54878cc631623b7511267125e0da4b15 Mon Sep 17 00:00:00 2001
> From: Christoph Hellwig <hch@lst.de>
> Date: Mon, 3 Apr 2017 14:51:35 +0200
> Subject: virtio_pci: don't use shared irq for virtqueues
> 
> Reimplement the shared irq feature manually, as we might have a larger
> number of virtqueues than the core shared interrupt code can handle
> in threaded interrupt mode.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  drivers/virtio/virtio_pci_common.c | 142 +++++++++++++++++++++----------------
>  drivers/virtio/virtio_pci_common.h |   1 +
>  2 files changed, 83 insertions(+), 60 deletions(-)

Well the original patch this is trying to fix is
07ec51480b5eb1233f8c1b0f5d7a7c8d1247c507 which dropped just 40 lines
with documentation. It did this by re-using error handling to switch
from per-vq to non-per-vq mode. Now this has separate flows for errors
and per-vq non-per-vq switch and (I think, as a result) is adding 140
lines which doesn't make me very happy.

> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c
> index 590534910dc6..6dd719543410 100644
> --- a/drivers/virtio/virtio_pci_common.c
> +++ b/drivers/virtio/virtio_pci_common.c
> @@ -137,6 +137,9 @@ void vp_del_vqs(struct virtio_device *vdev)
>  		kfree(vp_dev->msix_vector_map);
>  	}
>  
> +	/* free the shared virtuqueue irq if we don't use per-vq irqs */

typo

> +	if (vp_dev->shared_vq_vec)
> +		free_irq(pci_irq_vector(vp_dev->pci_dev, 1), vp_dev);

So we used to have enums for 1 and 0. I think it was cleaner.


>  	free_irq(pci_irq_vector(vp_dev->pci_dev, 0), vp_dev);
>  	pci_free_irq_vectors(vp_dev->pci_dev);
>  }
> @@ -147,10 +150,10 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
>  {
>  	struct virtio_pci_device *vp_dev = to_vp_device(vdev);
>  	const char *name = dev_name(&vp_dev->vdev.dev);
> -	int i, j, err = -ENOMEM, allocated_vectors, nvectors;
> +	struct pci_dev *pdev = vp_dev->pci_dev;
> +	int i, err = -ENOMEM, nvectors;
>  	unsigned flags = PCI_IRQ_MSIX;
> -	bool shared = false;
> -	u16 msix_vec;
> +	u16 msix_vec = 0;
>  
>  	if (desc) {
>  		flags |= PCI_IRQ_AFFINITY;
> @@ -162,19 +165,18 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
>  		if (callbacks[i])
>  			nvectors++;
>  
> -	/* Try one vector per queue first. */
> -	err = pci_alloc_irq_vectors_affinity(vp_dev->pci_dev, nvectors,
> -			nvectors, flags, desc);
> +	/* Try one vector for config and one per queue first. */
> +	err = pci_alloc_irq_vectors_affinity(pdev, nvectors, nvectors, flags,
> +			desc);
>  	if (err < 0) {
>  		/* Fallback to one vector for config, one shared for queues. */
> -		shared = true;
> -		err = pci_alloc_irq_vectors(vp_dev->pci_dev, 2, 2,
> +		nvectors = 2;
> +		vp_dev->shared_vq_vec = true;
> +		err = pci_alloc_irq_vectors(pdev, nvectors, nvectors,
>  				PCI_IRQ_MSIX);
>  		if (err < 0)
>  			return err;
>  	}
> -	if (err < 0)
> -		return err;
>  
>  	vp_dev->msix_vectors = nvectors;
>  	vp_dev->msix_names = kmalloc_array(nvectors,
> @@ -194,79 +196,99 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
>  	}
>  
>  	/* Set the vector used for configuration */
> -	snprintf(vp_dev->msix_names[0], sizeof(*vp_dev->msix_names),
> +	snprintf(vp_dev->msix_names[msix_vec], sizeof(*vp_dev->msix_names),
>  		 "%s-config", name);
> -	err = request_irq(pci_irq_vector(vp_dev->pci_dev, 0), vp_config_changed,
> -			0, vp_dev->msix_names[0], vp_dev);
> +	err = request_irq(pci_irq_vector(pdev, msix_vec), vp_config_changed, 0,
> +			  vp_dev->msix_names[msix_vec], vp_dev);
>  	if (err)
>  		goto out_free_msix_affinity_masks;
>  
>  	/* Verify we had enough resources to assign the vector */
> -	if (vp_dev->config_vector(vp_dev, 0) == VIRTIO_MSI_NO_VECTOR) {
> +	if (vp_dev->config_vector(vp_dev, msix_vec) == VIRTIO_MSI_NO_VECTOR) {
>  		err = -EBUSY;
>  		goto out_free_config_irq;
>  	}
>  
> -	vp_dev->msix_vector_map = kmalloc_array(nvqs,
> -			sizeof(*vp_dev->msix_vector_map), GFP_KERNEL);
> -	if (!vp_dev->msix_vector_map)
> -		goto out_disable_config_irq;
> -
> -	allocated_vectors = j = 1; /* vector 0 is the config interrupt */
> -	for (i = 0; i < nvqs; ++i) {
> -		if (!names[i]) {
> -			vqs[i] = NULL;
> -			continue;
> -		}
> -
> -		if (callbacks[i])
> -			msix_vec = allocated_vectors;
> -		else
> -			msix_vec = VIRTIO_MSI_NO_VECTOR;
> -
> -		vqs[i] = vp_dev->setup_vq(vp_dev, i, callbacks[i], names[i],
> -				msix_vec);
> -		if (IS_ERR(vqs[i])) {
> -			err = PTR_ERR(vqs[i]);
> -			goto out_remove_vqs;
> +	msix_vec++;
> +
> +	/*
> +	 * Use a different vector for each queue if they are available,
> +	 * else share the same vector for all VQs.
> +	 */
> +	if (vp_dev->shared_vq_vec) {
> +		snprintf(vp_dev->msix_names[msix_vec],
> +			 sizeof(vp_dev->msix_names[msix_vec]),
> +			 "%s-virtqueues", name);
> +		err = request_irq(pci_irq_vector(pdev, msix_vec),
> +				vp_vring_interrupt, 0,
> +				vp_dev->msix_names[msix_vec], vp_dev);
> +		if (err)
> +			goto out_disable_config_irq;
> +
> +		for (i = 0; i < nvqs; ++i) {
> +			if (!names[i]) {
> +				vqs[i] = NULL;
> +				continue;
> +			}
> +
> +			vqs[i] = vp_dev->setup_vq(vp_dev, i, callbacks[i],
> +					names[i], callbacks[i] ?
> +					msix_vec : VIRTIO_MSI_NO_VECTOR);
> +			if (IS_ERR(vqs[i])) {
> +				err = PTR_ERR(vqs[i]);
> +				goto out_remove_vqs;
> +			}
>  		}
> +	} else {
> +		vp_dev->msix_vector_map = kmalloc_array(nvqs,
> +				sizeof(*vp_dev->msix_vector_map), GFP_KERNEL);
> +		if (!vp_dev->msix_vector_map)
> +			goto out_disable_config_irq;
>  
> -		if (msix_vec == VIRTIO_MSI_NO_VECTOR) {
> -			vp_dev->msix_vector_map[i] = VIRTIO_MSI_NO_VECTOR;
> -			continue;
> -		}
> +		for (i = 0; i < nvqs; ++i) {
> +			if (!names[i]) {
> +				vqs[i] = NULL;
> +				continue;
> +			}
>  
> -		snprintf(vp_dev->msix_names[j],
> -			 sizeof(*vp_dev->msix_names), "%s-%s",
> -			 dev_name(&vp_dev->vdev.dev), names[i]);
> -		err = request_irq(pci_irq_vector(vp_dev->pci_dev, msix_vec),
> -				  vring_interrupt, IRQF_SHARED,
> -				  vp_dev->msix_names[j], vqs[i]);
> -		if (err) {
>  			/* don't free this irq on error */
>  			vp_dev->msix_vector_map[i] = VIRTIO_MSI_NO_VECTOR;
> -			goto out_remove_vqs;
> +
> +			vqs[i] = vp_dev->setup_vq(vp_dev, i, callbacks[i],
> +					names[i], callbacks[i] ?
> +					msix_vec : VIRTIO_MSI_NO_VECTOR);
> +			if (IS_ERR(vqs[i])) {
> +				err = PTR_ERR(vqs[i]);
> +				goto out_remove_vqs;
> +			}
> +
> +			if (!callbacks[i])
> +				continue;
> +
> +			snprintf(vp_dev->msix_names[msix_vec],
> +				 sizeof(*vp_dev->msix_names), "%s-%s",
> +				 dev_name(&vp_dev->vdev.dev), names[i]);
> +			err = request_irq(pci_irq_vector(pdev, msix_vec),
> +					  vring_interrupt, IRQF_SHARED,
> +					  vp_dev->msix_names[msix_vec],
> +					  vqs[i]);
> +			if (err)
> +				goto out_remove_vqs;
> +			vp_dev->msix_vector_map[i] = msix_vec++;
>  		}
> -		vp_dev->msix_vector_map[i] = msix_vec;
> -		j++;
> -
> -		/*
> -		 * Use a different vector for each queue if they are available,
> -		 * else share the same vector for all VQs.
> -		 */
> -		if (!shared)
> -			allocated_vectors++;
>  	}
>  
>  	return 0;
>  
>  out_remove_vqs:
>  	vp_remove_vqs(vdev);
> +	if (vp_dev->shared_vq_vec)
> +		free_irq(pci_irq_vector(pdev, 1), vp_dev);
>  	kfree(vp_dev->msix_vector_map);
>  out_disable_config_irq:
>  	vp_dev->config_vector(vp_dev, VIRTIO_MSI_NO_VECTOR);
>  out_free_config_irq:
> -	free_irq(pci_irq_vector(vp_dev->pci_dev, 0), vp_dev);
> +	free_irq(pci_irq_vector(pdev, 0), vp_dev);
>  out_free_msix_affinity_masks:
>  	for (i = 0; i < nvectors; i++) {
>  		if (vp_dev->msix_affinity_masks[i])
> @@ -276,7 +298,7 @@ static int vp_find_vqs_msix(struct virtio_device *vdev, unsigned nvqs,
>  out_free_msix_names:
>  	kfree(vp_dev->msix_names);
>  out_free_irq_vectors:
> -	pci_free_irq_vectors(vp_dev->pci_dev);
> +	pci_free_irq_vectors(pdev);
>  	return err;
>  }
>  
> @@ -346,7 +368,7 @@ int vp_set_vq_affinity(struct virtqueue *vq, int cpu)
>  	if (!vq->callback)
>  		return -EINVAL;
>  
> -	if (vp_dev->pci_dev->msix_enabled) {
> +	if (vp_dev->msix_vector_map) {
>  		int vec = vp_dev->msix_vector_map[vq->index];
>  		struct cpumask *mask = vp_dev->msix_affinity_masks[vec];
>  		unsigned int irq = pci_irq_vector(vp_dev->pci_dev, vec);
> diff --git a/drivers/virtio/virtio_pci_common.h b/drivers/virtio/virtio_pci_common.h
> index ac8c9d788964..d6d7fb99e47f 100644
> --- a/drivers/virtio/virtio_pci_common.h
> +++ b/drivers/virtio/virtio_pci_common.h
> @@ -72,6 +72,7 @@ struct virtio_pci_device {
>  	int msix_vectors;
>  	/* Map of per-VQ MSI-X vectors, may be NULL */
>  	unsigned *msix_vector_map;
> +	bool shared_vq_vec;


Pls add documentation. In fact I'd prefer a counter of vectors
used than a separate mode.
>  
>  	struct virtqueue *(*setup_vq)(struct virtio_pci_device *vp_dev,
>  				      unsigned idx,
> -- 
> 2.11.0

[toc] | [prev] | [next] | [standalone]

#1616621 — Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use shared interrupts for virtqueues")

On Mon, Apr 03, 2017 at 07:14:22PM +0300, Michael S. Tsirkin wrote:
> On Mon, Apr 03, 2017 at 04:18:23PM +0200, Christoph Hellwig wrote:
> > Mike,
> > 
> > can you try the patch below?
> > 
> > ---
> > >From fe41a30b54878cc631623b7511267125e0da4b15 Mon Sep 17 00:00:00 2001
> > From: Christoph Hellwig <hch@lst.de>
> > Date: Mon, 3 Apr 2017 14:51:35 +0200
> > Subject: virtio_pci: don't use shared irq for virtqueues
> > 
> > Reimplement the shared irq feature manually, as we might have a larger
> > number of virtqueues than the core shared interrupt code can handle
> > in threaded interrupt mode.
> > 
> > Signed-off-by: Christoph Hellwig <hch@lst.de>
> > ---
> >  drivers/virtio/virtio_pci_common.c | 142 +++++++++++++++++++++----------------
> >  drivers/virtio/virtio_pci_common.h |   1 +
> >  2 files changed, 83 insertions(+), 60 deletions(-)
> 
> Well the original patch this is trying to fix is
> 07ec51480b5eb1233f8c1b0f5d7a7c8d1247c507 which dropped just 40 lines
> with documentation. It did this by re-using error handling to switch
> from per-vq to non-per-vq mode. Now this has separate flows for errors
> and per-vq non-per-vq switch and (I think, as a result) is adding 140
> lines which doesn't make me very happy.

The above adds 23 lines.  We could entangle both loops again, but I'm
not sure it's going to buy us much.

[toc] | [prev] | [next] | [standalone]

#1615453

On Mon, 2017-04-03 at 16:18 +0200, Christoph Hellwig wrote:
> Mike,
> 
> can you try the patch below?

No more spinning kworker woes, but I still have a warning on hibernate,
threadirqs invariant.  I'm also seeing intermittent post hibernate hang
funnies in virgin source +- this patch, and without threadirqs.

[  110.223953] WARNING: CPU: 5 PID: 452 at drivers/pci/msi.c:1261 pci_irq_vector+0xb1/0xe0

	-Mike

[toc] | [prev] | [next] | [standalone]

#1615457 — Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use shared interrupts for virtqueues")

On Mon, Apr 03, 2017 at 07:56:32PM +0200, Mike Galbraith wrote:
> On Mon, 2017-04-03 at 16:18 +0200, Christoph Hellwig wrote:
> > Mike,
> > 
> > can you try the patch below?
> 
> No more spinning kworker woes, but I still have a warning on hibernate,
> threadirqs invariant.  I'm also seeing intermittent post hibernate hang
> funnies in virgin source +- this patch, and without threadirqs.
> 
> [  110.223953] WARNING: CPU: 5 PID: 452 at drivers/pci/msi.c:1261 pci_irq_vector+0xb1/0xe0
> 
> 	-Mike

I just sent a patch fixing that.
However I think we want to print a message when MSI fails to work so we
know guest is falling back on legacy interrupts.

-- 
MST

[toc] | [prev] | [next] | [standalone]

#1615695

On Mon, 2017-04-03 at 21:11 +0300, Michael S. Tsirkin wrote:
> On Mon, Apr 03, 2017 at 07:56:32PM +0200, Mike Galbraith wrote:
> > On Mon, 2017-04-03 at 16:18 +0200, Christoph Hellwig wrote:
> > > Mike,
> > > 
> > > can you try the patch below?
> > 
> > No more spinning kworker woes, but I still have a warning on hibernate,
> > threadirqs invariant.  I'm also seeing intermittent post hibernate hang
> > funnies in virgin source +- this patch, and without threadirqs.
> > 
> > [  110.223953] WARNING: CPU: 5 PID: 452 at drivers/pci/msi.c:1261 pci_irq_vector+0xb1/0xe0
> > 
> > 	> > -Mike
> 
> I just sent a patch fixing that.
> However I think we want to print a message when MSI fails to work so we
> know guest is falling back on legacy interrupts.

The warning persists.

[  137.656423] WARNING: CPU: 1 PID: 535 at drivers/pci/msi.c:1261 pci_irq_vector+0xb1/0xe0

WRT the post hibernate hang business, that is apparently not part of
the 4.11 woes (at least not solely), as 4.10.8 did not survive a 10
hibernate cycle loop.  RT is better at reproducing trouble (shrug, it
frequently is), but it matters not whether I'm running 4.10, master or
master-rt, they will all hang.

WRT gripe, I wedged virtio_pci-fix-msix-vector-tracking-on-cleanup in
on top, but it wasn't impressed.

	-Mike

[toc] | [prev] | [next] | [standalone]

#1616031 — Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use shared interrupts for virtqueues")

On Tue, Apr 04, 2017 at 06:02:52AM +0200, Mike Galbraith wrote:
> On Mon, 2017-04-03 at 21:11 +0300, Michael S. Tsirkin wrote:
> > On Mon, Apr 03, 2017 at 07:56:32PM +0200, Mike Galbraith wrote:
> > > On Mon, 2017-04-03 at 16:18 +0200, Christoph Hellwig wrote:
> > > > Mike,
> > > > 
> > > > can you try the patch below?
> > > 
> > > No more spinning kworker woes, but I still have a warning on hibernate,
> > > threadirqs invariant.  I'm also seeing intermittent post hibernate hang
> > > funnies in virgin source +- this patch, and without threadirqs.
> > > 
> > > [  110.223953] WARNING: CPU: 5 PID: 452 at drivers/pci/msi.c:1261 pci_irq_vector+0xb1/0xe0
> > > 
> > > 	> > -Mike
> > 
> > I just sent a patch fixing that.
> > However I think we want to print a message when MSI fails to work so we
> > know guest is falling back on legacy interrupts.
> 
> The warning persists.
> 
> [  137.656423] WARNING: CPU: 1 PID: 535 at drivers/pci/msi.c:1261 pci_irq_vector+0xb1/0xe0

Can you post the rest of the backtrace? Is it still in the console?

> WRT the post hibernate hang business, that is apparently not part of
> the 4.11 woes (at least not solely), as 4.10.8 did not survive a 10
> hibernate cycle loop.  RT is better at reproducing trouble (shrug, it
> frequently is), but it matters not whether I'm running 4.10, master or
> master-rt, they will all hang.
> 
> WRT gripe, I wedged virtio_pci-fix-msix-vector-tracking-on-cleanup in
> on top, but it wasn't impressed.
> 
> 	-Mike

[toc] | [prev] | [next] | [standalone]

#1616063

On Tue, 2017-04-04 at 16:38 +0300, Michael S. Tsirkin wrote:
> On Tue, Apr 04, 2017 at 06:02:52AM +0200, Mike Galbraith wrote:
> > On Mon, 2017-04-03 at 21:11 +0300, Michael S. Tsirkin wrote:
> > > On Mon, Apr 03, 2017 at 07:56:32PM +0200, Mike Galbraith wrote:
> > > > On Mon, 2017-04-03 at 16:18 +0200, Christoph Hellwig wrote:
> > > > > Mike,
> > > > > 
> > > > > can you try the patch below?
> > > > 
> > > > No more spinning kworker woes, but I still have a warning on
> > > > hibernate,
> > > > threadirqs invariant.  I'm also seeing intermittent post
> > > > hibernate hang
> > > > funnies in virgin source +- this patch, and without threadirqs.
> > > > 
> > > > [  110.223953] WARNING: CPU: 5 PID: 452 at
> > > > drivers/pci/msi.c:1261 pci_irq_vector+0xb1/0xe0
> > > > 
> > > > 	> > -Mike
> > > 
> > > I just sent a patch fixing that.
> > > However I think we want to print a message when MSI fails to work
> > > so we
> > > know guest is falling back on legacy interrupts.
> > 
> > The warning persists.
> > 
> > [  137.656423] WARNING: CPU: 1 PID: 535 at drivers/pci/msi.c:1261
> > pci_irq_vector+0xb1/0xe0
> 
> Can you post the rest of the backtrace? Is it still in the console?

This is from a dump of post hibernate loop dying vbox I captured and
squirreled away, so pid is different.  I'm not absolutely certain that
I didn't have my local patch set re-applied when I did this, so I'll
rebuild in the a.m..  My stuff is unrelated, so this should be fine.

[  328.475988] ------------[ cut here ]------------
[  328.476002] WARNING: CPU: 6 PID: 313 at drivers/pci/msi.c:1261 pci_irq_vector+0xb1/0xe0
[  328.476003] Modules linked in: fuse(E) ebtable_filter(E) ebtables(E) nf_log_ipv6(E) xt_pkttype(E) nf_log_ipv4(E) nf_log_common(E) xt_LOG(E) xt_limit(E) rpcsec_gss_krb5(E) nfsv4(E) dns_resolver(E) nfs(E) fscache(E) af_packet(E) iscsi_ibft(E) iscsi_boot_sysfs(E) ip6t_REJECT(E) xt_tcpudp(E) nf_conntrack_ipv6(E) nf_defrag_ipv6(E) ip6table_raw(E) ipt_REJECT(E) iptable_raw(E) xt_CT(E) iptable_filter(E) ip6table_mangle(E) nf_conntrack_netbios_ns(E) nf_conntrack_broadcast(E) nf_conntrack_ipv4(E) nf_defrag_ipv4(E) ip_tables(E) xt_conntrack(E) nf_conntrack(E) libcrc32c(E) ip6table_filter(E) ip6_tables(E) x_tables(E) snd_hda_codec_generic(E) snd_hda_intel(E) snd_hda_codec(E) snd_hda_core(E) joydev(E) snd_hwdep(E) snd_pcm(E) snd_timer(E) snd(E) 8139too(E) soundcore(E) i2c_piix4(E) virtio_balloon(E) crct10dif_pclmul(E)
[  328.476019]  crc32_pclmul(E) ppdev(E) ghash_clmulni_intel(E) parport_pc(E) acpi_cpufreq(E) pcbc(E) button(E) parport(E) aesni_intel(E) aes_x86_64(E) serio_raw(E) pcspkr(E) crypto_simd(E) glue_helper(E) cryptd(E) nfsd(E) auth_rpcgss(E) nfs_acl(E) lockd(E) dm_mod(E) grace(E) sunrpc(E) ext4(E) crc16(E) jbd2(E) mbcache(E) hid_generic(E) usbhid(E) sr_mod(E) cdrom(E) ata_generic(E) virtio_blk(E) virtio_rng(E) virtio_console(E) ata_piix(E) qxl(E) drm_kms_helper(E) syscopyarea(E) uhci_hcd(E) ehci_pci(E) sysfillrect(E) sysimgblt(E) ahci(E) fb_sys_fops(E) ehci_hcd(E) libahci(E) crc32c_intel(E) ttm(E) virtio_pci(E) virtio_ring(E) 8139cp(E) virtio(E) usbcore(E) floppy(E) mii(E) drm(E) libata(E) sg(E) scsi_mod(E) autofs4(E)
[  328.476037] CPU: 6 PID: 313 Comm: kworker/u16:2 Tainted: G            E   4.11.0-default #20
[  328.476038] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.8.1-0-g4adadbd-20161202_174313-build11a 04/01/2014
[  328.476041] Workqueue: events_unbound async_run_entry_fn
[  328.476042] Call Trace:
[  328.476056]  ? dump_stack+0x5c/0x85
[  328.476058]  ? __warn+0xc4/0xe0
[  328.476060]  ? pci_pm_poweroff+0xf0/0xf0
[  328.476062]  ? pci_irq_vector+0xb1/0xe0
[  328.476064]  ? vp_del_vqs+0xcb/0x120 [virtio_pci]
[  328.476066]  ? remove_common+0x60/0x80 [virtio_rng]
[  328.476067]  ? virtrng_freeze+0xa/0x10 [virtio_rng]
[  328.476068]  ? virtio_pci_freeze+0x19/0x40 [virtio_pci]
[  328.476069]  ? pci_pm_freeze+0x59/0xe0
[  328.476070]  ? dpm_run_callback+0x4d/0x170
[  328.476071]  ? __device_suspend+0x11f/0x3b0
[  328.476072]  ? pm_dev_dbg+0x70/0x70
[  328.476072]  ? async_suspend+0x1a/0x90
[  328.476082]  ? async_run_entry_fn+0x34/0x160
[  328.476083]  ? process_one_work+0x164/0x430
[  328.476084]  ? worker_thread+0x135/0x4d0
[  328.476085]  ? kthread+0xff/0x140
[  328.476086]  ? rescuer_thread+0x3c0/0x3c0
[  328.476087]  ? kthread_park+0x80/0x80
[  328.476088]  ? do_group_exit+0x39/0xa0
[  328.476090]  ? ret_from_fork+0x26/0x40
[  328.476091] ---[ end trace a045c2118936902f ]---

[toc] | [prev] | [next] | [standalone]

#1616068 — Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use shared interrupts for virtqueues")

On Tue, Apr 04, 2017 at 04:18:02PM +0200, Mike Galbraith wrote:
> On Tue, 2017-04-04 at 16:38 +0300, Michael S. Tsirkin wrote:
> > On Tue, Apr 04, 2017 at 06:02:52AM +0200, Mike Galbraith wrote:
> > > On Mon, 2017-04-03 at 21:11 +0300, Michael S. Tsirkin wrote:
> > > > On Mon, Apr 03, 2017 at 07:56:32PM +0200, Mike Galbraith wrote:
> > > > > On Mon, 2017-04-03 at 16:18 +0200, Christoph Hellwig wrote:
> > > > > > Mike,
> > > > > > 
> > > > > > can you try the patch below?
> > > > > 
> > > > > No more spinning kworker woes, but I still have a warning on
> > > > > hibernate,
> > > > > threadirqs invariant.  I'm also seeing intermittent post
> > > > > hibernate hang
> > > > > funnies in virgin source +- this patch, and without threadirqs.
> > > > > 
> > > > > [  110.223953] WARNING: CPU: 5 PID: 452 at
> > > > > drivers/pci/msi.c:1261 pci_irq_vector+0xb1/0xe0
> > > > > 
> > > > > 	> > -Mike
> > > > 
> > > > I just sent a patch fixing that.
> > > > However I think we want to print a message when MSI fails to work
> > > > so we
> > > > know guest is falling back on legacy interrupts.
> > > 
> > > The warning persists.
> > > 
> > > [  137.656423] WARNING: CPU: 1 PID: 535 at drivers/pci/msi.c:1261
> > > pci_irq_vector+0xb1/0xe0
> > 
> > Can you post the rest of the backtrace? Is it still in the console?
> 
> This is from a dump of post hibernate loop dying vbox I captured and
> squirreled away, so pid is different.  I'm not absolutely certain that
> I didn't have my local patch set re-applied when I did this, so I'll
> rebuild in the a.m..  My stuff is unrelated, so this should be fine.
> 
> [  328.475988] ------------[ cut here ]------------
> [  328.476002] WARNING: CPU: 6 PID: 313 at drivers/pci/msi.c:1261 pci_irq_vector+0xb1/0xe0
> [  328.476003] Modules linked in: fuse(E) ebtable_filter(E) ebtables(E) nf_log_ipv6(E) xt_pkttype(E) nf_log_ipv4(E) nf_log_common(E) xt_LOG(E) xt_limit(E) rpcsec_gss_krb5(E) nfsv4(E) dns_resolver(E) nfs(E) fscache(E) af_packet(E) iscsi_ibft(E) iscsi_boot_sysfs(E) ip6t_REJECT(E) xt_tcpudp(E) nf_conntrack_ipv6(E) nf_defrag_ipv6(E) ip6table_raw(E) ipt_REJECT(E) iptable_raw(E) xt_CT(E) iptable_filter(E) ip6table_mangle(E) nf_conntrack_netbios_ns(E) nf_conntrack_broadcast(E) nf_conntrack_ipv4(E) nf_defrag_ipv4(E) ip_tables(E) xt_conntrack(E) nf_conntrack(E) libcrc32c(E) ip6table_filter(E) ip6_tables(E) x_tables(E) snd_hda_codec_generic(E) snd_hda_intel(E) snd_hda_codec(E) snd_hda_core(E) joydev(E) snd_hwdep(E) snd_pcm(E) snd_timer(E) snd(E) 8139too(E) soundcore(E) i2c_piix4(E) virtio_balloon(E) crct10dif_pclmul(E)
> [  328.476019]  crc32_pclmul(E) ppdev(E) ghash_clmulni_intel(E) parport_pc(E) acpi_cpufreq(E) pcbc(E) button(E) parport(E) aesni_intel(E) aes_x86_64(E) serio_raw(E) pcspkr(E) crypto_simd(E) glue_helper(E) cryptd(E) nfsd(E) auth_rpcgss(E) nfs_acl(E) lockd(E) dm_mod(E) grace(E) sunrpc(E) ext4(E) crc16(E) jbd2(E) mbcache(E) hid_generic(E) usbhid(E) sr_mod(E) cdrom(E) ata_generic(E) virtio_blk(E) virtio_rng(E) virtio_console(E) ata_piix(E) qxl(E) drm_kms_helper(E) syscopyarea(E) uhci_hcd(E) ehci_pci(E) sysfillrect(E) sysimgblt(E) ahci(E) fb_sys_fops(E) ehci_hcd(E) libahci(E) crc32c_intel(E) ttm(E) virtio_pci(E) virtio_ring(E) 8139cp(E) virtio(E) usbcore(E) floppy(E) mii(E) drm(E) libata(E) sg(E) scsi_mod(E) autofs4(E)
> [  328.476037] CPU: 6 PID: 313 Comm: kworker/u16:2 Tainted: G            E   4.11.0-default #20
> [  328.476038] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.8.1-0-g4adadbd-20161202_174313-build11a 04/01/2014
> [  328.476041] Workqueue: events_unbound async_run_entry_fn
> [  328.476042] Call Trace:
> [  328.476056]  ? dump_stack+0x5c/0x85
> [  328.476058]  ? __warn+0xc4/0xe0
> [  328.476060]  ? pci_pm_poweroff+0xf0/0xf0
> [  328.476062]  ? pci_irq_vector+0xb1/0xe0
> [  328.476064]  ? vp_del_vqs+0xcb/0x120 [virtio_pci]
> [  328.476066]  ? remove_common+0x60/0x80 [virtio_rng]
> [  328.476067]  ? virtrng_freeze+0xa/0x10 [virtio_rng]
> [  328.476068]  ? virtio_pci_freeze+0x19/0x40 [virtio_pci]
> [  328.476069]  ? pci_pm_freeze+0x59/0xe0
> [  328.476070]  ? dpm_run_callback+0x4d/0x170
> [  328.476071]  ? __device_suspend+0x11f/0x3b0
> [  328.476072]  ? pm_dev_dbg+0x70/0x70
> [  328.476072]  ? async_suspend+0x1a/0x90
> [  328.476082]  ? async_run_entry_fn+0x34/0x160
> [  328.476083]  ? process_one_work+0x164/0x430
> [  328.476084]  ? worker_thread+0x135/0x4d0
> [  328.476085]  ? kthread+0xff/0x140
> [  328.476086]  ? rescuer_thread+0x3c0/0x3c0
> [  328.476087]  ? kthread_park+0x80/0x80
> [  328.476088]  ? do_group_exit+0x39/0xa0
> [  328.476090]  ? ret_from_fork+0x26/0x40
> [  328.476091] ---[ end trace a045c2118936902f ]---

Interesting, it's rng this time. I'll try that.

-- 
MST

[toc] | [prev] | [next] | [standalone]

#1616121 — Re: Random guest crashes since 5c34d002dcc7 ("virtio_pci: use shared interrupts for virtqueues")

On Tue, Apr 04, 2017 at 04:18:02PM +0200, Mike Galbraith wrote:
> On Tue, 2017-04-04 at 16:38 +0300, Michael S. Tsirkin wrote:
> > On Tue, Apr 04, 2017 at 06:02:52AM +0200, Mike Galbraith wrote:
> > > On Mon, 2017-04-03 at 21:11 +0300, Michael S. Tsirkin wrote:
> > > > On Mon, Apr 03, 2017 at 07:56:32PM +0200, Mike Galbraith wrote:
> > > > > On Mon, 2017-04-03 at 16:18 +0200, Christoph Hellwig wrote:
> > > > > > Mike,
> > > > > > 
> > > > > > can you try the patch below?
> > > > > 
> > > > > No more spinning kworker woes, but I still have a warning on
> > > > > hibernate,
> > > > > threadirqs invariant.  I'm also seeing intermittent post
> > > > > hibernate hang
> > > > > funnies in virgin source +- this patch, and without threadirqs.
> > > > > 
> > > > > [  110.223953] WARNING: CPU: 5 PID: 452 at
> > > > > drivers/pci/msi.c:1261 pci_irq_vector+0xb1/0xe0
> > > > > 
> > > > > 	> > -Mike
> > > > 
> > > > I just sent a patch fixing that.
> > > > However I think we want to print a message when MSI fails to work
> > > > so we
> > > > know guest is falling back on legacy interrupts.
> > > 
> > > The warning persists.
> > > 
> > > [  137.656423] WARNING: CPU: 1 PID: 535 at drivers/pci/msi.c:1261
> > > pci_irq_vector+0xb1/0xe0
> > 
> > Can you post the rest of the backtrace? Is it still in the console?
> 
> This is from a dump of post hibernate loop dying vbox I captured and
> squirreled away, so pid is different.  I'm not absolutely certain that
> I didn't have my local patch set re-applied when I did this, so I'll
> rebuild in the a.m..  My stuff is unrelated, so this should be fine.
> 
> [  328.475988] ------------[ cut here ]------------
> [  328.476002] WARNING: CPU: 6 PID: 313 at drivers/pci/msi.c:1261 pci_irq_vector+0xb1/0xe0
> [  328.476003] Modules linked in: fuse(E) ebtable_filter(E) ebtables(E) nf_log_ipv6(E) xt_pkttype(E) nf_log_ipv4(E) nf_log_common(E) xt_LOG(E) xt_limit(E) rpcsec_gss_krb5(E) nfsv4(E) dns_resolver(E) nfs(E) fscache(E) af_packet(E) iscsi_ibft(E) iscsi_boot_sysfs(E) ip6t_REJECT(E) xt_tcpudp(E) nf_conntrack_ipv6(E) nf_defrag_ipv6(E) ip6table_raw(E) ipt_REJECT(E) iptable_raw(E) xt_CT(E) iptable_filter(E) ip6table_mangle(E) nf_conntrack_netbios_ns(E) nf_conntrack_broadcast(E) nf_conntrack_ipv4(E) nf_defrag_ipv4(E) ip_tables(E) xt_conntrack(E) nf_conntrack(E) libcrc32c(E) ip6table_filter(E) ip6_tables(E) x_tables(E) snd_hda_codec_generic(E) snd_hda_intel(E) snd_hda_codec(E) snd_hda_core(E) joydev(E) snd_hwdep(E) snd_pcm(E) snd_timer(E) snd(E) 8139too(E) soundcore(E) i2c_piix4(E) virtio_balloon(E) crct10dif_pclmul(E)
> [  328.476019]  crc32_pclmul(E) ppdev(E) ghash_clmulni_intel(E) parport_pc(E) acpi_cpufreq(E) pcbc(E) button(E) parport(E) aesni_intel(E) aes_x86_64(E) serio_raw(E) pcspkr(E) crypto_simd(E) glue_helper(E) cryptd(E) nfsd(E) auth_rpcgss(E) nfs_acl(E) lockd(E) dm_mod(E) grace(E) sunrpc(E) ext4(E) crc16(E) jbd2(E) mbcache(E) hid_generic(E) usbhid(E) sr_mod(E) cdrom(E) ata_generic(E) virtio_blk(E) virtio_rng(E) virtio_console(E) ata_piix(E) qxl(E) drm_kms_helper(E) syscopyarea(E) uhci_hcd(E) ehci_pci(E) sysfillrect(E) sysimgblt(E) ahci(E) fb_sys_fops(E) ehci_hcd(E) libahci(E) crc32c_intel(E) ttm(E) virtio_pci(E) virtio_ring(E) 8139cp(E) virtio(E) usbcore(E) floppy(E) mii(E) drm(E) libata(E) sg(E) scsi_mod(E) autofs4(E)
> [  328.476037] CPU: 6 PID: 313 Comm: kworker/u16:2 Tainted: G            E   4.11.0-default #20
> [  328.476038] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.8.1-0-g4adadbd-20161202_174313-build11a 04/01/2014
> [  328.476041] Workqueue: events_unbound async_run_entry_fn
> [  328.476042] Call Trace:
> [  328.476056]  ? dump_stack+0x5c/0x85
> [  328.476058]  ? __warn+0xc4/0xe0
> [  328.476060]  ? pci_pm_poweroff+0xf0/0xf0
> [  328.476062]  ? pci_irq_vector+0xb1/0xe0
> [  328.476064]  ? vp_del_vqs+0xcb/0x120 [virtio_pci]
> [  328.476066]  ? remove_common+0x60/0x80 [virtio_rng]
> [  328.476067]  ? virtrng_freeze+0xa/0x10 [virtio_rng]
> [  328.476068]  ? virtio_pci_freeze+0x19/0x40 [virtio_pci]
> [  328.476069]  ? pci_pm_freeze+0x59/0xe0
> [  328.476070]  ? dpm_run_callback+0x4d/0x170
> [  328.476071]  ? __device_suspend+0x11f/0x3b0
> [  328.476072]  ? pm_dev_dbg+0x70/0x70
> [  328.476072]  ? async_suspend+0x1a/0x90
> [  328.476082]  ? async_run_entry_fn+0x34/0x160
> [  328.476083]  ? process_one_work+0x164/0x430
> [  328.476084]  ? worker_thread+0x135/0x4d0
> [  328.476085]  ? kthread+0xff/0x140
> [  328.476086]  ? rescuer_thread+0x3c0/0x3c0
> [  328.476087]  ? kthread_park+0x80/0x80
> [  328.476088]  ? do_group_exit+0x39/0xa0
> [  328.476090]  ? ret_from_fork+0x26/0x40
> [  328.476091] ---[ end trace a045c2118936902f ]---


I couldn't reproduce it - let's make sure we are using the
same tree. Could you pls try

git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next 

It's currently at cc79d42a7d7e57ff64f406a1fd3740afebac0b44
-- 
MST

[toc] | [prev] | [next] | [standalone]

#1616260

On Tue, 2017-04-04 at 18:30 +0300, Michael S. Tsirkin wrote:

> I couldn't reproduce it - let's make sure we are using the
> same tree. Could you pls try
> 
> git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next 
> 
> It's currently at cc79d42a7d7e57ff64f406a1fd3740afebac0b44

Things that make ya go hmm...

[   87.940161] ------------[ cut here ]------------
[   87.940180] WARNING: CPU: 0 PID: 97 at drivers/pci/msi.c:1251 pci_irq_vector+0xcb/0xe0
[   87.940181] Modules linked in: dm_mod(E) fuse(E) ebtable_filter(E) ebtables(E) rpcsec_gss_krb5(E) nfsv4(E) dns_resolver(E) nfs(E) fscache(E) nf_log_ipv6(E) xt_pkttype(E) nf_log_ipv4(E) nf_log_common(E) xt_LOG(E) xt_limit(E) af_packet(E) iscsi_ibft(E) iscsi_boot_sysfs(E) ip6t_REJECT(E) xt_tcpudp(E) nf_conntrack_ipv6(E) nf_defrag_ipv6(E) ip6table_raw(E) ipt_REJECT(E) iptable_raw(E) xt_CT(E) iptable_filter(E) ip6table_mangle(E) nf_conntrack_netbios_ns(E) nf_conntrack_broadcast(E) nf_conntrack_ipv4(E) nf_defrag_ipv4(E) ip_tables(E) xt_conntrack(E) nf_conntrack(E) libcrc32c(E) ip6table_filter(E) ip6_tables(E) x_tables(E) snd_hda_codec_generic(E) snd_hda_intel(E) snd_hda_codec(E) joydev(E) snd_hda_core(E) snd_hwdep(E) snd_pcm(E) snd_timer(E) snd(E) 8139too(E) ppdev(E) soundcore(E) parport_pc(E) i2c_piix4(E)
[   87.940206]  parport(E) virtio_balloon(E) crct10dif_pclmul(E) crc32_pclmul(E) crc32c_intel(E) ghash_clmulni_intel(E) serio_raw(E) acpi_cpufreq(E) pcbc(E) button(E) aesni_intel(E) pcspkr(E) aes_x86_64(E) crypto_simd(E) glue_helper(E) cryptd(E) nfsd(E) auth_rpcgss(E) nfs_acl(E) lockd(E) grace(E) sunrpc(E) ext4(E) crc16(E) jbd2(E) mbcache(E) hid_generic(E) usbhid(E) ata_generic(E) ata_piix(E) sr_mod(E) cdrom(E) virtio_blk(E) virtio_rng(E) virtio_console(E) qxl(E) drm_kms_helper(E) syscopyarea(E) sysfillrect(E) sysimgblt(E) fb_sys_fops(E) ehci_pci(E) ttm(E) uhci_hcd(E) ehci_hcd(E) floppy(E) ahci(E) libahci(E) virtio_pci(E) drm(E) virtio_ring(E) virtio(E) usbcore(E) libata(E) 8139cp(E) mii(E) sg(E) scsi_mod(E) autofs4(E)
[   87.940233] CPU: 0 PID: 97 Comm: kworker/u16:1 Tainted: G            E   4.11.0-default #1
[   87.940234] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.8.1-0-g4adadbd-20161202_174313-build11a 04/01/2014
[   87.940240] Workqueue: events_unbound async_run_entry_fn
[   87.940241] Call Trace:
[   87.940246]  ? dump_stack+0x5c/0x85
[   87.940255]  ? __warn+0xc4/0xe0
[   87.940258]  ? pci_pm_poweroff+0xf0/0xf0
[   87.940269]  ? pci_irq_vector+0xcb/0xe0
[   87.940272]  ? vp_synchronize_vectors+0x3e/0x50 [virtio_pci]
[   87.940275]  ? virtcons_freeze+0x1a/0xd0 [virtio_console]
[   87.940276]  ? virtio_pci_freeze+0x19/0x40 [virtio_pci]
[   87.940277]  ? pci_pm_freeze+0x59/0xe0
[   87.940281]  ? dpm_run_callback+0x4d/0x170
[   87.940283]  ? __device_suspend+0x11f/0x3b0
[   87.940283]  ? pm_dev_dbg+0x70/0x70
[   87.940284]  ? async_suspend+0x1a/0x90
[   87.940286]  ? async_run_entry_fn+0x34/0x160
[   87.940287]  ? process_one_work+0x164/0x430
[   87.940288]  ? worker_thread+0x135/0x4d0
[   87.940290]  ? kthread+0xff/0x140
[   87.940291]  ? rescuer_thread+0x3c0/0x3c0
[   87.940292]  ? kthread_park+0x80/0x80
[   87.940293]  ? kthread_park+0x80/0x80
[   87.940299]  ? ret_from_fork+0x26/0x40
[   87.940300] ---[ end trace 5d65fe0efc4b61d7 ]---

[toc] | [prev] | [next] | [standalone]

#1616271

On Tue, 2017-04-04 at 19:40 +0200, Mike Galbraith wrote:
> On Tue, 2017-04-04 at 18:30 +0300, Michael S. Tsirkin wrote:
> 
> > I couldn't reproduce it - let's make sure we are using the
> > same tree. Could you pls try
> > 
> > git://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux
> > -next 
> > 
> > It's currently at cc79d42a7d7e57ff64f406a1fd3740afebac0b44
> 
> Things that make ya go hmm...

Making double sure we're on the same page...

git@homer:..git/vhost> git branch
* linux-next
  master
git@homer:..git/vhost> git describe
warning: tag 'for_linus' is really 'tags_for_linus' here
for_linus-220128-gcc79d42a7d7e
git@homer:..git/vhost> git status
On branch linux-next
Your branch is up-to-date with 'origin/linux-next'.
Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git checkout -- <file>..." to discard changes in working directory)

        modified:   Makefile
        modified:   scripts/setlocalversion

no changes added to commit (use "git add" and/or "git commit -a")
git@homer:..git/vhost>

Modifications are me whacking '+' sign and -rc5.. I don't do those.

[toc] | [prev] | [next] | [standalone]

Page 1 of 3  [1] 2 3  Next page →

Back to top | Article view | linux.kernel

csiph-web