Re: [dpdk-dev] [PATCH v2 02/24] virtio: Use weaker barriers

["Ouyang, Changchun" <changchun.ouyang@intel.com>]

> -----Original Message-----
> From: Xie, Huawei
> Sent: Wednesday, January 28, 2015 12:16 AM
> To: Stephen Hemminger
> Cc: Ouyang, Changchun; dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v2 02/24] virtio: Use weaker barriers
> 
> 
> 
> > -----Original Message-----
> > From: Stephen Hemminger [mailto:stephen@networkplumber.org]
> > Sent: Tuesday, January 27, 2015 5:59 PM
> > To: Xie, Huawei
> > Cc: Ouyang, Changchun; dev@dpdk.org
> > Subject: Re: [dpdk-dev] [PATCH v2 02/24] virtio: Use weaker barriers
> >
> >
> > > I recall our original code is virtio_wmb().
> > > Use store fence to ensure all updates to entries before updating the
> index.
> > > Why do we need virtio_rmb() here and add virtio_wmb after
> > vq_update_avail_idx()?
> >
> > Store fence is unnecessary, Intel CPU's are cache coherent, please
> > read the virtio Linux ring header file for explanation. A full fence
> > WMB is more expensive and causes CPU stall
> >
> 
> 
> I mean virtio_wmb rather than virtio_rmb should be used here, and both of
> them are defined as compiler barrier.
> 
> The following code is linux virtio driver for adding buffer to vring.
> /* Put entry in available array (but don't update avail->idx until they
> 	 * do sync). */
> 	avail = (vq->vring.avail->idx & (vq->vring.num-1));
> 	vq->vring.avail->ring[avail] = head;
> 
> 	/* Descriptors and available array need to be set before we expose
> the
> 	 * new available array entries. */
> 	virtio_wmb(vq->weak_barriers);
> 	vq->vring.avail->idx++;
> 

Yes, use virtio_wmb is better here, will change it in next version.

Thanks
Changchun