> -----Original Message-----
> From: Maxime Coquelin <maxime.coque...@redhat.com>
> Sent: Monday, July 5, 2021 4:59 PM
> To: Hu, Jiayu <jiayu...@intel.com>; dev@dpdk.org
> Cc: Xia, Chenbo <chenbo....@intel.com>; Wang, Yinan
> <yinan.w...@intel.com>
> Subject: Re: [PATCH 2/2] vhost: add thread unsafe async registration
> functions
> On 5/28/21 10:11 AM, Jiayu Hu wrote:
> > This patch is to add thread unsafe version for async register and
> > unregister functions.
> >
> > Signed-off-by: Jiayu Hu <jiayu...@intel.com>
> > ---
> > 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)
> > +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,
> > VHOST_LOG_CONFIG(ERR,
> > "async register failed: cannot allocate memory for
> async_pkts_info "
> > "(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 "
> > "(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
> > rte_vhost_async_channel_register(int vid, uint16_t queue_id,
> > VHOST_LOG_CONFIG(ERR,
> > "async register failed: cannot allocate memory for
> vec_pool "
> > "(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,
> > VHOST_LOG_CONFIG(ERR,
> > "async register failed: cannot allocate
> memory for async buffers "
> > "(vid %d, qid: %d)\n", vid, queue_id);
> > - goto reg_out;
> > + return -1;
> > }
> > } else {
> > vq->async_descs_split = rte_malloc_socket(NULL, @@ -
> 1715,22
> > +1689,92 @@ int rte_vhost_async_channel_register(int vid, uint16_t
> queue_id,
> > VHOST_LOG_CONFIG(ERR,
> > "async register failed: cannot allocate
> memory for async descs "
> > "(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) {
> > + 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
