From: Victor Kaplansky <vkaplans@redhat.com>
To: dev@dpdk.org, yliu@fridaylinux.org, tiwei.bie@intel.com,
jianfeng.tan@intel.com
Cc: stable@dpdk.org, jfreiman@redhat.com,
Maxime Coquelin <maxime.coquelin@redhat.com>,
Victor Kaplansky <victork@redhat.com>
Subject: [dpdk-stable] [RFC PATCH v2] vhost_user: protect active rings from async ring changes
Date: Wed, 6 Dec 2017 17:28:20 +0200 [thread overview]
Message-ID: <20171206152731.2242-1-vkaplans@redhat.com> (raw)
v2:
o Fixed checkpatch complains
o Added Signed-off-by
o Refined placement of guard to exclude IOMMU messages
o TODO: performance degradation measurement
See https://bugzilla.redhat.com/show_bug.cgi?id=1450680
When performing live migration or memory hot-plugging,
the changes to the device and vrings made by message handler
done independently from vring usage by PMD threads.
This causes for example segfauls during live-migration
with MQ enable, but in general virtually any request
sent by qemu changing the state of device can cause
problems.
These patches fixes all above issues by adding a spinlock
to every vring and requiring message handler to start operation
only after ensuring that all PMD threads related to the divece
are out of critical section accessing the vring data.
Each vring has its own lock in order to not create contention
between PMD threads of different vrings and to prevent
performance degradation by scaling queue pair number.
Signed-off-by: Victor Kaplansky <victork@redhat.com>
---
lib/librte_vhost/vhost.h | 1 +
lib/librte_vhost/vhost_user.c | 46 +++++++++++++++++++++++++++++++++++++++++++
lib/librte_vhost/virtio_net.c | 8 ++++++++
3 files changed, 55 insertions(+)
diff --git a/lib/librte_vhost/vhost.h b/lib/librte_vhost/vhost.h
index 1cc81c17..2ebe147f 100644
--- a/lib/librte_vhost/vhost.h
+++ b/lib/librte_vhost/vhost.h
@@ -137,6 +137,7 @@ struct vhost_virtqueue {
TAILQ_HEAD(, vhost_iotlb_entry) iotlb_list;
int iotlb_cache_nr;
TAILQ_HEAD(, vhost_iotlb_entry) iotlb_pending_list;
+ rte_spinlock_t active_lock;
} __rte_cache_aligned;
/* Old kernels have no such macros defined */
diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c
index f4c7ce46..60d82e01 100644
--- a/lib/librte_vhost/vhost_user.c
+++ b/lib/librte_vhost/vhost_user.c
@@ -1190,6 +1190,46 @@ vhost_user_check_and_alloc_queue_pair(struct virtio_net *dev, VhostUserMsg *msg)
return alloc_vring_queue(dev, vring_idx);
}
+static void
+vhost_user_lock_all_queue_pairs(struct virtio_net *dev)
+{
+ unsigned int i = 0;
+ unsigned int vq_num = 0;
+
+ while (vq_num < dev->nr_vring) {
+ struct vhost_virtqueue *vq = dev->virtqueue[i];
+
+ if (!vq) {
+ i++;
+ continue;
+ }
+
+ rte_spinlock_lock(&vq->active_lock);
+ vq_num++;
+ i++;
+ }
+}
+
+static void
+vhost_user_unlock_all_queue_pairs(struct virtio_net *dev)
+{
+ unsigned int i = 0;
+ unsigned int vq_num = 0;
+
+ while (vq_num < dev->nr_vring) {
+ struct vhost_virtqueue *vq = dev->virtqueue[i];
+
+ if (!vq) {
+ i++;
+ continue;
+ }
+
+ rte_spinlock_unlock(&vq->active_lock);
+ vq_num++;
+ i++;
+ }
+}
+
int
vhost_user_msg_handler(int vid, int fd)
{
@@ -1241,6 +1281,9 @@ vhost_user_msg_handler(int vid, int fd)
return -1;
}
+ if (msg.request.master != VHOST_USER_IOTLB_MSG)
+ vhost_user_lock_all_queue_pairs(dev);
+
switch (msg.request.master) {
case VHOST_USER_GET_FEATURES:
msg.payload.u64 = vhost_user_get_features(dev);
@@ -1342,6 +1385,9 @@ vhost_user_msg_handler(int vid, int fd)
}
+ if (msg.request.master != VHOST_USER_IOTLB_MSG)
+ vhost_user_unlock_all_queue_pairs(dev);
+
if (msg.flags & VHOST_USER_NEED_REPLY) {
msg.payload.u64 = !!ret;
msg.size = sizeof(msg.payload.u64);
diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c
index 6fee16e5..0337dc28 100644
--- a/lib/librte_vhost/virtio_net.c
+++ b/lib/librte_vhost/virtio_net.c
@@ -44,6 +44,7 @@
#include <rte_udp.h>
#include <rte_sctp.h>
#include <rte_arp.h>
+#include <rte_spinlock.h>
#include "iotlb.h"
#include "vhost.h"
@@ -1180,9 +1181,12 @@ rte_vhost_dequeue_burst(int vid, uint16_t queue_id,
}
vq = dev->virtqueue[queue_id];
+
if (unlikely(vq->enabled == 0))
return 0;
+ rte_spinlock_lock(&vq->active_lock);
+
vq->batch_copy_nb_elems = 0;
if (dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM))
@@ -1240,6 +1244,7 @@ rte_vhost_dequeue_burst(int vid, uint16_t queue_id,
if (rarp_mbuf == NULL) {
RTE_LOG(ERR, VHOST_DATA,
"Failed to allocate memory for mbuf.\n");
+ rte_spinlock_unlock(&vq->active_lock);
return 0;
}
@@ -1356,6 +1361,8 @@ rte_vhost_dequeue_burst(int vid, uint16_t queue_id,
if (dev->features & (1ULL << VIRTIO_F_IOMMU_PLATFORM))
vhost_user_iotlb_rd_unlock(vq);
+ rte_spinlock_unlock(&vq->active_lock);
+
if (unlikely(rarp_mbuf != NULL)) {
/*
* Inject it to the head of "pkts" array, so that switch's mac
@@ -1366,5 +1373,6 @@ rte_vhost_dequeue_burst(int vid, uint16_t queue_id,
i += 1;
}
+
return i;
}
--
Victor
next reply other threads:[~2017-12-06 15:28 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-12-06 15:28 Victor Kaplansky [this message]
2017-12-06 18:01 ` Maxime Coquelin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20171206152731.2242-1-vkaplans@redhat.com \
--to=vkaplans@redhat.com \
--cc=dev@dpdk.org \
--cc=jfreiman@redhat.com \
--cc=jianfeng.tan@intel.com \
--cc=maxime.coquelin@redhat.com \
--cc=stable@dpdk.org \
--cc=tiwei.bie@intel.com \
--cc=victork@redhat.com \
--cc=yliu@fridaylinux.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).