On Fri, 29 Jan 2010 12:07:08 am Michael S. Tsirkin wrote:
> On Thu, Jan 28, 2010 at 10:01:09AM +1030, Rusty Russell wrote:
> > On Thu, 28 Jan 2010 09:12:23 am Michael S. Tsirkin wrote:
> > > Where possible, we should use SMP barriers which are more lightweight than
> > > mandatory barriers, because mandatory barriers also control MMIO effects
> > > on
> > > accesses through relaxed memory I/O windows (which virtio does not use)
> > > (compare specifically smp_rmb and rmb on x86_64).
> >
> > Xen had a similar issue, in that UP guests running on SMP hosts need to
> > issue
> > SMP barriers. Is this not also a requirement for virtio?
>
> Of course it is. That's why I have ifdef CONFIG_SMP and use
> mandatory barriers on UP.
Sorry, this was an off-the-cuff comment. I was concerned that UP barriers
might be insufficient on some archs. But I can't find any such archs when
I actually look (at least, x86, s390 and powerpc).
I've applied your patch, minus the first two substitutions:
@@ -36,10 +54,10 @@
panic("%s:in_use = %i\n", \
(_vq)->vq.name, (_vq)->in_use); \
(_vq)->in_use = __LINE__; \
- mb(); \
+ virtio_mb(); \
} while (0)
#define END_USE(_vq) \
- do { BUG_ON(!(_vq)->in_use); (_vq)->in_use = 0; mb(); } while(0)
+ do { BUG_ON(!(_vq)->in_use); (_vq)->in_use = 0; virtio_mb(); } while(0)
#else
#define BAD_RING(_vq, fmt, args...) \
do { \
These barriers are actually trying to make sure in_use is set in a timely
manner (this is debug code). They're bogus AFAICT: if you don't have any
other synchronization then you're in danger of nesting and you want to know.
And barriers might change timing too much when DEBUG is defined.
Thanks!
Rusty.
_______________________________________________
Virtualization mailing list
[email protected]
https://lists.linux-foundation.org/mailman/listinfo/virtualization