On Mon, May 24, 2010 at 01:01:34PM +0300, Michael S. Tsirkin wrote:
On Mon, May 24, 2010 at 12:55:30PM +0300, Gleb Natapov wrote:
On Mon, May 24, 2010 at 12:39:41PM +0300, Michael S. Tsirkin wrote:
On Mon, May 24, 2010 at 12:35:21PM +0300, Gleb Natapov wrote:
On Thu, May 20, 2010 at 04:36:32PM +0300, Michael S. Tsirkin wrote:
diff --git a/src/virtio-ring.h b/src/virtio-ring.h index 3fb86fe..8b546f4 100644 --- a/src/virtio-ring.h +++ b/src/virtio-ring.h @@ -9,8 +9,9 @@
#define virt_to_phys(v) (unsigned long)(v) #define phys_to_virt(p) (void*)(p) -#define wmb() barrier() -#define mb() barrier() +/* Compiler barrier is enough as an x86 CPU does not reorder reads or writes */ +#define smp_rmb() barrier() +#define smp_wmb() barrier()
I thought you were going to use real memory barriers (although we concluded compiler barrier should be enough, but it's better to be safe then sorry).
-- Gleb.
On x86, ops of the same kind are ordered by CPU, so all we need to do is
- avoid depending on order between reads and writes
- prevent the compiler from reordering memory accesses
So on Linux smp_rmb() and smp_wmb() are just compiler barriers too? (look like they are).
On x86, yes.
Of course x86. We are talking about x86 BIOS code here ;)
In that case lets use barrier() directly no need to obfuscate the code.
I think it's better to use smp_rmb/smp_wmb because this code tends to get copied around, it might end up on non-x86 platforms. For these, barrier() would be wrong.
So they will have a bug. It will teach them to not blindly copy code.
Another reason is that it makes it easier to audit code comparing it with linux virtio guest.
If you know that on x86 smp_(w|r)mb() are just barrier() it's still easy. To be honest though this is the reason I left virt_to_phys()/phys_to_virt(), so Kevin this is your call.
-- Gleb.