-----Original Message-----
From: Maxime Coquelin <redacted>
Sent: Monday, July 5, 2021 4:59 PM
To: Hu, Jiayu <redacted>; dev@dpdk.org
Cc: Xia, Chenbo <redacted>; Wang, Yinan
[off-list ref]
Subject: Re: [PATCH 2/2] vhost: add thread unsafe async registration
functions
On 5/28/21 10:11 AM, Jiayu Hu wrote:

quoted

This patch is to add thread unsafe version for async register and
unregister functions.

Signed-off-by: Jiayu Hu <redacted>
---
 doc/guides/prog_guide/vhost_lib.rst |  12 +++
 lib/vhost/rte_vhost_async.h         |  42 ++++++++++
 lib/vhost/version.map               |   4 +
 lib/vhost/vhost.c                   | 161 +++++++++++++++++++++++++++---------
 4 files changed, 178 insertions(+), 41 deletions(-)

diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c index

c96f633..025e150 100644

--- a/lib/vhost/vhost.c
+++ b/lib/vhost/vhost.c

@@ -1609,46 +1609,20 @@ int rte_vhost_extern_callback_register(int vid,
 	return 0;
 }

-int rte_vhost_async_channel_register(int vid, uint16_t queue_id,
-					uint32_t features,
-					struct rte_vhost_async_channel_ops

*ops)

quoted

+static __rte_always_inline int
+async_channel_register(int vid, uint16_t queue_id,
+		struct rte_vhost_async_features f,
+		struct rte_vhost_async_channel_ops *ops)
 {
-	struct vhost_virtqueue *vq;
 	struct virtio_net *dev = get_device(vid);
-	struct rte_vhost_async_features f;
+	struct vhost_virtqueue *vq = dev->virtqueue[queue_id];
 	int node;

-	if (dev == NULL || ops == NULL)
-		return -1;
-
-	f.intval = features;
-
-	if (queue_id >= VHOST_MAX_VRING)
-		return -1;
-
-	vq = dev->virtqueue[queue_id];
-
-	if (unlikely(vq == NULL || !dev->async_copy))
-		return -1;
-
-	if (unlikely(!f.async_inorder)) {
-		VHOST_LOG_CONFIG(ERR,
-			"async copy is not supported on non-inorder mode "
-			"(vid %d, qid: %d)\n", vid, queue_id);
-		return -1;
-	}
-
-	if (unlikely(ops->check_completed_copies == NULL ||
-		ops->transfer_data == NULL))
-		return -1;
-
-	rte_spinlock_lock(&vq->access_lock);
-
 	if (unlikely(vq->async_registered)) {
 		VHOST_LOG_CONFIG(ERR,
 			"async register failed: channel already registered "
 			"(vid %d, qid: %d)\n", vid, queue_id);
-		goto reg_out;
+		return -1;
 	}

 #ifdef RTE_LIBRTE_VHOST_NUMA

@@ -1670,7 +1644,7 @@ int rte_vhost_async_channel_register(int vid,

uint16_t queue_id,

quoted

 		VHOST_LOG_CONFIG(ERR,
 			"async register failed: cannot allocate memory for

async_pkts_info "

quoted

 			"(vid %d, qid: %d)\n", vid, queue_id);
-		goto reg_out;
+		return -1;
 	}

 	vq->it_pool = rte_malloc_socket(NULL, @@ -1681,7 +1655,7 @@ int
rte_vhost_async_channel_register(int vid, uint16_t queue_id,
 		VHOST_LOG_CONFIG(ERR,
 			"async register failed: cannot allocate memory for

it_pool "

quoted

 			"(vid %d, qid: %d)\n", vid, queue_id);
-		goto reg_out;
+		return -1;
 	}

 	vq->vec_pool = rte_malloc_socket(NULL, @@ -1692,7 +1666,7 @@

int

quoted

rte_vhost_async_channel_register(int vid, uint16_t queue_id,
 		VHOST_LOG_CONFIG(ERR,
 			"async register failed: cannot allocate memory for

vec_pool "

quoted

 			"(vid %d, qid: %d)\n", vid, queue_id);
-		goto reg_out;
+		return -1;
 	}

 	if (vq_is_packed(dev)) {

@@ -1704,7 +1678,7 @@ int rte_vhost_async_channel_register(int vid,

uint16_t queue_id,

quoted

 			VHOST_LOG_CONFIG(ERR,
 				"async register failed: cannot allocate

memory for async buffers "

quoted

 				"(vid %d, qid: %d)\n", vid, queue_id);
-			goto reg_out;
+			return -1;
 		}
 	} else {
 		vq->async_descs_split = rte_malloc_socket(NULL, @@ -

1715,22

quoted

+1689,92 @@ int rte_vhost_async_channel_register(int vid, uint16_t

queue_id,

quoted

 			VHOST_LOG_CONFIG(ERR,
 				"async register failed: cannot allocate

memory for async descs "

quoted

 				"(vid %d, qid: %d)\n", vid, queue_id);
-			goto reg_out;
+			return -1;
 		}
 	}

 	vq->async_ops.check_completed_copies = ops-
check_completed_copies;
 	vq->async_ops.transfer_data = ops->transfer_data;
-
 	vq->async_inorder = f.async_inorder;
 	vq->async_threshold = f.async_threshold;
-
 	vq->async_registered = true;

-reg_out:
+	return 0;
+}
+
+int rte_vhost_async_channel_register(int vid, uint16_t queue_id,
+					uint32_t features,
+					struct rte_vhost_async_channel_ops

*ops) {

quoted

+	struct vhost_virtqueue *vq;
+	struct virtio_net *dev = get_device(vid);
+	struct rte_vhost_async_features f;
+	int ret;
+
+	if (dev == NULL || ops == NULL)
+		return -1;
+
+	f.intval = features;

Not directly related to this patch set, but could you please rework struct
rte_vhost_async_features? There is no point to pack the flags and threshold
values.

Also, the prototype should just pass the struct directly, or add different fields
for the threshold and the features.

Will rework the structure in v2.

Thanks,
Jiayu