* [dpdk-dev] [PATCH RFC v2 0/3] Thread safe rte_vhost_enqueue_burst().
@ 2016-02-19 10:56 Ilya Maximets
2016-02-19 10:56 ` [dpdk-dev] [PATCH RFC v2 1/3] vhost: use SMP barriers instead of compiler ones Ilya Maximets
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Ilya Maximets @ 2016-02-19 10:56 UTC (permalink / raw)
To: dev, Huawei Xie, Yuanhan Liu; +Cc: Ilya Maximets, Dyasly Sergey
Implementation of rte_vhost_enqueue_burst() based on lockless ring-buffer
algorithm and contains almost all to be thread-safe, but it's not.
This set adds required changes.
First patch in set is a standalone patch that fixes many times discussed
issue with barriers on different architectures.
Second and third adds fixes to make rte_vhost_enqueue_burst thread safe.
version 2:
* Documentation patch dropped. Other patches of series still
may be merged to fix existing issues and keep code in
consistent state for the future.
* buf_vec field of struct vhost_virtqueue marked as deprecated.
Ilya Maximets (3):
vhost: use SMP barriers instead of compiler ones.
vhost: make buf vector for scatter RX local.
vhost: avoid reordering of used->idx and last_used_idx updating.
doc/guides/rel_notes/deprecation.rst | 1 +
lib/librte_vhost/rte_virtio_net.h | 2 +-
lib/librte_vhost/vhost_rxtx.c | 67 ++++++++++++++++++++----------------
3 files changed, 40 insertions(+), 30 deletions(-)
--
2.5.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* [dpdk-dev] [PATCH RFC v2 1/3] vhost: use SMP barriers instead of compiler ones.
2016-02-19 10:56 [dpdk-dev] [PATCH RFC v2 0/3] Thread safe rte_vhost_enqueue_burst() Ilya Maximets
@ 2016-02-19 10:56 ` Ilya Maximets
2016-02-19 10:56 ` [dpdk-dev] [PATCH RFC v2 2/3] vhost: make buf vector for scatter RX local Ilya Maximets
2016-02-19 10:56 ` [dpdk-dev] [PATCH RFC v2 3/3] vhost: avoid reordering of used->idx and last_used_idx updating Ilya Maximets
2 siblings, 0 replies; 4+ messages in thread
From: Ilya Maximets @ 2016-02-19 10:56 UTC (permalink / raw)
To: dev, Huawei Xie, Yuanhan Liu; +Cc: Ilya Maximets, Dyasly Sergey
Since commit 4c02e453cc62 ("eal: introduce SMP memory barriers") virtio
uses architecture dependent SMP barriers. vHost should use them too.
Fixes: 4c02e453cc62 ("eal: introduce SMP memory barriers")
Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
---
lib/librte_vhost/vhost_rxtx.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/lib/librte_vhost/vhost_rxtx.c b/lib/librte_vhost/vhost_rxtx.c
index 5e7e5b1..411dd95 100644
--- a/lib/librte_vhost/vhost_rxtx.c
+++ b/lib/librte_vhost/vhost_rxtx.c
@@ -274,7 +274,7 @@ virtio_dev_rx(struct virtio_net *dev, uint16_t queue_id,
}
}
- rte_compiler_barrier();
+ rte_smp_wmb();
/* Wait until it's our turn to add our buffer to the used ring. */
while (unlikely(vq->last_used_idx != res_base_idx))
@@ -575,7 +575,7 @@ virtio_dev_merge_rx(struct virtio_net *dev, uint16_t queue_id,
entry_success = copy_from_mbuf_to_vring(dev, queue_id,
res_base_idx, res_cur_idx, pkts[pkt_idx]);
- rte_compiler_barrier();
+ rte_smp_wmb();
/*
* Wait until it's our turn to add our buffer
@@ -917,7 +917,7 @@ rte_vhost_dequeue_burst(struct virtio_net *dev, uint16_t queue_id,
entry_success++;
}
- rte_compiler_barrier();
+ rte_smp_rmb();
vq->used->idx += entry_success;
/* Kick guest if required. */
if (!(vq->avail->flags & VRING_AVAIL_F_NO_INTERRUPT))
--
2.5.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* [dpdk-dev] [PATCH RFC v2 2/3] vhost: make buf vector for scatter RX local.
2016-02-19 10:56 [dpdk-dev] [PATCH RFC v2 0/3] Thread safe rte_vhost_enqueue_burst() Ilya Maximets
2016-02-19 10:56 ` [dpdk-dev] [PATCH RFC v2 1/3] vhost: use SMP barriers instead of compiler ones Ilya Maximets
@ 2016-02-19 10:56 ` Ilya Maximets
2016-02-19 10:56 ` [dpdk-dev] [PATCH RFC v2 3/3] vhost: avoid reordering of used->idx and last_used_idx updating Ilya Maximets
2 siblings, 0 replies; 4+ messages in thread
From: Ilya Maximets @ 2016-02-19 10:56 UTC (permalink / raw)
To: dev, Huawei Xie, Yuanhan Liu; +Cc: Ilya Maximets, Dyasly Sergey
Array of buf_vector's is just an array for temporary storing information
about available descriptors. It used only locally in virtio_dev_merge_rx()
and there is no reason for that array to be shared.
Fix that by allocating local buf_vec inside virtio_dev_merge_rx().
buf_vec field of struct vhost_virtqueue marked as deprecated.
Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
---
doc/guides/rel_notes/deprecation.rst | 1 +
lib/librte_vhost/rte_virtio_net.h | 2 +-
lib/librte_vhost/vhost_rxtx.c | 45 ++++++++++++++++++------------------
3 files changed, 25 insertions(+), 23 deletions(-)
diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index e94d4a2..40f350d 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -7,6 +7,7 @@ API and ABI deprecation notices are to be posted here.
Deprecation Notices
-------------------
+* Field buf_vec of struct vhost_virtqueue have been deprecated.
* The following fields have been deprecated in rte_eth_stats:
ibadcrc, ibadlen, imcasts, fdirmatch, fdirmiss,
diff --git a/lib/librte_vhost/rte_virtio_net.h b/lib/librte_vhost/rte_virtio_net.h
index 10dcb90..db05d68 100644
--- a/lib/librte_vhost/rte_virtio_net.h
+++ b/lib/librte_vhost/rte_virtio_net.h
@@ -91,7 +91,7 @@ struct vhost_virtqueue {
int kickfd; /**< Currently unused as polling mode is enabled. */
int enabled;
uint64_t reserved[16]; /**< Reserve some spaces for future extension. */
- struct buf_vector buf_vec[BUF_VECTOR_MAX]; /**< for scatter RX. */
+ struct buf_vector buf_vec[BUF_VECTOR_MAX] __rte_deprecated; /**< @deprecated Buffer for scatter RX. */
} __rte_cache_aligned;
diff --git a/lib/librte_vhost/vhost_rxtx.c b/lib/librte_vhost/vhost_rxtx.c
index 411dd95..9095fb1 100644
--- a/lib/librte_vhost/vhost_rxtx.c
+++ b/lib/librte_vhost/vhost_rxtx.c
@@ -295,7 +295,7 @@ virtio_dev_rx(struct virtio_net *dev, uint16_t queue_id,
static inline uint32_t __attribute__((always_inline))
copy_from_mbuf_to_vring(struct virtio_net *dev, uint32_t queue_id,
uint16_t res_base_idx, uint16_t res_end_idx,
- struct rte_mbuf *pkt)
+ struct rte_mbuf *pkt, struct buf_vector *buf_vec)
{
uint32_t vec_idx = 0;
uint32_t entry_success = 0;
@@ -325,7 +325,7 @@ copy_from_mbuf_to_vring(struct virtio_net *dev, uint32_t queue_id,
*/
vq = dev->virtqueue[queue_id];
- vb_addr = gpa_to_vva(dev, vq->buf_vec[vec_idx].buf_addr);
+ vb_addr = gpa_to_vva(dev, buf_vec[vec_idx].buf_addr);
vb_hdr_addr = vb_addr;
/* Prefetch buffer address. */
@@ -345,19 +345,19 @@ copy_from_mbuf_to_vring(struct virtio_net *dev, uint32_t queue_id,
seg_avail = rte_pktmbuf_data_len(pkt);
vb_offset = vq->vhost_hlen;
- vb_avail = vq->buf_vec[vec_idx].buf_len - vq->vhost_hlen;
+ vb_avail = buf_vec[vec_idx].buf_len - vq->vhost_hlen;
entry_len = vq->vhost_hlen;
if (vb_avail == 0) {
uint32_t desc_idx =
- vq->buf_vec[vec_idx].desc_idx;
+ buf_vec[vec_idx].desc_idx;
if ((vq->desc[desc_idx].flags
& VRING_DESC_F_NEXT) == 0) {
/* Update used ring with desc information */
vq->used->ring[cur_idx & (vq->size - 1)].id
- = vq->buf_vec[vec_idx].desc_idx;
+ = buf_vec[vec_idx].desc_idx;
vq->used->ring[cur_idx & (vq->size - 1)].len
= entry_len;
@@ -367,12 +367,12 @@ copy_from_mbuf_to_vring(struct virtio_net *dev, uint32_t queue_id,
}
vec_idx++;
- vb_addr = gpa_to_vva(dev, vq->buf_vec[vec_idx].buf_addr);
+ vb_addr = gpa_to_vva(dev, buf_vec[vec_idx].buf_addr);
/* Prefetch buffer address. */
rte_prefetch0((void *)(uintptr_t)vb_addr);
vb_offset = 0;
- vb_avail = vq->buf_vec[vec_idx].buf_len;
+ vb_avail = buf_vec[vec_idx].buf_len;
}
cpy_len = RTE_MIN(vb_avail, seg_avail);
@@ -399,11 +399,11 @@ copy_from_mbuf_to_vring(struct virtio_net *dev, uint32_t queue_id,
* entry reach to its end.
* But the segment doesn't complete.
*/
- if ((vq->desc[vq->buf_vec[vec_idx].desc_idx].flags &
+ if ((vq->desc[buf_vec[vec_idx].desc_idx].flags &
VRING_DESC_F_NEXT) == 0) {
/* Update used ring with desc information */
vq->used->ring[cur_idx & (vq->size - 1)].id
- = vq->buf_vec[vec_idx].desc_idx;
+ = buf_vec[vec_idx].desc_idx;
vq->used->ring[cur_idx & (vq->size - 1)].len
= entry_len;
entry_len = 0;
@@ -413,9 +413,9 @@ copy_from_mbuf_to_vring(struct virtio_net *dev, uint32_t queue_id,
vec_idx++;
vb_addr = gpa_to_vva(dev,
- vq->buf_vec[vec_idx].buf_addr);
+ buf_vec[vec_idx].buf_addr);
vb_offset = 0;
- vb_avail = vq->buf_vec[vec_idx].buf_len;
+ vb_avail = buf_vec[vec_idx].buf_len;
cpy_len = RTE_MIN(vb_avail, seg_avail);
} else {
/*
@@ -434,7 +434,7 @@ copy_from_mbuf_to_vring(struct virtio_net *dev, uint32_t queue_id,
* from buf_vec.
*/
uint32_t desc_idx =
- vq->buf_vec[vec_idx].desc_idx;
+ buf_vec[vec_idx].desc_idx;
if ((vq->desc[desc_idx].flags &
VRING_DESC_F_NEXT) == 0) {
@@ -456,9 +456,9 @@ copy_from_mbuf_to_vring(struct virtio_net *dev, uint32_t queue_id,
/* Get next buffer from buf_vec. */
vec_idx++;
vb_addr = gpa_to_vva(dev,
- vq->buf_vec[vec_idx].buf_addr);
+ buf_vec[vec_idx].buf_addr);
vb_avail =
- vq->buf_vec[vec_idx].buf_len;
+ buf_vec[vec_idx].buf_len;
vb_offset = 0;
}
@@ -471,7 +471,7 @@ copy_from_mbuf_to_vring(struct virtio_net *dev, uint32_t queue_id,
*/
/* Update used ring with desc information */
vq->used->ring[cur_idx & (vq->size - 1)].id
- = vq->buf_vec[vec_idx].desc_idx;
+ = buf_vec[vec_idx].desc_idx;
vq->used->ring[cur_idx & (vq->size - 1)].len
= entry_len;
entry_success++;
@@ -485,7 +485,7 @@ copy_from_mbuf_to_vring(struct virtio_net *dev, uint32_t queue_id,
static inline void __attribute__((always_inline))
update_secure_len(struct vhost_virtqueue *vq, uint32_t id,
- uint32_t *secure_len, uint32_t *vec_idx)
+ uint32_t *secure_len, uint32_t *vec_idx, struct buf_vector *buf_vec)
{
uint16_t wrapped_idx = id & (vq->size - 1);
uint32_t idx = vq->avail->ring[wrapped_idx];
@@ -496,9 +496,9 @@ update_secure_len(struct vhost_virtqueue *vq, uint32_t id,
do {
next_desc = 0;
len += vq->desc[idx].len;
- vq->buf_vec[vec_id].buf_addr = vq->desc[idx].addr;
- vq->buf_vec[vec_id].buf_len = vq->desc[idx].len;
- vq->buf_vec[vec_id].desc_idx = idx;
+ buf_vec[vec_id].buf_addr = vq->desc[idx].addr;
+ buf_vec[vec_id].buf_len = vq->desc[idx].len;
+ buf_vec[vec_id].desc_idx = idx;
vec_id++;
if (vq->desc[idx].flags & VRING_DESC_F_NEXT) {
@@ -523,6 +523,7 @@ virtio_dev_merge_rx(struct virtio_net *dev, uint16_t queue_id,
uint16_t avail_idx;
uint16_t res_base_idx, res_cur_idx;
uint8_t success = 0;
+ struct buf_vector buf_vec[BUF_VECTOR_MAX];
LOG_DEBUG(VHOST_DATA, "(%"PRIu64") virtio_dev_merge_rx()\n",
dev->device_fh);
@@ -561,8 +562,8 @@ virtio_dev_merge_rx(struct virtio_net *dev, uint16_t queue_id,
if (unlikely(res_cur_idx == avail_idx))
goto merge_rx_exit;
- update_secure_len(vq, res_cur_idx,
- &secure_len, &vec_idx);
+ update_secure_len(vq, res_cur_idx, &secure_len,
+ &vec_idx, buf_vec);
res_cur_idx++;
} while (pkt_len > secure_len);
@@ -573,7 +574,7 @@ virtio_dev_merge_rx(struct virtio_net *dev, uint16_t queue_id,
} while (success == 0);
entry_success = copy_from_mbuf_to_vring(dev, queue_id,
- res_base_idx, res_cur_idx, pkts[pkt_idx]);
+ res_base_idx, res_cur_idx, pkts[pkt_idx], buf_vec);
rte_smp_wmb();
--
2.5.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* [dpdk-dev] [PATCH RFC v2 3/3] vhost: avoid reordering of used->idx and last_used_idx updating.
2016-02-19 10:56 [dpdk-dev] [PATCH RFC v2 0/3] Thread safe rte_vhost_enqueue_burst() Ilya Maximets
2016-02-19 10:56 ` [dpdk-dev] [PATCH RFC v2 1/3] vhost: use SMP barriers instead of compiler ones Ilya Maximets
2016-02-19 10:56 ` [dpdk-dev] [PATCH RFC v2 2/3] vhost: make buf vector for scatter RX local Ilya Maximets
@ 2016-02-19 10:56 ` Ilya Maximets
2 siblings, 0 replies; 4+ messages in thread
From: Ilya Maximets @ 2016-02-19 10:56 UTC (permalink / raw)
To: dev, Huawei Xie, Yuanhan Liu; +Cc: Ilya Maximets, Dyasly Sergey
Calling rte_vhost_enqueue_burst() simultaneously from different threads
for the same queue_id requires additional SMP memory barrier to avoid
reordering of used->idx and last_used_idx updates.
In case of virtio_dev_rx() memory barrier rte_mb() simply moved one
instruction higher.
Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
---
lib/librte_vhost/vhost_rxtx.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/lib/librte_vhost/vhost_rxtx.c b/lib/librte_vhost/vhost_rxtx.c
index 9095fb1..a03f687 100644
--- a/lib/librte_vhost/vhost_rxtx.c
+++ b/lib/librte_vhost/vhost_rxtx.c
@@ -281,10 +281,13 @@ virtio_dev_rx(struct virtio_net *dev, uint16_t queue_id,
rte_pause();
*(volatile uint16_t *)&vq->used->idx += count;
- vq->last_used_idx = res_end_idx;
- /* flush used->idx update before we read avail->flags. */
+ /*
+ * Flush used->idx update to make it visible to virtio and all other
+ * threads before allowing to modify it.
+ */
rte_mb();
+ vq->last_used_idx = res_end_idx;
/* Kick the guest if necessary. */
if (!(vq->avail->flags & VRING_AVAIL_F_NO_INTERRUPT))
@@ -586,19 +589,24 @@ virtio_dev_merge_rx(struct virtio_net *dev, uint16_t queue_id,
rte_pause();
*(volatile uint16_t *)&vq->used->idx += entry_success;
+ /*
+ * Flush used->idx update to make it visible to all
+ * other threads before allowing to modify it.
+ */
+ rte_smp_wmb();
+
vq->last_used_idx = res_cur_idx;
}
merge_rx_exit:
if (likely(pkt_idx)) {
- /* flush used->idx update before we read avail->flags. */
+ /* Flush used->idx update to make it visible to virtio. */
rte_mb();
/* Kick the guest if necessary. */
if (!(vq->avail->flags & VRING_AVAIL_F_NO_INTERRUPT))
eventfd_write(vq->callfd, (eventfd_t)1);
}
-
return pkt_idx;
}
--
2.5.0
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-02-19 10:56 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-19 10:56 [dpdk-dev] [PATCH RFC v2 0/3] Thread safe rte_vhost_enqueue_burst() Ilya Maximets
2016-02-19 10:56 ` [dpdk-dev] [PATCH RFC v2 1/3] vhost: use SMP barriers instead of compiler ones Ilya Maximets
2016-02-19 10:56 ` [dpdk-dev] [PATCH RFC v2 2/3] vhost: make buf vector for scatter RX local Ilya Maximets
2016-02-19 10:56 ` [dpdk-dev] [PATCH RFC v2 3/3] vhost: avoid reordering of used->idx and last_used_idx updating Ilya Maximets
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).