From: VMware, Inc <>
Date: Fri, 18 Sep 2009 21:33:58 +0000 (-0700)
Subject: Add comments regarding barrier usage.
X-Git-Tag: 2009.09.18-193784~29
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=de54043e4cfe716a2fe901cc594f2c73a9effe6d;p=thirdparty%2Fopen-vm-tools.git

Add comments regarding barrier usage.

Add comments to explain why we use the naked barriers for PVSCSI
and the races that may arise without the barrier.

Signed-off-by: Marcelo Vanzin <mvanzin@vmware.com>
---

diff --git a/open-vm-tools/modules/linux/pvscsi/pvscsi.c b/open-vm-tools/modules/linux/pvscsi/pvscsi.c
index 7c22ea300..6c086825a 100644
--- a/open-vm-tools/modules/linux/pvscsi/pvscsi.c
+++ b/open-vm-tools/modules/linux/pvscsi/pvscsi.c
@@ -623,6 +623,13 @@ static void pvscsi_complete_request(struct pvscsi_adapter *adapter,
 	cmd->scsi_done(cmd);
 }
 
+/*
+ * barrier usage : Since the PVSCSI device is emulated, there could be cases
+ * where we may want to serialize some accesses between the driver and the
+ * emulation layer. We use compiler barriers instead of the more expensive
+ * memory barriers because PVSCSI is only supported on X86 which has strong
+ * memory access ordering.
+ */
 static void pvscsi_process_completion_ring(struct pvscsi_adapter *adapter)
 {
 	PVSCSIRingsState *s = adapter->rings_state;
@@ -632,9 +639,20 @@ static void pvscsi_process_completion_ring(struct pvscsi_adapter *adapter)
 	while (s->cmpConsIdx != s->cmpProdIdx) {
 		PVSCSIRingCmpDesc *e = ring + (s->cmpConsIdx &
 					       MASK(cmp_entries));
-
+		/*
+		 * This barrier() ensures that *e is not dereferenced while
+		 * the device emulation still writes data into the slot.
+		 * Since the device emulation advances s->cmpProdIdx only after
+		 * updating the slot we want to check it first.
+		 */
 		barrier();
 		pvscsi_complete_request(adapter, e);
+		/*
+		 * This barrier() ensures that compiler doesn't reorder write
+		 * to s->cmpConsIdx before the read of (*e) inside
+		 * pvscsi_complete_request. Otherwise, device emulation may
+		 * overwrite *e before we had a chance to read it.
+		 */
 		barrier();
 		s->cmpConsIdx++;
 	}