svn commit: r240427 - head/sys/dev/virtio

[Bryan Venteicher]

Hi,


----- Original Message -----
> From: "John Baldwin" <jhb at freebsd.org>
> To: "Konstantin Belousov" <kostikbel at gmail.com>
> Cc: "Bryan Venteicher" <bryanv at daemoninthecloset.org>, svn-src-head at freebsd.org, svn-src-all at freebsd.org,
> src-committers at freebsd.org, "Peter Grehan" <grehan at freebsd.org>
> Sent: Friday, September 14, 2012 7:17:54 AM
> Subject: Re: svn commit: r240427 - head/sys/dev/virtio
> 
> On Friday, September 14, 2012 3:55:20 am Konstantin Belousov wrote:
> > On Fri, Sep 14, 2012 at 12:47:52AM -0500, Bryan Venteicher wrote:
> > > > > I also found myself wanting an atomic_load_rel_*() type
> > > > > function.
> > > > 
> > > > That would be odd I think.  _rel barriers only affect stores,
> > > > so
> > > > there would be no defined ordering between the load and the
> > > > subsequent stores.  (With our current definitions of _acq and
> > > > _rel.)  If you need a full fence for some reason, than a plain
> > > > mb() may be the best thing in that case.
> > > > 
> > > 
> > > I'm able to batch add descriptors (via vq_ring_update_avail()),
> > > but when checking if I must notify the host, I need to make sure
> > > the latest avail->idx is visible before checking the flag from
> > > the host on whether notifications are disabled. Gratuitous
> > > notifications are fine, but skipping one is not.
> > > 
> > > In the patch, I kludge this with:
> > >     atomic_add_rel_16(&flags, 0);
> > >     foo = flags;
> > Don't you need
> > 	atomic_store_rel_16(&foo, flags);
> > instead ?
> > 
> > You might do a cas_rel over the containing 32bit word as well.
> 
> Yes, the right barrier here would be to use the barrier on the
> assignment
> to foo.  That ensures all other writes post before the write to
> 'foo'.

>From reading between the lines of atomic(9), I wasn't totally sure
where the load of flags is guaranteed to occur with respect to the
previous loads/stores.

Bryan

> 
> --
> John Baldwin
>