From patchwork Fri Jul 22 13:53:19 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Marchand X-Patchwork-Id: 114116 X-Patchwork-Delegate: maxime.coquelin@redhat.com Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id AAEEBA0032; Fri, 22 Jul 2022 15:53:51 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 00775427F9; Fri, 22 Jul 2022 15:53:50 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id 3168C427F9 for ; Fri, 22 Jul 2022 15:53:48 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1658498027; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=LM77dc1Ae4RaKHCXX5LvcPKOMudjTOcncQQa7n+W5Y8=; b=AoiexMbE0IKsYVmyVUzqZBrTJx4Ik7uaq90zREujk0IPtOfBdiQUj80NcSH9YSQP+mG9HC vTDOWg0VEwq9Qyu4pyFAfsA7LvSh+L9hezLuRBIOF4rx+T28Cf2OHz/AKh61JkITu0/2jp D9Nr/nlBCZz7XxJvJ3vAhj26m0cAN6U= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-470--FUgTvuSPwyyD_o4ElLydA-1; Fri, 22 Jul 2022 09:53:38 -0400 X-MC-Unique: -FUgTvuSPwyyD_o4ElLydA-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id C49EF811E81; Fri, 22 Jul 2022 13:53:37 +0000 (UTC) Received: from localhost.localdomain (unknown [10.40.192.6]) by smtp.corp.redhat.com (Postfix) with ESMTP id CBE112166B26; Fri, 22 Jul 2022 13:53:36 +0000 (UTC) From: David Marchand To: dev@dpdk.org Cc: Maxime Coquelin , Chenbo Xia Subject: [PATCH 1/2] vhost: keep a reference to virtqueue index Date: Fri, 22 Jul 2022 15:53:19 +0200 Message-Id: <20220722135320.109269-1-david.marchand@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.78 on 10.11.54.6 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=david.marchand@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Having a back reference to the index of the vq in the dev->virtqueue[] array makes it possible to unify the internal API with only passing dev and vq. It also allows displaying the vq index in log messages. Remove virtqueue index checks were unneeded (like in static helpers called from a loop on all available virtqueue). Move virtqueue index validity checks the sooner possible. Signed-off-by: David Marchand --- lib/vhost/iotlb.c | 5 +-- lib/vhost/iotlb.h | 2 +- lib/vhost/vhost.c | 74 ++++++++++++++---------------------- lib/vhost/vhost.h | 3 ++ lib/vhost/vhost_user.c | 58 +++++++++++++--------------- lib/vhost/virtio_net.c | 86 +++++++++++++++++++----------------------- 6 files changed, 98 insertions(+), 130 deletions(-) diff --git a/lib/vhost/iotlb.c b/lib/vhost/iotlb.c index 35b4193606..dd35338ec0 100644 --- a/lib/vhost/iotlb.c +++ b/lib/vhost/iotlb.c @@ -293,10 +293,9 @@ vhost_user_iotlb_flush_all(struct vhost_virtqueue *vq) } int -vhost_user_iotlb_init(struct virtio_net *dev, int vq_index) +vhost_user_iotlb_init(struct virtio_net *dev, struct vhost_virtqueue *vq) { char pool_name[RTE_MEMPOOL_NAMESIZE]; - struct vhost_virtqueue *vq = dev->virtqueue[vq_index]; int socket = 0; if (vq->iotlb_pool) { @@ -319,7 +318,7 @@ vhost_user_iotlb_init(struct virtio_net *dev, int vq_index) TAILQ_INIT(&vq->iotlb_pending_list); snprintf(pool_name, sizeof(pool_name), "iotlb_%u_%d_%d", - getpid(), dev->vid, vq_index); + getpid(), dev->vid, vq->index); VHOST_LOG_CONFIG(dev->ifname, DEBUG, "IOTLB cache name: %s\n", pool_name); /* If already created, free it and recreate */ diff --git a/lib/vhost/iotlb.h b/lib/vhost/iotlb.h index 8d0ff7473b..738e31e7b9 100644 --- a/lib/vhost/iotlb.h +++ b/lib/vhost/iotlb.h @@ -47,6 +47,6 @@ void vhost_user_iotlb_pending_insert(struct virtio_net *dev, struct vhost_virtqu void vhost_user_iotlb_pending_remove(struct vhost_virtqueue *vq, uint64_t iova, uint64_t size, uint8_t perm); void vhost_user_iotlb_flush_all(struct vhost_virtqueue *vq); -int vhost_user_iotlb_init(struct virtio_net *dev, int vq_index); +int vhost_user_iotlb_init(struct virtio_net *dev, struct vhost_virtqueue *vq); #endif /* _VHOST_IOTLB_H_ */ diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c index 60cb05a0ff..97bae0de91 100644 --- a/lib/vhost/vhost.c +++ b/lib/vhost/vhost.c @@ -575,23 +575,10 @@ vring_invalidate(struct virtio_net *dev, struct vhost_virtqueue *vq) } static void -init_vring_queue(struct virtio_net *dev, uint32_t vring_idx) +init_vring_queue(struct virtio_net *dev, struct vhost_virtqueue *vq) { - struct vhost_virtqueue *vq; int numa_node = SOCKET_ID_ANY; - if (vring_idx >= VHOST_MAX_VRING) { - VHOST_LOG_CONFIG(dev->ifname, ERR, "failed to init vring, out of bound (%d)\n", - vring_idx); - return; - } - - vq = dev->virtqueue[vring_idx]; - if (!vq) { - VHOST_LOG_CONFIG(dev->ifname, ERR, "virtqueue not allocated (%d)\n", vring_idx); - return; - } - memset(vq, 0, sizeof(struct vhost_virtqueue)); vq->kickfd = VIRTIO_UNINITIALIZED_EVENTFD; @@ -607,32 +594,20 @@ init_vring_queue(struct virtio_net *dev, uint32_t vring_idx) #endif vq->numa_node = numa_node; - vhost_user_iotlb_init(dev, vring_idx); + vhost_user_iotlb_init(dev, vq); } static void -reset_vring_queue(struct virtio_net *dev, uint32_t vring_idx) +reset_vring_queue(struct virtio_net *dev, struct vhost_virtqueue *vq) { - struct vhost_virtqueue *vq; + uint32_t vring_idx; int callfd; - if (vring_idx >= VHOST_MAX_VRING) { - VHOST_LOG_CONFIG(dev->ifname, ERR, "failed to reset vring, out of bound (%d)\n", - vring_idx); - return; - } - - vq = dev->virtqueue[vring_idx]; - if (!vq) { - VHOST_LOG_CONFIG(dev->ifname, ERR, - "failed to reset vring, virtqueue not allocated (%d)\n", - vring_idx); - return; - } - callfd = vq->callfd; - init_vring_queue(dev, vring_idx); + vring_idx = vq->index; + init_vring_queue(dev, vq); vq->callfd = callfd; + vq->index = vring_idx; } int @@ -655,8 +630,9 @@ alloc_vring_queue(struct virtio_net *dev, uint32_t vring_idx) } dev->virtqueue[i] = vq; - init_vring_queue(dev, i); + init_vring_queue(dev, vq); rte_spinlock_init(&vq->access_lock); + vq->index = vring_idx; vq->avail_wrap_counter = 1; vq->used_wrap_counter = 1; vq->signalled_used_valid = false; @@ -681,8 +657,16 @@ reset_device(struct virtio_net *dev) dev->protocol_features = 0; dev->flags &= VIRTIO_DEV_BUILTIN_VIRTIO_NET; - for (i = 0; i < dev->nr_vring; i++) - reset_vring_queue(dev, i); + for (i = 0; i < dev->nr_vring; i++) { + struct vhost_virtqueue *vq = dev->virtqueue[i]; + + if (!vq) { + VHOST_LOG_CONFIG(dev->ifname, ERR, + "failed to reset vring, virtqueue not allocated (%d)\n", i); + continue; + } + reset_vring_queue(dev, vq); + } } /* @@ -1661,17 +1645,15 @@ rte_vhost_extern_callback_register(int vid, } static __rte_always_inline int -async_channel_register(int vid, uint16_t queue_id) +async_channel_register(struct virtio_net *dev, struct vhost_virtqueue *vq) { - struct virtio_net *dev = get_device(vid); - struct vhost_virtqueue *vq = dev->virtqueue[queue_id]; struct vhost_async *async; int node = vq->numa_node; if (unlikely(vq->async)) { VHOST_LOG_CONFIG(dev->ifname, ERR, "async register failed: already registered (qid: %d)\n", - queue_id); + vq->index); return -1; } @@ -1679,7 +1661,7 @@ async_channel_register(int vid, uint16_t queue_id) if (!async) { VHOST_LOG_CONFIG(dev->ifname, ERR, "failed to allocate async metadata (qid: %d)\n", - queue_id); + vq->index); return -1; } @@ -1688,7 +1670,7 @@ async_channel_register(int vid, uint16_t queue_id) if (!async->pkts_info) { VHOST_LOG_CONFIG(dev->ifname, ERR, "failed to allocate async_pkts_info (qid: %d)\n", - queue_id); + vq->index); goto out_free_async; } @@ -1697,7 +1679,7 @@ async_channel_register(int vid, uint16_t queue_id) if (!async->pkts_cmpl_flag) { VHOST_LOG_CONFIG(dev->ifname, ERR, "failed to allocate async pkts_cmpl_flag (qid: %d)\n", - queue_id); + vq->index); goto out_free_async; } @@ -1708,7 +1690,7 @@ async_channel_register(int vid, uint16_t queue_id) if (!async->buffers_packed) { VHOST_LOG_CONFIG(dev->ifname, ERR, "failed to allocate async buffers (qid: %d)\n", - queue_id); + vq->index); goto out_free_inflight; } } else { @@ -1718,7 +1700,7 @@ async_channel_register(int vid, uint16_t queue_id) if (!async->descs_split) { VHOST_LOG_CONFIG(dev->ifname, ERR, "failed to allocate async descs (qid: %d)\n", - queue_id); + vq->index); goto out_free_inflight; } } @@ -1753,7 +1735,7 @@ rte_vhost_async_channel_register(int vid, uint16_t queue_id) return -1; rte_spinlock_lock(&vq->access_lock); - ret = async_channel_register(vid, queue_id); + ret = async_channel_register(dev, vq); rte_spinlock_unlock(&vq->access_lock); return ret; @@ -1782,7 +1764,7 @@ rte_vhost_async_channel_register_thread_unsafe(int vid, uint16_t queue_id) return -1; } - return async_channel_register(vid, queue_id); + return async_channel_register(dev, vq); } int diff --git a/lib/vhost/vhost.h b/lib/vhost/vhost.h index 40fac3b7c6..c6260b54cc 100644 --- a/lib/vhost/vhost.h +++ b/lib/vhost/vhost.h @@ -309,6 +309,9 @@ struct vhost_virtqueue { /* Currently unused as polling mode is enabled */ int kickfd; + /* Index of this vq in dev->virtqueue[] */ + uint32_t index; + /* inflight share memory info */ union { struct rte_vhost_inflight_info_split *inflight_split; diff --git a/lib/vhost/vhost_user.c b/lib/vhost/vhost_user.c index 4ad28bac45..73e69450fd 100644 --- a/lib/vhost/vhost_user.c +++ b/lib/vhost/vhost_user.c @@ -240,22 +240,20 @@ vhost_backend_cleanup(struct virtio_net *dev) } static void -vhost_user_notify_queue_state(struct virtio_net *dev, uint16_t index, - int enable) +vhost_user_notify_queue_state(struct virtio_net *dev, struct vhost_virtqueue *vq, + int enable) { struct rte_vdpa_device *vdpa_dev = dev->vdpa_dev; - struct vhost_virtqueue *vq = dev->virtqueue[index]; /* Configure guest notifications on enable */ if (enable && vq->notif_enable != VIRTIO_UNINITIALIZED_NOTIF) vhost_enable_guest_notification(dev, vq, vq->notif_enable); if (vdpa_dev && vdpa_dev->ops->set_vring_state) - vdpa_dev->ops->set_vring_state(dev->vid, index, enable); + vdpa_dev->ops->set_vring_state(dev->vid, vq->index, enable); if (dev->notify_ops->vring_state_changed) - dev->notify_ops->vring_state_changed(dev->vid, - index, enable); + dev->notify_ops->vring_state_changed(dev->vid, vq->index, enable); } /* @@ -493,12 +491,11 @@ vhost_user_set_vring_num(struct virtio_net **pdev, * make them on the same numa node as the memory of vring descriptor. */ #ifdef RTE_LIBRTE_VHOST_NUMA -static struct virtio_net* -numa_realloc(struct virtio_net *dev, int index) +static struct virtio_net * +numa_realloc(struct virtio_net *dev, struct vhost_virtqueue *vq) { int node, dev_node; struct virtio_net *old_dev; - struct vhost_virtqueue *vq; struct batch_copy_elem *bce; struct guest_page *gp; struct rte_vhost_memory *mem; @@ -506,7 +503,6 @@ numa_realloc(struct virtio_net *dev, int index) int ret; old_dev = dev; - vq = dev->virtqueue[index]; /* * If VQ is ready, it is too late to reallocate, it certainly already @@ -519,7 +515,7 @@ numa_realloc(struct virtio_net *dev, int index) if (ret) { VHOST_LOG_CONFIG(dev->ifname, ERR, "unable to get virtqueue %d numa information.\n", - index); + vq->index); return dev; } @@ -530,14 +526,14 @@ numa_realloc(struct virtio_net *dev, int index) if (!vq) { VHOST_LOG_CONFIG(dev->ifname, ERR, "failed to realloc virtqueue %d on node %d\n", - index, node); + vq->index, node); return dev; } - if (vq != dev->virtqueue[index]) { + if (vq != dev->virtqueue[vq->index]) { VHOST_LOG_CONFIG(dev->ifname, INFO, "reallocated virtqueue on node %d\n", node); - dev->virtqueue[index] = vq; - vhost_user_iotlb_init(dev, index); + dev->virtqueue[vq->index] = vq; + vhost_user_iotlb_init(dev, vq); } if (vq_is_packed(dev)) { @@ -665,8 +661,8 @@ numa_realloc(struct virtio_net *dev, int index) return dev; } #else -static struct virtio_net* -numa_realloc(struct virtio_net *dev, int index __rte_unused) +static struct virtio_net * +numa_realloc(struct virtio_net *dev, struct vhost_virtqueue *vq __rte_unused) { return dev; } @@ -739,9 +735,8 @@ log_addr_to_gpa(struct virtio_net *dev, struct vhost_virtqueue *vq) } static struct virtio_net * -translate_ring_addresses(struct virtio_net *dev, int vq_index) +translate_ring_addresses(struct virtio_net *dev, struct vhost_virtqueue *vq) { - struct vhost_virtqueue *vq = dev->virtqueue[vq_index]; struct vhost_vring_addr *addr = &vq->ring_addrs; uint64_t len, expected_len; @@ -765,8 +760,8 @@ translate_ring_addresses(struct virtio_net *dev, int vq_index) return dev; } - dev = numa_realloc(dev, vq_index); - vq = dev->virtqueue[vq_index]; + dev = numa_realloc(dev, vq); + vq = dev->virtqueue[vq->index]; addr = &vq->ring_addrs; len = sizeof(struct vring_packed_desc_event); @@ -807,8 +802,8 @@ translate_ring_addresses(struct virtio_net *dev, int vq_index) return dev; } - dev = numa_realloc(dev, vq_index); - vq = dev->virtqueue[vq_index]; + dev = numa_realloc(dev, vq); + vq = dev->virtqueue[vq->index]; addr = &vq->ring_addrs; len = sizeof(struct vring_avail) + sizeof(uint16_t) * vq->size; @@ -887,7 +882,7 @@ vhost_user_set_vring_addr(struct virtio_net **pdev, if ((vq->enabled && (dev->features & (1ULL << VHOST_USER_F_PROTOCOL_FEATURES))) || access_ok) { - dev = translate_ring_addresses(dev, ctx->msg.payload.addr.index); + dev = translate_ring_addresses(dev, vq); if (!dev) return RTE_VHOST_MSG_RESULT_ERR; @@ -1396,7 +1391,7 @@ vhost_user_set_mem_table(struct virtio_net **pdev, */ vring_invalidate(dev, vq); - dev = translate_ring_addresses(dev, i); + dev = translate_ring_addresses(dev, vq); if (!dev) { dev = *pdev; goto free_mem_table; @@ -1781,7 +1776,7 @@ vhost_user_set_vring_call(struct virtio_net **pdev, if (vq->ready) { vq->ready = false; - vhost_user_notify_queue_state(dev, file.index, 0); + vhost_user_notify_queue_state(dev, vq, 0); } if (vq->callfd >= 0) @@ -2029,7 +2024,8 @@ vhost_user_set_vring_kick(struct virtio_net **pdev, file.index, file.fd); /* Interpret ring addresses only when ring is started. */ - dev = translate_ring_addresses(dev, file.index); + vq = dev->virtqueue[file.index]; + dev = translate_ring_addresses(dev, vq); if (!dev) { if (file.fd != VIRTIO_INVALID_EVENTFD) close(file.fd); @@ -2039,8 +2035,6 @@ vhost_user_set_vring_kick(struct virtio_net **pdev, *pdev = dev; - vq = dev->virtqueue[file.index]; - /* * When VHOST_USER_F_PROTOCOL_FEATURES is not negotiated, * the ring starts already enabled. Otherwise, it is enabled via @@ -2052,7 +2046,7 @@ vhost_user_set_vring_kick(struct virtio_net **pdev, if (vq->ready) { vq->ready = false; - vhost_user_notify_queue_state(dev, file.index, 0); + vhost_user_notify_queue_state(dev, vq, 0); } if (vq->kickfd >= 0) @@ -2595,7 +2589,7 @@ vhost_user_iotlb_msg(struct virtio_net **pdev, if (is_vring_iotlb(dev, vq, imsg)) { rte_spinlock_lock(&vq->access_lock); - *pdev = dev = translate_ring_addresses(dev, i); + *pdev = dev = translate_ring_addresses(dev, vq); rte_spinlock_unlock(&vq->access_lock); } } @@ -3159,7 +3153,7 @@ vhost_user_msg_handler(int vid, int fd) if (cur_ready != (vq && vq->ready)) { vq->ready = cur_ready; - vhost_user_notify_queue_state(dev, i, cur_ready); + vhost_user_notify_queue_state(dev, vq, cur_ready); } } diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c index 35fa4670fd..467dfb203f 100644 --- a/lib/vhost/virtio_net.c +++ b/lib/vhost/virtio_net.c @@ -1555,22 +1555,12 @@ virtio_dev_rx_packed(struct virtio_net *dev, } static __rte_always_inline uint32_t -virtio_dev_rx(struct virtio_net *dev, uint16_t queue_id, +virtio_dev_rx(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mbuf **pkts, uint32_t count) { - struct vhost_virtqueue *vq; uint32_t nb_tx = 0; VHOST_LOG_DATA(dev->ifname, DEBUG, "%s\n", __func__); - if (unlikely(!is_valid_virt_queue_idx(queue_id, 0, dev->nr_vring))) { - VHOST_LOG_DATA(dev->ifname, ERR, - "%s: invalid virtqueue idx %d.\n", - __func__, queue_id); - return 0; - } - - vq = dev->virtqueue[queue_id]; - rte_spinlock_lock(&vq->access_lock); if (unlikely(!vq->enabled)) @@ -1620,7 +1610,14 @@ rte_vhost_enqueue_burst(int vid, uint16_t queue_id, return 0; } - return virtio_dev_rx(dev, queue_id, pkts, count); + if (unlikely(!is_valid_virt_queue_idx(queue_id, 0, dev->nr_vring))) { + VHOST_LOG_DATA(dev->ifname, ERR, + "%s: invalid virtqueue idx %d.\n", + __func__, queue_id); + return 0; + } + + return virtio_dev_rx(dev, dev->virtqueue[queue_id], pkts, count); } static __rte_always_inline uint16_t @@ -1669,8 +1666,7 @@ store_dma_desc_info_packed(struct vring_used_elem_packed *s_ring, static __rte_noinline uint32_t virtio_dev_rx_async_submit_split(struct virtio_net *dev, struct vhost_virtqueue *vq, - uint16_t queue_id, struct rte_mbuf **pkts, uint32_t count, - int16_t dma_id, uint16_t vchan_id) + struct rte_mbuf **pkts, uint32_t count, int16_t dma_id, uint16_t vchan_id) { struct buf_vector buf_vec[BUF_VECTOR_MAX]; uint32_t pkt_idx = 0; @@ -1732,7 +1728,7 @@ virtio_dev_rx_async_submit_split(struct virtio_net *dev, struct vhost_virtqueue VHOST_LOG_DATA(dev->ifname, DEBUG, "%s: failed to transfer %u packets for queue %u.\n", - __func__, pkt_err, queue_id); + __func__, pkt_err, vq->index); /* update number of completed packets */ pkt_idx = n_xfer; @@ -1878,8 +1874,7 @@ dma_error_handler_packed(struct vhost_virtqueue *vq, uint16_t slot_idx, static __rte_noinline uint32_t virtio_dev_rx_async_submit_packed(struct virtio_net *dev, struct vhost_virtqueue *vq, - uint16_t queue_id, struct rte_mbuf **pkts, uint32_t count, - int16_t dma_id, uint16_t vchan_id) + struct rte_mbuf **pkts, uint32_t count, int16_t dma_id, uint16_t vchan_id) { uint32_t pkt_idx = 0; uint32_t remained = count; @@ -1924,7 +1919,7 @@ virtio_dev_rx_async_submit_packed(struct virtio_net *dev, struct vhost_virtqueue if (unlikely(pkt_err)) { VHOST_LOG_DATA(dev->ifname, DEBUG, "%s: failed to transfer %u packets for queue %u.\n", - __func__, pkt_err, queue_id); + __func__, pkt_err, vq->index); dma_error_handler_packed(vq, slot_idx, pkt_err, &pkt_idx); } @@ -2045,11 +2040,9 @@ write_back_completed_descs_packed(struct vhost_virtqueue *vq, } static __rte_always_inline uint16_t -vhost_poll_enqueue_completed(struct virtio_net *dev, uint16_t queue_id, - struct rte_mbuf **pkts, uint16_t count, int16_t dma_id, - uint16_t vchan_id) +vhost_poll_enqueue_completed(struct virtio_net *dev, struct vhost_virtqueue *vq, + struct rte_mbuf **pkts, uint16_t count, int16_t dma_id, uint16_t vchan_id) { - struct vhost_virtqueue *vq = dev->virtqueue[queue_id]; struct vhost_async *async = vq->async; struct async_inflight_info *pkts_info = async->pkts_info; uint16_t nr_cpl_pkts = 0; @@ -2156,7 +2149,7 @@ rte_vhost_poll_enqueue_completed(int vid, uint16_t queue_id, goto out; } - n_pkts_cpl = vhost_poll_enqueue_completed(dev, queue_id, pkts, count, dma_id, vchan_id); + n_pkts_cpl = vhost_poll_enqueue_completed(dev, vq, pkts, count, dma_id, vchan_id); vhost_queue_stats_update(dev, vq, pkts, n_pkts_cpl); vq->stats.inflight_completed += n_pkts_cpl; @@ -2216,12 +2209,11 @@ rte_vhost_clear_queue_thread_unsafe(int vid, uint16_t queue_id, } if ((queue_id & 1) == 0) - n_pkts_cpl = vhost_poll_enqueue_completed(dev, queue_id, - pkts, count, dma_id, vchan_id); - else { + n_pkts_cpl = vhost_poll_enqueue_completed(dev, vq, pkts, count, + dma_id, vchan_id); + else n_pkts_cpl = async_poll_dequeue_completed(dev, vq, pkts, count, - dma_id, vchan_id, dev->flags & VIRTIO_DEV_LEGACY_OL_FLAGS); - } + dma_id, vchan_id, dev->flags & VIRTIO_DEV_LEGACY_OL_FLAGS); vhost_queue_stats_update(dev, vq, pkts, n_pkts_cpl); vq->stats.inflight_completed += n_pkts_cpl; @@ -2275,12 +2267,11 @@ rte_vhost_clear_queue(int vid, uint16_t queue_id, struct rte_mbuf **pkts, } if ((queue_id & 1) == 0) - n_pkts_cpl = vhost_poll_enqueue_completed(dev, queue_id, - pkts, count, dma_id, vchan_id); - else { + n_pkts_cpl = vhost_poll_enqueue_completed(dev, vq, pkts, count, + dma_id, vchan_id); + else n_pkts_cpl = async_poll_dequeue_completed(dev, vq, pkts, count, - dma_id, vchan_id, dev->flags & VIRTIO_DEV_LEGACY_OL_FLAGS); - } + dma_id, vchan_id, dev->flags & VIRTIO_DEV_LEGACY_OL_FLAGS); vhost_queue_stats_update(dev, vq, pkts, n_pkts_cpl); vq->stats.inflight_completed += n_pkts_cpl; @@ -2292,19 +2283,12 @@ rte_vhost_clear_queue(int vid, uint16_t queue_id, struct rte_mbuf **pkts, } static __rte_always_inline uint32_t -virtio_dev_rx_async_submit(struct virtio_net *dev, uint16_t queue_id, +virtio_dev_rx_async_submit(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mbuf **pkts, uint32_t count, int16_t dma_id, uint16_t vchan_id) { - struct vhost_virtqueue *vq; uint32_t nb_tx = 0; VHOST_LOG_DATA(dev->ifname, DEBUG, "%s\n", __func__); - if (unlikely(!is_valid_virt_queue_idx(queue_id, 0, dev->nr_vring))) { - VHOST_LOG_DATA(dev->ifname, ERR, - "%s: invalid virtqueue idx %d.\n", - __func__, queue_id); - return 0; - } if (unlikely(!dma_copy_track[dma_id].vchans || !dma_copy_track[dma_id].vchans[vchan_id].pkts_cmpl_flag_addr)) { @@ -2314,8 +2298,6 @@ virtio_dev_rx_async_submit(struct virtio_net *dev, uint16_t queue_id, return 0; } - vq = dev->virtqueue[queue_id]; - rte_spinlock_lock(&vq->access_lock); if (unlikely(!vq->enabled || !vq->async)) @@ -2333,11 +2315,11 @@ virtio_dev_rx_async_submit(struct virtio_net *dev, uint16_t queue_id, goto out; if (vq_is_packed(dev)) - nb_tx = virtio_dev_rx_async_submit_packed(dev, vq, queue_id, - pkts, count, dma_id, vchan_id); + nb_tx = virtio_dev_rx_async_submit_packed(dev, vq, pkts, count, + dma_id, vchan_id); else - nb_tx = virtio_dev_rx_async_submit_split(dev, vq, queue_id, - pkts, count, dma_id, vchan_id); + nb_tx = virtio_dev_rx_async_submit_split(dev, vq, pkts, count, + dma_id, vchan_id); vq->stats.inflight_submitted += nb_tx; @@ -2368,7 +2350,15 @@ rte_vhost_submit_enqueue_burst(int vid, uint16_t queue_id, return 0; } - return virtio_dev_rx_async_submit(dev, queue_id, pkts, count, dma_id, vchan_id); + if (unlikely(!is_valid_virt_queue_idx(queue_id, 0, dev->nr_vring))) { + VHOST_LOG_DATA(dev->ifname, ERR, + "%s: invalid virtqueue idx %d.\n", + __func__, queue_id); + return 0; + } + + return virtio_dev_rx_async_submit(dev, dev->virtqueue[queue_id], pkts, count, + dma_id, vchan_id); } static inline bool From patchwork Fri Jul 22 13:53:20 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Marchand X-Patchwork-Id: 114115 X-Patchwork-Delegate: maxime.coquelin@redhat.com Return-Path: X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 7EF3EA0032; Fri, 22 Jul 2022 15:53:46 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 24F484021E; Fri, 22 Jul 2022 15:53:46 +0200 (CEST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mails.dpdk.org (Postfix) with ESMTP id 7DFCA40156 for ; Fri, 22 Jul 2022 15:53:44 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1658498023; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=bPGojHVZ/qzky+wfxTYwg08TZZbkv6cDHmL/0rMVpvg=; b=COpT+vspMATpZu0/7/dlHKLv/uCuo+6n4ZJ3BIwO4ny4GHcnAlWiOwTxU0IHa7wI+jDe94 j2k9rADF5QahnmsKQfdHkK8Hb9ii87CRdklJquQLabl2TgkfTBjl41fCjLXq+Haf6CxoGY FyN0vCaGXKe0SG1xdaufUR2Fdk7jSFI= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-554-XkP_8-MxMQWed4qXjrZQwg-1; Fri, 22 Jul 2022 09:53:40 -0400 X-MC-Unique: XkP_8-MxMQWed4qXjrZQwg-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 9FC833C10684; Fri, 22 Jul 2022 13:53:40 +0000 (UTC) Received: from localhost.localdomain (unknown [10.40.192.6]) by smtp.corp.redhat.com (Postfix) with ESMTP id D89662166B26; Fri, 22 Jul 2022 13:53:39 +0000 (UTC) From: David Marchand To: dev@dpdk.org Cc: Maxime Coquelin , Chenbo Xia Subject: [PATCH 2/2] vhost: stop using mempool for IOTLB Date: Fri, 22 Jul 2022 15:53:20 +0200 Message-Id: <20220722135320.109269-2-david.marchand@redhat.com> In-Reply-To: <20220722135320.109269-1-david.marchand@redhat.com> References: <20220722135320.109269-1-david.marchand@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.78 on 10.11.54.6 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=david.marchand@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org A mempool consumes 3 memzones (with the default ring mempool driver). Default DPDK configuration allows RTE_MAX_MEMZONE (2560) memzones. Assuming there is no other memzones (well, there is always other memzones in a DPDK application), that means that we can have a maximum of 853 mempools. The IOTLB so far was requesting a mempool per vq, which means that at the maximum, the vhost library can request mempools for 426 qps. This limit was recently reached on big systems with a lot of virtio ports (and multiqueue in use). While the limit on mempool count could be something we need to look at the DPDK project level, there is no reason to use mempools with the IOTLB code. The IOTLB cache entries do not need to be DMA-able and are only used by the current process (in multiprocess context). Getting/putting objects from/in the mempool is always associated with some other locks, so some level of contention is already present. We can convert to a malloc'd pool with objects put in a free list protected by a spinlock. Signed-off-by: David Marchand --- lib/vhost/iotlb.c | 102 ++++++++++++++++++++++++++++------------------ lib/vhost/iotlb.h | 1 + lib/vhost/vhost.c | 2 +- lib/vhost/vhost.h | 4 +- 4 files changed, 67 insertions(+), 42 deletions(-) diff --git a/lib/vhost/iotlb.c b/lib/vhost/iotlb.c index dd35338ec0..2a78929e78 100644 --- a/lib/vhost/iotlb.c +++ b/lib/vhost/iotlb.c @@ -13,6 +13,7 @@ struct vhost_iotlb_entry { TAILQ_ENTRY(vhost_iotlb_entry) next; + SLIST_ENTRY(vhost_iotlb_entry) next_free; uint64_t iova; uint64_t uaddr; @@ -22,6 +23,28 @@ struct vhost_iotlb_entry { #define IOTLB_CACHE_SIZE 2048 +static struct vhost_iotlb_entry * +vhost_user_iotlb_pool_get(struct vhost_virtqueue *vq) +{ + struct vhost_iotlb_entry *node; + + rte_spinlock_lock(&vq->iotlb_free_lock); + node = SLIST_FIRST(&vq->iotlb_free_list); + if (node != NULL) + SLIST_REMOVE_HEAD(&vq->iotlb_free_list, next_free); + rte_spinlock_unlock(&vq->iotlb_free_lock); + return node; +} + +static void +vhost_user_iotlb_pool_put(struct vhost_virtqueue *vq, + struct vhost_iotlb_entry *node) +{ + rte_spinlock_lock(&vq->iotlb_free_lock); + SLIST_INSERT_HEAD(&vq->iotlb_free_list, node, next_free); + rte_spinlock_unlock(&vq->iotlb_free_lock); +} + static void vhost_user_iotlb_cache_random_evict(struct vhost_virtqueue *vq); @@ -34,7 +57,7 @@ vhost_user_iotlb_pending_remove_all(struct vhost_virtqueue *vq) RTE_TAILQ_FOREACH_SAFE(node, &vq->iotlb_pending_list, next, temp_node) { TAILQ_REMOVE(&vq->iotlb_pending_list, node, next); - rte_mempool_put(vq->iotlb_pool, node); + vhost_user_iotlb_pool_put(vq, node); } rte_rwlock_write_unlock(&vq->iotlb_pending_lock); @@ -66,22 +89,21 @@ vhost_user_iotlb_pending_insert(struct virtio_net *dev, struct vhost_virtqueue * uint64_t iova, uint8_t perm) { struct vhost_iotlb_entry *node; - int ret; - ret = rte_mempool_get(vq->iotlb_pool, (void **)&node); - if (ret) { + node = vhost_user_iotlb_pool_get(vq); + if (node == NULL) { VHOST_LOG_CONFIG(dev->ifname, DEBUG, - "IOTLB pool %s empty, clear entries for pending insertion\n", - vq->iotlb_pool->name); + "IOTLB pool for vq %"PRIu32" empty, clear entries for pending insertion\n", + vq->index); if (!TAILQ_EMPTY(&vq->iotlb_pending_list)) vhost_user_iotlb_pending_remove_all(vq); else vhost_user_iotlb_cache_random_evict(vq); - ret = rte_mempool_get(vq->iotlb_pool, (void **)&node); - if (ret) { + node = vhost_user_iotlb_pool_get(vq); + if (node == NULL) { VHOST_LOG_CONFIG(dev->ifname, ERR, - "IOTLB pool %s still empty, pending insertion failure\n", - vq->iotlb_pool->name); + "IOTLB pool vq %"PRIu32" still empty, pending insertion failure\n", + vq->index); return; } } @@ -113,7 +135,7 @@ vhost_user_iotlb_pending_remove(struct vhost_virtqueue *vq, if ((node->perm & perm) != node->perm) continue; TAILQ_REMOVE(&vq->iotlb_pending_list, node, next); - rte_mempool_put(vq->iotlb_pool, node); + vhost_user_iotlb_pool_put(vq, node); } rte_rwlock_write_unlock(&vq->iotlb_pending_lock); @@ -128,7 +150,7 @@ vhost_user_iotlb_cache_remove_all(struct vhost_virtqueue *vq) RTE_TAILQ_FOREACH_SAFE(node, &vq->iotlb_list, next, temp_node) { TAILQ_REMOVE(&vq->iotlb_list, node, next); - rte_mempool_put(vq->iotlb_pool, node); + vhost_user_iotlb_pool_put(vq, node); } vq->iotlb_cache_nr = 0; @@ -149,7 +171,7 @@ vhost_user_iotlb_cache_random_evict(struct vhost_virtqueue *vq) RTE_TAILQ_FOREACH_SAFE(node, &vq->iotlb_list, next, temp_node) { if (!entry_idx) { TAILQ_REMOVE(&vq->iotlb_list, node, next); - rte_mempool_put(vq->iotlb_pool, node); + vhost_user_iotlb_pool_put(vq, node); vq->iotlb_cache_nr--; break; } @@ -165,22 +187,21 @@ vhost_user_iotlb_cache_insert(struct virtio_net *dev, struct vhost_virtqueue *vq uint64_t size, uint8_t perm) { struct vhost_iotlb_entry *node, *new_node; - int ret; - ret = rte_mempool_get(vq->iotlb_pool, (void **)&new_node); - if (ret) { + new_node = vhost_user_iotlb_pool_get(vq); + if (new_node == NULL) { VHOST_LOG_CONFIG(dev->ifname, DEBUG, - "IOTLB pool %s empty, clear entries for cache insertion\n", - vq->iotlb_pool->name); + "IOTLB pool vq %"PRIu32" empty, clear entries for cache insertion\n", + vq->index); if (!TAILQ_EMPTY(&vq->iotlb_list)) vhost_user_iotlb_cache_random_evict(vq); else vhost_user_iotlb_pending_remove_all(vq); - ret = rte_mempool_get(vq->iotlb_pool, (void **)&new_node); - if (ret) { + new_node = vhost_user_iotlb_pool_get(vq); + if (new_node == NULL) { VHOST_LOG_CONFIG(dev->ifname, ERR, - "IOTLB pool %s still empty, cache insertion failed\n", - vq->iotlb_pool->name); + "IOTLB pool vq %"PRIu32" still empty, cache insertion failed\n", + vq->index); return; } } @@ -198,7 +219,7 @@ vhost_user_iotlb_cache_insert(struct virtio_net *dev, struct vhost_virtqueue *vq * So if iova already in list, assume identical. */ if (node->iova == new_node->iova) { - rte_mempool_put(vq->iotlb_pool, new_node); + vhost_user_iotlb_pool_put(vq, new_node); goto unlock; } else if (node->iova > new_node->iova) { TAILQ_INSERT_BEFORE(node, new_node, next); @@ -235,7 +256,7 @@ vhost_user_iotlb_cache_remove(struct vhost_virtqueue *vq, if (iova < node->iova + node->size) { TAILQ_REMOVE(&vq->iotlb_list, node, next); - rte_mempool_put(vq->iotlb_pool, node); + vhost_user_iotlb_pool_put(vq, node); vq->iotlb_cache_nr--; } } @@ -295,7 +316,7 @@ vhost_user_iotlb_flush_all(struct vhost_virtqueue *vq) int vhost_user_iotlb_init(struct virtio_net *dev, struct vhost_virtqueue *vq) { - char pool_name[RTE_MEMPOOL_NAMESIZE]; + unsigned int i; int socket = 0; if (vq->iotlb_pool) { @@ -304,6 +325,7 @@ vhost_user_iotlb_init(struct virtio_net *dev, struct vhost_virtqueue *vq) * just drop all cached and pending entries. */ vhost_user_iotlb_flush_all(vq); + rte_free(vq->iotlb_pool); } #ifdef RTE_LIBRTE_VHOST_NUMA @@ -311,32 +333,32 @@ vhost_user_iotlb_init(struct virtio_net *dev, struct vhost_virtqueue *vq) socket = 0; #endif + rte_spinlock_init(&vq->iotlb_free_lock); rte_rwlock_init(&vq->iotlb_lock); rte_rwlock_init(&vq->iotlb_pending_lock); + SLIST_INIT(&vq->iotlb_free_list); TAILQ_INIT(&vq->iotlb_list); TAILQ_INIT(&vq->iotlb_pending_list); - snprintf(pool_name, sizeof(pool_name), "iotlb_%u_%d_%d", - getpid(), dev->vid, vq->index); - VHOST_LOG_CONFIG(dev->ifname, DEBUG, "IOTLB cache name: %s\n", pool_name); - - /* If already created, free it and recreate */ - vq->iotlb_pool = rte_mempool_lookup(pool_name); - rte_mempool_free(vq->iotlb_pool); - - vq->iotlb_pool = rte_mempool_create(pool_name, - IOTLB_CACHE_SIZE, sizeof(struct vhost_iotlb_entry), 0, - 0, 0, NULL, NULL, NULL, socket, - RTE_MEMPOOL_F_NO_CACHE_ALIGN | - RTE_MEMPOOL_F_SP_PUT); + vq->iotlb_pool = rte_calloc_socket("iotlb", IOTLB_CACHE_SIZE, + sizeof(struct vhost_iotlb_entry), 0, socket); if (!vq->iotlb_pool) { - VHOST_LOG_CONFIG(dev->ifname, ERR, "Failed to create IOTLB cache pool %s\n", - pool_name); + VHOST_LOG_CONFIG(dev->ifname, ERR, + "Failed to create IOTLB cache pool for vq %"PRIu32"\n", + vq->index); return -1; } + for (i = 0; i < IOTLB_CACHE_SIZE; i++) + vhost_user_iotlb_pool_put(vq, &vq->iotlb_pool[i]); vq->iotlb_cache_nr = 0; return 0; } + +void +vhost_user_iotlb_destroy(struct vhost_virtqueue *vq) +{ + rte_free(vq->iotlb_pool); +} diff --git a/lib/vhost/iotlb.h b/lib/vhost/iotlb.h index 738e31e7b9..e27ebebcf5 100644 --- a/lib/vhost/iotlb.h +++ b/lib/vhost/iotlb.h @@ -48,5 +48,6 @@ void vhost_user_iotlb_pending_remove(struct vhost_virtqueue *vq, uint64_t iova, uint64_t size, uint8_t perm); void vhost_user_iotlb_flush_all(struct vhost_virtqueue *vq); int vhost_user_iotlb_init(struct virtio_net *dev, struct vhost_virtqueue *vq); +void vhost_user_iotlb_destroy(struct vhost_virtqueue *vq); #endif /* _VHOST_IOTLB_H_ */ diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c index 97bae0de91..0dfebace58 100644 --- a/lib/vhost/vhost.c +++ b/lib/vhost/vhost.c @@ -394,7 +394,7 @@ free_vq(struct virtio_net *dev, struct vhost_virtqueue *vq) vhost_free_async_mem(vq); rte_free(vq->batch_copy_elems); - rte_mempool_free(vq->iotlb_pool); + vhost_user_iotlb_destroy(vq); rte_free(vq->log_cache); rte_free(vq); } diff --git a/lib/vhost/vhost.h b/lib/vhost/vhost.h index c6260b54cc..782d916ae0 100644 --- a/lib/vhost/vhost.h +++ b/lib/vhost/vhost.h @@ -299,10 +299,12 @@ struct vhost_virtqueue { rte_rwlock_t iotlb_lock; rte_rwlock_t iotlb_pending_lock; - struct rte_mempool *iotlb_pool; + struct vhost_iotlb_entry *iotlb_pool; TAILQ_HEAD(, vhost_iotlb_entry) iotlb_list; TAILQ_HEAD(, vhost_iotlb_entry) iotlb_pending_list; int iotlb_cache_nr; + rte_spinlock_t iotlb_free_lock; + SLIST_HEAD(, vhost_iotlb_entry) iotlb_free_list; /* Used to notify the guest (trigger interrupt) */ int callfd;