From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dmarchan@redhat.com>
Received: from mail-ua1-f68.google.com (mail-ua1-f68.google.com
[209.85.222.68]) by dpdk.org (Postfix) with ESMTP id D879E1B1F4
for <dev@dpdk.org>; Wed, 9 Jan 2019 11:05:46 +0100 (CET)
Received: by mail-ua1-f68.google.com with SMTP id j3so2236694uap.3
for <dev@dpdk.org>; Wed, 09 Jan 2019 02:05:46 -0800 (PST)
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
d=1e100.net; s=20161025;
h=x-gm-message-state:mime-version:references:in-reply-to:from:date
:message-id:subject:to:cc;
bh=6iQ5TBJBLyJui3uEOioWLqyiidDNO//6ReNQdqg+Gc4=;
b=IuTai/RLUv5JLTq3A/Hq4d6WxGTyxjIiBPkTMIih/ga+9Aj3QIkXTCUdFG9ON1cKqA
wk6fEMAS7gf5dwA97ivGSp3uPt8wHXEVpUwMfwy8mmJWQeXN4Z3NkqVnwir4Xm8pndLX
MDbfb7HrjiK/RARGvXrvJDDSGHyoPP+MHtJ9RDaxkM+QoRHkohqhdWMKvKDImLWRUO8z
eR3iNjOuhfhhdFG0NRL1qMrZFwS3ghit9IRI7iYqGK1FCXbi//BGKJuE0+aEW8Fhq0tI
6t6/M6Fm4AsLADn5FuPpKGTCYggA8JatWy3c2u2aYQw5aCFVJ80lDAOk90XHbQmk+xw7
lE+A==
X-Gm-Message-State: AJcUukfp+QNjEF+haE9P3fLnQNjGEeU6qyuqvytvoaYepu2Q5skTOhvX
nsZ0lf47gsjrziePGOpJsb6d1aiTKfDSNqi4exFBNA==
X-Google-Smtp-Source: ALg8bN77OR5thQpxm0z4yX/JqsVOkqEcb7czek9vGKKTLk76CbrG95PWLJckuqmu1QpfMeyYw+vWMhvkhYEFt4bSRmY=
X-Received: by 2002:ab0:4121:: with SMTP id j30mr1833271uad.65.1547028346264;
Wed, 09 Jan 2019 02:05:46 -0800 (PST)
MIME-Version: 1.0
References: <20190109085426.39965-1-yskoh@mellanox.com>
<CAJFAV8x+WBbei_HwQeGMdb+YZw8YSEWgP4uVp0Nihewc=myEaw@mail.gmail.com>
<20190109095205.us53xaocvokx4jog@glumotte.dev.6wind.com>
<45D5A06B-4014-428E-A588-6E188C87A5AA@mellanox.com>
In-Reply-To: <45D5A06B-4014-428E-A588-6E188C87A5AA@mellanox.com>
From: David Marchand <david.marchand@redhat.com>
Date: Wed, 9 Jan 2019 11:05:35 +0100
Message-ID: <CAJFAV8wZKRSR-_E0AY7oYjGJf=8PqWH8JE6NQE_UJiEh0xYe_A@mail.gmail.com>
To: Yongseok Koh <yskoh@mellanox.com>
Cc: Olivier Matz <olivier.matz@6wind.com>, Shahaf Shuler <shahafs@mellanox.com>,
"dev@dpdk.org" <dev@dpdk.org>, "stable@dpdk.org" <stable@dpdk.org>
Content-Type: text/plain; charset="UTF-8"
X-Content-Filtered-By: Mailman/MimeDel 2.1.15
Subject: Re: [dpdk-dev] [PATCH] net/mlx5: fix instruction hotspot on
replenishing Rx buffer
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Wed, 09 Jan 2019 10:05:47 -0000
On Wed, Jan 9, 2019 at 10:56 AM Yongseok Koh <yskoh@mellanox.com> wrote:
>
> > On Jan 9, 2019, at 1:52 AM, Olivier Matz <olivier.matz@6wind.com> wrote:
> >
> > On Wed, Jan 09, 2019 at 10:38:07AM +0100, David Marchand wrote:
> >> On Wed, Jan 9, 2019 at 9:54 AM Yongseok Koh <yskoh@mellanox.com> wrote:
> >>
> >>> On replenishing Rx buffers for vectorized Rx, mbuf->buf_addr isn't
> needed
> >>> to be accessed as it is static and easily calculated from the mbuf
> address.
> >>> Accessing the mbuf content causes unnecessary load stall and it is
> worsened
> >>> on ARM.
> >>>
> >>> Fixes: 545b884b1da3 ("net/mlx5: fix buffer address posting in SSE Rx")
> >>> Cc: stable@dpdk.org
> >>>
> >>> Signed-off-by: Yongseok Koh <yskoh@mellanox.com>
> >>> ---
> >>> drivers/net/mlx5/mlx5_rxtx_vec.h | 8 ++++++--
> >>> 1 file changed, 6 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/drivers/net/mlx5/mlx5_rxtx_vec.h
> >>> b/drivers/net/mlx5/mlx5_rxtx_vec.h
> >>> index fda7004e2d..ced5547307 100644
> >>> --- a/drivers/net/mlx5/mlx5_rxtx_vec.h
> >>> +++ b/drivers/net/mlx5/mlx5_rxtx_vec.h
> >>> @@ -102,8 +102,12 @@ mlx5_rx_replenish_bulk_mbuf(struct mlx5_rxq_data
> >>> *rxq, uint16_t n)
> >>> return;
> >>> }
> >>> for (i = 0; i < n; ++i) {
> >>> - wq[i].addr =
> rte_cpu_to_be_64((uintptr_t)elts[i]->buf_addr
> >>> +
> >>> - RTE_PKTMBUF_HEADROOM);
> >>> + uintptr_t buf_addr =
> >>> + (uintptr_t)elts[i] + sizeof(struct rte_mbuf) +
> >>> + rte_pktmbuf_priv_size(rxq->mp) +
> >>> RTE_PKTMBUF_HEADROOM;
> >>> +
> >>> + assert(buf_addr == (uintptr_t)elts[i]->buf_addr);
> >>> + wq[i].addr = rte_cpu_to_be_64(buf_addr);
> >>> /* If there's only one MR, no need to replace LKey in
> WQE.
> >>> */
> >>> if (unlikely(mlx5_mr_btree_len(&rxq->mr_ctrl.cache_bh) >
> >>> 1))
> >>> wq[i].lkey = mlx5_rx_mb2mr(rxq, elts[i]);
> >>> --
> >>> 2.11.0
> >>>
> >>>
> >> How about having a macro / inline in the mbuf api to get this
> information
> >> in a consistent/unique way ?
> >> I can see we have this calculation at least in rte_pktmbuf_init() and
> >> rte_pktmbuf_detach().
> >
> > Agree. Maybe rte_mbuf_default_buf_addr(m) ?
>
> I'm also okay to add. Will come up with a new patch.
>
> > Side note, is the assert() correct in the patch? I'd say there's a
> > difference of RTE_PKTMBUF_HEADROOM between the 2 values.
>
> Oops, my fault. Thanks for the catch, you saved a crash. :-)
>
Is this assert really necessary if we have a common macro ?
I was under the impression that this assert is there to catch misalignement
between the mbuf api and the driver.
--
David Marchand