[Qemu-devel] Re: [PATCH-RFC 0/3] qemu: memory barriers in virtio

[Michael S. Tsirkin]

On Wed, Dec 23, 2009 at 01:32:53PM +0000, Paul Brook wrote:
> On Wednesday 23 December 2009, Michael S. Tsirkin wrote:
> > On Wed, Dec 23, 2009 at 12:25:46PM +0000, Paul Brook wrote:
> > > > > > So possibly this means that we
> > > > > > could optimize the barrier away, but I don't think this amounts to
> > > > > > a serious issue, I guess portability/readability is more important.
> > > > >
> > > > > The more important issue is that regular devices which to not require
> > > > > coherency or ordering can omit this lock.
> > > >
> > > > So let them. What's the issue?
> > >
> > > The issue is that at you're only fixing half the problem. Barriers aren't
> > > sufficient to get the semantics you need. You also need atomicity.
> > >
> > > Given we need both, why not actually defined an API that gives you this?
> > 
> > Because, I do not want to define APIs, I want to reuse an existing one.
> 
> Except that, say you said later in your email, no API exists for doing atomic 
> accesses, so you need different code anyway.

Did I say that? I take it back :) Real devices simply can not do things
like atomic increment etc.  So all we really need is storing two bytes
in memory, and the only reason we do memcpy there is stw needs to
support arbitrary alignment (is that right? if not, we could just fix
stw without a new API)

So the API we need is
stw_aligned()
which has nothing to do with atomics or barriers.

> > E.g. what is stw_barrier?  atomic read followed by a barrier?  read
> > preceded by a barrier? and what kind of barrier? IMO
> 
> stw_barrier is an ordered atomic store.

_barrier implies atomic? Okay ... still, ordered with respect to what?
Preceding stores? Following stores? loads?  All of the above?
We can define such an API, but it's easier to misuse than a familiar and
documented one IMO.

> >With KVM, even these partial barriers add small but measureable overhead
> >(about 2%).
> 
> Now are you measuring that overhead? How much additional overhead does a full 
> barrier incur?
> 
> Paul

This started as an attempt to fix a bug seen by a customer when running
an unnamed benchmark :).  I gave this patch to a customer, the patch
does not help :( But I also got feedback about performance overhead.

With my patch datapath has a single read barrier, and a write barrier,
but write barrier is a nop. If you make write barrier a read barrier,
and add another barrier before operations, I expect about 4 times
the number, so that will be what 8%?

Frankly, I do not think we should add code on data path "just in case".

-- 
MST