vhost: fix false sharing
authorKevin Traynor <ktraynor@redhat.com>
Thu, 23 Mar 2017 15:44:58 +0000 (15:44 +0000)
committerYuanhan Liu <yuanhan.liu@linux.intel.com>
Sat, 1 Apr 2017 08:36:17 +0000 (10:36 +0200)
The broadcast_rarp field in the virtio_net struct is checked in the
dequeue datapath regardless of whether descriptors are available or not.

As it is checked with cmpset leading to a write, false sharing on the
virtio_net struct can happen between enqueue and dequeue datapaths
regardless of whether a RARP is requested. In OVS, the issue can cause
a uni-directional performance drop of up to 15%.

Fix that by only performing the cmpset if a read of broadcast_rarp
indicates that the cmpset is likely to succeed.

Fixes: a66bcad32240 ("vhost: arrange struct fields for better cache sharing")
Cc: stable@dpdk.org
Signed-off-by: Kevin Traynor <ktraynor@redhat.com>
Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
Acked-by: Yuanhan Liu <yuanhan.liu@linux.intel.com>
lib/librte_vhost/virtio_net.c

index 337470d..d0a3b11 100644 (file)
@@ -1056,9 +1056,21 @@ rte_vhost_dequeue_burst(int vid, uint16_t queue_id,
         * array, to looks like that guest actually send such packet.
         *
         * Check user_send_rarp() for more information.
+        *
+        * broadcast_rarp shares a cacheline in the virtio_net structure
+        * with some fields that are accessed during enqueue and
+        * rte_atomic16_cmpset() causes a write if using cmpxchg. This could
+        * result in false sharing between enqueue and dequeue.
+        *
+        * Prevent unnecessary false sharing by reading broadcast_rarp first
+        * and only performing cmpset if the read indicates it is likely to
+        * be set.
         */
-       if (unlikely(rte_atomic16_cmpset((volatile uint16_t *)
-                                        &dev->broadcast_rarp.cnt, 1, 0))) {
+
+       if (unlikely(rte_atomic16_read(&dev->broadcast_rarp) &&
+                       rte_atomic16_cmpset((volatile uint16_t *)
+                               &dev->broadcast_rarp.cnt, 1, 0))) {
+
                rarp_mbuf = rte_pktmbuf_alloc(mbuf_pool);
                if (rarp_mbuf == NULL) {
                        RTE_LOG(ERR, VHOST_DATA,