Re: [External Email] Re: [PATCH 1/2] virtio-mmio: Process vrings more proactively

[Fam Zheng]

On 7/19/19 11:17 PM, Michael S. Tsirkin wrote:
> On Fri, Jul 19, 2019 at 09:31:34PM +0800, Fei Li wrote:
> > From: Fam Zheng <zhengfeiran@xxxxxxxxxxxxx>
> >
> > This allows the backend to _not_ trap mmio read of the status register
> > after injecting IRQ in the data path, which can improve the performance
> > significantly by avoiding a vmexit for each interrupt.
> >
> > More importantly it also makes it possible for Firecracker to hook up
> > virtio-mmio with vhost-net, in which case there isn't a way to implement
> > proper status register handling.
> >
> > For a complete backend that does set either INT_CONFIG bit or INT_VRING
> > bit upon generating irq, what happens hasn't changed.
> >
> > Signed-off-by: Fam Zheng <zhengfeiran@xxxxxxxxxxxxx>
> This has a side effect of skipping vring callbacks
> if they trigger at the same time with a config
> interrupt.
> I don't see why this is safe.

Good point! I think the block can be moved out from the else block and 
run unconditionally then.

Fam


> > ---
> >   drivers/virtio/virtio_mmio.c | 4 +---
> >   1 file changed, 1 insertion(+), 3 deletions(-)
> >
> > diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c
> > index e09edb5c5e06..9b42502b2204 100644
> > --- a/drivers/virtio/virtio_mmio.c
> > +++ b/drivers/virtio/virtio_mmio.c
> > @@ -295,9 +295,7 @@ static irqreturn_t vm_interrupt(int irq, void *opaque)
> >   	if (unlikely(status & VIRTIO_MMIO_INT_CONFIG)) {
> >   		virtio_config_changed(&vm_dev->vdev);
> >   		ret = IRQ_HANDLED;
> > -	}
> > -
> > -	if (likely(status & VIRTIO_MMIO_INT_VRING)) {
> > +	} else {
> >   		spin_lock_irqsave(&vm_dev->lock, flags);
> >   		list_for_each_entry(info, &vm_dev->virtqueues, node)
> >   			ret |= vring_interrupt(irq, info->vq);
> > --
> > 2.11.0