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 Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/virtualization