OASIS Mailing List ArchivesView the OASIS mailing list archive below
or browse/search using MarkMail.

 


Help: OASIS Mailing Lists Help | MarkMail Help

virtio-dev message

[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]


Subject: [PATCH v4 3/6] virtio/vhost-vsock: avoid potential deadlock


In some circumstances, like when a RESET packet needs to be sent back,
both the send and receive locks are held. This makes lockdep unhappy,
because both locks belong to the same class. In general it is not a good
idea to hold both the send and receive locks at the same time; this patch
introduces a reset queue, serviced by the default workqueue. This
guarantees that the reset packets are sent back with only one lock being
held, which makes lockdep happy.

Signed-off-by: Claudio Imbrenda <imbrenda@linux.vnet.ibm.com>
---
 net/vmw_vsock/virtio_transport_common.c | 98 +++++++++++++++++++++++++++++++--
 1 file changed, 93 insertions(+), 5 deletions(-)

diff --git a/net/vmw_vsock/virtio_transport_common.c b/net/vmw_vsock/virtio_transport_common.c
index eab1644..fa54359 100644
--- a/net/vmw_vsock/virtio_transport_common.c
+++ b/net/vmw_vsock/virtio_transport_common.c
@@ -8,6 +8,7 @@
  * This work is licensed under the terms of the GNU GPL, version 2.
  */
 #include <linux/spinlock.h>
+#include <linux/workqueue.h>
 #include <linux/module.h>
 #include <linux/ctype.h>
 #include <linux/list.h>
@@ -22,6 +23,27 @@
 #define CREATE_TRACE_POINTS
 #include <trace/events/vsock_virtio_transport_common.h>
 
+enum {
+	VSOCK_RESET_NOSOCK = 0,
+	VSOCK_RESET_WITHSOCK,
+};
+
+struct reset_queue {
+	struct list_head list;
+	union {
+		struct virtio_vsock_pkt *pkt;
+		struct vsock_sock *vsk;
+	} u;
+	char type;
+};
+
+static void virtio_transport_send_reset_work(struct work_struct *unused);
+
+static DEFINE_SPINLOCK(resetqueue_lock);
+static LIST_HEAD(queued_resets);
+static DECLARE_WORK(virtio_transport_resets_wq,
+			virtio_transport_send_reset_work);
+
 static const struct virtio_transport *virtio_transport_get_ops(void)
 {
 	const struct vsock_transport *t = vsock_core_get_transport();
@@ -544,19 +566,61 @@ void virtio_transport_destruct(struct vsock_sock *vsk)
 }
 EXPORT_SYMBOL_GPL(virtio_transport_destruct);
 
-static int virtio_transport_send_reset(struct vsock_sock *vsk,
-				       struct virtio_vsock_pkt *pkt)
+static void virtio_transport_send_reset_work(struct work_struct *unused)
 {
+	struct reset_queue *q;
 	struct virtio_vsock_pkt_info info = {
 		.op = VIRTIO_VSOCK_OP_RST,
 		.type = VIRTIO_VSOCK_TYPE_STREAM,
 	};
 
+	spin_lock(&resetqueue_lock);
+	while (!list_empty(&queued_resets)) {
+		q = list_first_entry(&queued_resets, struct reset_queue, list);
+		list_del(&q->list);
+		spin_unlock(&resetqueue_lock);
+
+		/* Since we are now asynchronous, we ignore the return values.
+		 * After all we don't even know who we should report them to.
+		 */
+		switch (q->type) {
+		case VSOCK_RESET_NOSOCK:
+			(void)virtio_transport_send_pkt_no_sock(q->u.pkt);
+			break;
+		case VSOCK_RESET_WITHSOCK:
+			(void)virtio_transport_send_pkt(q->u.vsk, &info);
+			break;
+		}
+
+		kfree(q);
+		spin_lock(&resetqueue_lock);
+	}
+	spin_unlock(&resetqueue_lock);
+}
+
+static int virtio_transport_send_reset(struct vsock_sock *vsk,
+				       struct virtio_vsock_pkt *pkt)
+{
+	struct reset_queue *r;
+
 	/* Send RST only if the original pkt is not a RST pkt */
 	if (le16_to_cpu(pkt->hdr.op) == VIRTIO_VSOCK_OP_RST)
 		return 0;
 
-	return virtio_transport_send_pkt(vsk, &info);
+	r = kmalloc(sizeof(*r), GFP_KERNEL);
+	if (!r)
+		return -ENOMEM;
+
+	r->u.vsk = vsk;
+	r->type = VSOCK_RESET_WITHSOCK;
+
+	spin_lock(&resetqueue_lock);
+	list_add_tail(&r->list, &queued_resets);
+	spin_unlock(&resetqueue_lock);
+
+	schedule_work(&virtio_transport_resets_wq);
+
+	return 0;
 }
 
 /* Normally packets are associated with a socket.  There may be no socket if an
@@ -569,19 +633,36 @@ static int virtio_transport_send_reset_no_sock(struct virtio_vsock_pkt *pkt)
 		.type = le16_to_cpu(pkt->hdr.type),
 	};
 
+	struct reset_queue *r;
+
 	/* Send RST only if the original pkt is not a RST pkt */
 	if (le16_to_cpu(pkt->hdr.op) == VIRTIO_VSOCK_OP_RST)
 		return 0;
 
+	r = kmalloc(sizeof(*r), GFP_KERNEL);
+	if (!r)
+		return -ENOMEM;
+
 	pkt = virtio_transport_alloc_pkt(&info, 0,
 					 le32_to_cpu(pkt->hdr.dst_cid),
 					 le32_to_cpu(pkt->hdr.dst_port),
 					 le32_to_cpu(pkt->hdr.src_cid),
 					 le32_to_cpu(pkt->hdr.src_port));
-	if (!pkt)
+	if (!pkt) {
+		kfree(r);
 		return -ENOMEM;
+	}
 
-	return virtio_transport_send_pkt_no_sock(pkt);
+	r->u.pkt = pkt;
+	r->type = VSOCK_RESET_NOSOCK;
+
+	spin_lock(&resetqueue_lock);
+	list_add_tail(&r->list, &queued_resets);
+	spin_unlock(&resetqueue_lock);
+
+	schedule_work(&virtio_transport_resets_wq);
+
+	return 0;
 }
 
 static int
@@ -830,6 +911,13 @@ void virtio_transport_free_pkt(struct virtio_vsock_pkt *pkt)
 }
 EXPORT_SYMBOL_GPL(virtio_transport_free_pkt);
 
+static void __exit virtio_transport_common_exit(void)
+{
+	cancel_work_sync(&virtio_transport_resets_wq);
+}
+
+module_exit(virtio_transport_common_exit);
+
 MODULE_LICENSE("GPL v2");
 MODULE_AUTHOR("Asias He");
 MODULE_DESCRIPTION("common code for virtio vsock");
-- 
1.9.1



[Date Prev] | [Thread Prev] | [Thread Next] | [Date Next] -- [Date Index] | [Thread Index] | [List Home]