Re: [Qemu-devel] [PATCH 2/3] virtio: access ISR atomically

[Michael S. Tsirkin]

On Thu, Nov 17, 2016 at 02:49:58PM -0500, Paolo Bonzini wrote:
> 
> 
> ----- Original Message -----
> > From: "Michael S. Tsirkin" <address@hidden>
> > To: "Paolo Bonzini" <address@hidden>
> > Cc: address@hidden, "alex williamson" <address@hidden>, address@hidden, 
> > address@hidden
> > Sent: Thursday, November 17, 2016 6:55:50 PM
> > Subject: Re: [PATCH 2/3] virtio: access ISR atomically
> > 
> > On Wed, Nov 16, 2016 at 07:05:50PM +0100, Paolo Bonzini wrote:
> > > @@ -1318,10 +1318,18 @@ void virtio_del_queue(VirtIODevice *vdev, int n)
> > >      vdev->vq[n].vring.num_default = 0;
> > >  }
> > >  
> > > +static void virtio_set_isr(VirtIODevice *vdev, int value)
> > > +{
> > > +    uint8_t old = atomic_read(&vdev->isr);
> > > +    if ((old & value) != value) {
> > > +        atomic_or(&vdev->isr, value);
> > > +    }
> > > +}
> > > +
> > 
> > Paolo, can you pls comment on why is it done like this,
> > as opposed to a single atomic_or?
> 
> To avoid cacheline bouncing in the common case where MSI is active
> but the host doesn't read ISR.
> 
> Paolo

You mean the guest does not read ISR?
And so this helps keep the field read-mostly for all CPUs,
sharing the cache line.
OK but this is worth documenting.

-- 
MST