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

[Michael S. Tsirkin]

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@xxxxxxxxxx>
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@xxxxxxxxxx>

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;
 }