From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 03898A058A; Fri, 17 Apr 2020 08:56:01 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id C6C181DE5C; Fri, 17 Apr 2020 08:56:00 +0200 (CEST) Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id 9F2801DE5A for ; Fri, 17 Apr 2020 08:55:58 +0200 (CEST) IronPort-SDR: xRzMSpW48l2u8DlyxdNPCM2VYseDCYX8rrVYy3J+dJew4/aXY1Db6Z2svcI+VmOEath+DXtw4W 13NoZP/E1vuA== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 16 Apr 2020 23:55:57 -0700 IronPort-SDR: Rgp6x7IxEnviVqAIiuGb9a75VMk301Fy5fOJJIMBdLEKhoYNN5CdbETYpaGSFCyefwNsUORZSK 2bRH1+Dpffsg== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.72,394,1580803200"; d="scan'208";a="257492319" Received: from yexl-server.sh.intel.com (HELO localhost) ([10.67.117.17]) by orsmga006.jf.intel.com with ESMTP; 16 Apr 2020 23:55:54 -0700 Date: Fri, 17 Apr 2020 14:51:45 +0800 From: Ye Xiaolong To: Joyce Kong Cc: maxime.coquelin@redhat.com, stephen@networkplumber.org, tiwei.bie@intel.com, zhihong.wang@intel.com, thomas@monjalon.net, jerinj@marvell.com, yinan.wang@intel.com, honnappa.nagarahalli@arm.com, gavin.hu@arm.com, nd@arm.com, dev@dpdk.org Message-ID: <20200417065145.GA57965@intel.com> References: <20200212092456.29433-1-joyce.kong@arm.com> <20200406152634.606-2-joyce.kong@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200406152634.606-2-joyce.kong@arm.com> User-Agent: Mutt/1.9.4 (2018-02-28) Subject: Re: [dpdk-dev] [PATCH v3 1/2] virtio: one way barrier for split vring used idx X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 04/06, Joyce Kong wrote: >In case VIRTIO_F_ORDER_PLATFORM(36) is not negotiated, then the frontend >and backend are assumed to be implemented in software, that is they can >run on identical CPUs in an SMP configuration. >Thus a weak form of memory barriers like rte_smp_r/wmb, other than >rte_cio_r/wmb, is sufficient for this case(vq->hw->weak_barriers == 1) >and yields better performance. >For the above case, this patch helps yielding even better performance >by replacing the two-way barriers with C11 one-way barriers for used >index in split ring. > >Signed-off-by: Joyce Kong >Reviewed-by: Gavin Hu >--- > drivers/net/virtio/virtio_ethdev.c | 9 ++-- > drivers/net/virtio/virtio_ring.h | 2 +- > drivers/net/virtio/virtio_rxtx.c | 46 +++++++++---------- > drivers/net/virtio/virtio_rxtx_simple_neon.c | 5 +- > drivers/net/virtio/virtio_rxtx_simple_sse.c | 5 +- > .../net/virtio/virtio_user/virtio_user_dev.c | 8 ++-- > drivers/net/virtio/virtqueue.c | 2 +- > drivers/net/virtio/virtqueue.h | 37 ++++++++++++--- > lib/librte_vhost/virtio_net.c | 5 +- > 9 files changed, 71 insertions(+), 48 deletions(-) > >diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c >index f9d0ea70d..a4a865bfa 100644 >--- a/drivers/net/virtio/virtio_ethdev.c >+++ b/drivers/net/virtio/virtio_ethdev.c >@@ -285,13 +285,12 @@ virtio_send_command_split(struct virtnet_ctl *cvq, > > virtqueue_notify(vq); > >- rte_rmb(); >- while (VIRTQUEUE_NUSED(vq) == 0) { >- rte_rmb(); >+ /* virtqueue_nused has a load-acquire or rte_cio_rmb inside */ >+ while (virtqueue_nused(vq) == 0) > usleep(100); >- } > >- while (VIRTQUEUE_NUSED(vq)) { >+ /* virtqueue_nused has a load-acquire or rte_cio_rmb inside */ >+ while (virtqueue_nused(vq)) { > uint32_t idx, desc_idx, used_idx; > struct vring_used_elem *uep; > >diff --git a/drivers/net/virtio/virtio_ring.h b/drivers/net/virtio/virtio_ring.h >index 7ba34662e..0f6574f68 100644 >--- a/drivers/net/virtio/virtio_ring.h >+++ b/drivers/net/virtio/virtio_ring.h >@@ -59,7 +59,7 @@ struct vring_used_elem { > > struct vring_used { > uint16_t flags; >- volatile uint16_t idx; >+ uint16_t idx; > struct vring_used_elem ring[0]; > }; > >diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c >index 752faa0f6..9ba26fd95 100644 >--- a/drivers/net/virtio/virtio_rxtx.c >+++ b/drivers/net/virtio/virtio_rxtx.c >@@ -45,7 +45,7 @@ virtio_dev_rx_queue_done(void *rxq, uint16_t offset) > struct virtnet_rx *rxvq = rxq; > struct virtqueue *vq = rxvq->vq; > >- return VIRTQUEUE_NUSED(vq) >= offset; >+ return virtqueue_nused(vq) >= offset; > } > > void >@@ -1243,9 +1243,8 @@ virtio_recv_pkts(void *rx_queue, struct rte_mbuf **rx_pkts, uint16_t nb_pkts) > if (unlikely(hw->started == 0)) > return nb_rx; > >- nb_used = VIRTQUEUE_NUSED(vq); >- >- virtio_rmb(hw->weak_barriers); >+ /* virtqueue_nused has a load-acquire or rte_cio_rmb inside */ Small nit, I don't think we need to add this comment to every occurrence of virtqueue_nused, what about moving it to the definition of this function? Thanks, Xiaolong