From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 0D30042C34; Mon, 5 Jun 2023 10:27:26 +0200 (CEST) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id D9BAC40A7F; Mon, 5 Jun 2023 10:27:25 +0200 (CEST) Received: from mail-wm1-f52.google.com (mail-wm1-f52.google.com [209.85.128.52]) by mails.dpdk.org (Postfix) with ESMTP id A30FA4003C for ; Mon, 5 Jun 2023 10:27:24 +0200 (CEST) Received: by mail-wm1-f52.google.com with SMTP id 5b1f17b1804b1-3f6e68cc738so38846415e9.1 for ; Mon, 05 Jun 2023 01:27:24 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind.com; s=google; t=1685953644; x=1688545644; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=mLA0zvhzoNAcergmP3caDNTpQ3AyefwkV8duex2vhuQ=; b=KXda7FmpukoS18gAy1/ih3lJ0a4nif9ccis0sETWfhwGhogJ5fNhcYoRYmwxRW4vkS QlhDahR7Ycq0+HzPR5XB/ZC9W/I7tlMI4Ls9ROf0DdtLpK3bhRcuaCB9YqioTCWKfotE yN+pCqW0bkzWcf4FVHXNwvslGRXI1W2bd0Yr3gkX19ji2pi6Q9z4UbjCTZV4LaYmMSao 2cO1Y4mvLcaxTvYt3e57KEN+SwjancIujU/DXk+HyQb+m2j+wdy4ApUvg6fM8fzb01RP 2LtqenQUzzFsNpfUHH0ioUP7PHlx3kSL72/SeDxEsBo5GO7L5VjhYLzMjY95IpvRNLKh RPEg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1685953644; x=1688545644; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=mLA0zvhzoNAcergmP3caDNTpQ3AyefwkV8duex2vhuQ=; b=eRHyuFl4lDiYzG3HpvLPHqdJBpV56en+GMpDBXwK0kNVwZTEK19engFmUqUcrPXMyH ab630TjIVOiV0vuX2bizSLXIKo06oyB1CbPtS2FNFXFDpHzvjsH9U5/MRTvnI65yc0I9 5QYGxjDKNlFiPmY/w7yuorFUtMNQ3zyePsuoA4NSKDb4cafz9nzIC1ONK8kjrWF41HIO /DYMTDhhgUBzznBBGOUK3I61btIgw2PhxuDCrMjqCXVDfl+GBypZqC5v8A+ARHtwVS6v CiY0GRIzlDgLBGAppkX2UsMKmyvSEprm7P8X9kSvH1NCu6b64G5dISa/4E3sH7pt4qAK qpIQ== X-Gm-Message-State: AC+VfDytEwlr8rav6HpUSrsuObsqc2oDJbPliQDw8k6jx5b/9nZpv5Jp ubBXeCH3EnenCwTVXBrQDYinLw== X-Google-Smtp-Source: ACHHUZ7i1bkWT1A52j05thnvR7Bsiv7lfJ7id0/gcyopyKmcALQx1gP2fbdNOBqpphkfu+glr9iU6A== X-Received: by 2002:a05:600c:221a:b0:3f7:e65f:7af2 with SMTP id z26-20020a05600c221a00b003f7e65f7af2mr486100wml.39.1685953644233; Mon, 05 Jun 2023 01:27:24 -0700 (PDT) Received: from 6wind.com ([2a01:e0a:5ac:6460:c065:401d:87eb:9b25]) by smtp.gmail.com with ESMTPSA id p18-20020a05600c205200b003f427687ba7sm9944636wmg.41.2023.06.05.01.27.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 05 Jun 2023 01:27:23 -0700 (PDT) Date: Mon, 5 Jun 2023 10:27:22 +0200 From: Olivier Matz To: Tyler Retzlaff Cc: dev@dpdk.org, david.marchand@redhat.com, Bruce Richardson , Kevin Laatz , Qiming Yang , Qi Zhang , Wenjun Wu , Tetsuya Mukawa , Honnappa.Nagarahalli@arm.com, thomas@monjalon.net Subject: Re: [PATCH v4 6/6] net/ring: replace rte atomics with GCC builtin atomics Message-ID: References: <1679084388-19267-1-git-send-email-roretzla@linux.microsoft.com> <1685735107-19208-1-git-send-email-roretzla@linux.microsoft.com> <1685735107-19208-7-git-send-email-roretzla@linux.microsoft.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1685735107-19208-7-git-send-email-roretzla@linux.microsoft.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 Hi Tyler, Few comments below. On Fri, Jun 02, 2023 at 12:45:07PM -0700, Tyler Retzlaff wrote: > Replace the use of rte_atomic.h types and functions, instead use GCC > supplied C++11 memory model builtins. > > Signed-off-by: Tyler Retzlaff > Acked-by: Morten Brørup > Acked-by: Bruce Richardson > --- > drivers/net/ring/rte_eth_ring.c | 26 ++++++++++++++++---------- > 1 file changed, 16 insertions(+), 10 deletions(-) > > diff --git a/drivers/net/ring/rte_eth_ring.c b/drivers/net/ring/rte_eth_ring.c > index e8bc9b6..43eb627 100644 > --- a/drivers/net/ring/rte_eth_ring.c > +++ b/drivers/net/ring/rte_eth_ring.c > @@ -44,8 +44,8 @@ enum dev_action { > > struct ring_queue { > struct rte_ring *rng; > - rte_atomic64_t rx_pkts; > - rte_atomic64_t tx_pkts; > + uint64_t rx_pkts; > + uint64_t tx_pkts; > }; > > struct pmd_internals { > @@ -80,9 +80,10 @@ struct pmd_internals { > const uint16_t nb_rx = (uint16_t)rte_ring_dequeue_burst(r->rng, > ptrs, nb_bufs, NULL); > if (r->rng->flags & RING_F_SC_DEQ) > - r->rx_pkts.cnt += nb_rx; > + r->rx_pkts += nb_rx; > else > - rte_atomic64_add(&(r->rx_pkts), nb_rx); > + /* NOTE: review for potential ordering optimization */ > + __atomic_fetch_add(&r->rx_pkts, nb_rx, __ATOMIC_SEQ_CST); We can use __ATOMIC_RELAXED here (and below too), since there is no ordering constraint. We only want statistics to be correct. You can remove the other NOTEs from the patch. > return nb_rx; > } > > @@ -94,9 +95,10 @@ struct pmd_internals { > const uint16_t nb_tx = (uint16_t)rte_ring_enqueue_burst(r->rng, > ptrs, nb_bufs, NULL); > if (r->rng->flags & RING_F_SP_ENQ) > - r->tx_pkts.cnt += nb_tx; > + r->tx_pkts += nb_tx; > else > - rte_atomic64_add(&(r->tx_pkts), nb_tx); > + /* NOTE: review for potential ordering optimization */ > + __atomic_fetch_add(&r->tx_pkts, nb_tx, __ATOMIC_SEQ_CST); > return nb_tx; > } > > @@ -184,13 +186,15 @@ struct pmd_internals { > > for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS && > i < dev->data->nb_rx_queues; i++) { > - stats->q_ipackets[i] = internal->rx_ring_queues[i].rx_pkts.cnt; > + /* NOTE: review for atomic access */ > + stats->q_ipackets[i] = internal->rx_ring_queues[i].rx_pkts; > rx_total += stats->q_ipackets[i]; > } > > for (i = 0; i < RTE_ETHDEV_QUEUE_STAT_CNTRS && > i < dev->data->nb_tx_queues; i++) { > - stats->q_opackets[i] = internal->tx_ring_queues[i].tx_pkts.cnt; > + /* NOTE: review for atomic access */ > + stats->q_opackets[i] = internal->tx_ring_queues[i].tx_pkts; > tx_total += stats->q_opackets[i]; > } > > @@ -207,9 +211,11 @@ struct pmd_internals { > struct pmd_internals *internal = dev->data->dev_private; > > for (i = 0; i < dev->data->nb_rx_queues; i++) > - internal->rx_ring_queues[i].rx_pkts.cnt = 0; > + /* NOTE: review for atomic access */ > + internal->rx_ring_queues[i].rx_pkts = 0; > for (i = 0; i < dev->data->nb_tx_queues; i++) > - internal->tx_ring_queues[i].tx_pkts.cnt = 0; > + /* NOTE: review for atomic access */ > + internal->tx_ring_queues[i].tx_pkts = 0; > > return 0; > } > -- > 1.8.3.1 >