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 32A00A0527; Mon, 9 Nov 2020 19:04:45 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 2095D6A1C; Mon, 9 Nov 2020 19:04:43 +0100 (CET) Received: from mail-pf1-f193.google.com (mail-pf1-f193.google.com [209.85.210.193]) by dpdk.org (Postfix) with ESMTP id 2A400697B for ; Mon, 9 Nov 2020 19:04:42 +0100 (CET) Received: by mail-pf1-f193.google.com with SMTP id c20so8886138pfr.8 for ; Mon, 09 Nov 2020 10:04:42 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=a3MBfZvWTxcRPwOMiDGun2RydCiASD4rkX9VfSN7iAI=; b=qbYmeKPvNAXqHNOeS4kyfAqpmoGtdJLaNPdFKKdrvZ7UVvhKRClIH0nYx9fh2I8Maa W/QPKp2jUZX4MYRrHy8LxZGK8Fskf6imjh4QfPjE0NGy89jL7ZQ/ms54FoFHDCL4e5qK GG+FbhblBUcTn4W3fWi4NJYPcJJDHbL804SaOwwym06FxDR4G7w9XC1KrIRA6Od3WSZB yPEVV22CUL9PpqErJ2jLQmnSnV8Ic3Qr0P9pRCpGD1LDE9mfBpg73Bk5EscO9eFjF62S SAN+rI28YDZufcl0wQrx1My0n67xfKKYuCZc+7sq/hddUkTSfiJYyMKJck58WBxERB4E fatA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=a3MBfZvWTxcRPwOMiDGun2RydCiASD4rkX9VfSN7iAI=; b=kmixnaJnnWcWfceET5t9hj5xFpTjo84g6JfR+2z5idNksRjYNa8yL4ruH7lNJx8pdF atgD1ZLN3E5ttKzjjASd+pJc7ICK27AvLJloMnRGUykIx6Iqhdkp+YhTZVczo3SlYZ3j A4MLxXkRUS+Sipnx0AaZrXAy/IbE9LZfVXZcJnulTmed2ZTBczSFvo/jIOit/SI67zrd YyQUo/hntfpM4W1eKvEy2wOL1K2n9ffa/SHgffo0I2QRu/1NbpHd5bAW2lGiqYutC2kb gYSVGJ6ghhj8ZAu8G3n/0XEZ0goZfjrmniDSHa0iVTsEv6M/kA1DZLKFsEIc3EqVjBX9 yJJQ== X-Gm-Message-State: AOAM533b+HBTalOZP1qPAex3oBVU+3Zjav+yUDc07Mu92SkxLPvMgw4C P8ktnnR5PEL4KNSAq41i8jsGtQ== X-Google-Smtp-Source: ABdhPJzOmLkcAe2AC+MdFaTbbme/lSaU637Q7gc++MfWOhhMyIEfyimNMOoU1I30Mrx5eWC6ryhGBA== X-Received: by 2002:a63:190e:: with SMTP id z14mr13516215pgl.272.1604945080227; Mon, 09 Nov 2020 10:04:40 -0800 (PST) Received: from hermes.local (204-195-22-127.wavecable.com. [204.195.22.127]) by smtp.gmail.com with ESMTPSA id z23sm11022467pgf.12.2020.11.09.10.04.38 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 09 Nov 2020 10:04:39 -0800 (PST) Date: Mon, 9 Nov 2020 10:04:30 -0800 From: Stephen Hemminger To: Morten =?UTF-8?B?QnLDuHJ1cA==?= Cc: "Bruce Richardson" , "Jerin Jacob" , "Thomas Monjalon" , "dpdk-dev" , "David Marchand" , "Ferruh Yigit" , "Olivier Matz" , "Ananyev, Konstantin" , "Andrew Rybchenko" , "Viacheslav Ovsiienko" , "Ajit Khaparde" , "Jerin Jacob" , "Hemant Agrawal" , "Ray Kinsella" , "Neil Horman" , "Nithin Dabilpuram" , "Kiran Kumar K" , Message-ID: <20201109100430.5edb39f3@hermes.local> In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35C61407@smartserver.smartshare.dk> References: <20201107155306.463148-1-thomas@monjalon.net> <4509916.LqRtgDRpI1@thomas> <6088267.6fNGb03Fmp@thomas> <98CBD80474FA8B44BF855DF32C47DC35C61405@smartserver.smartshare.dk> <20201109100605.GB831@bricha3-MOBL.ger.corp.intel.com> <98CBD80474FA8B44BF855DF32C47DC35C61407@smartserver.smartshare.dk> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Subject: Re: [dpdk-dev] [dpdk-techboard] [PATCH 1/1] mbuf: move pool pointer in first half 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 Mon, 9 Nov 2020 11:21:02 +0100 Morten Br=C3=B8rup wrote: > > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Bruce Richardson > > Sent: Monday, November 9, 2020 11:06 AM > >=20 > > On Mon, Nov 09, 2020 at 09:16:27AM +0100, Morten Br=C3=B8rup wrote: =20 > > > +CC techboard > > > =20 > > > > From: Jerin Jacob [mailto:jerinjacobk@gmail.com] > > > > Sent: Monday, November 9, 2020 6:18 AM > > > > > > > > On Sun, Nov 8, 2020 at 2:03 AM Thomas Monjalon =20 > > =20 > > > > wrote: =20 > > > > > > > > > > 07/11/2020 20:05, Jerin Jacob: =20 > > > > > > On Sun, Nov 8, 2020 at 12:09 AM Thomas Monjalon =20 > > > > wrote: =20 > > > > > > > 07/11/2020 18:12, Jerin Jacob: =20 > > > > > > > > On Sat, Nov 7, 2020 at 10:04 PM Thomas Monjalon =20 > > > > wrote: =20 > > > > > > > > > > > > > > > > > > The mempool pointer in the mbuf struct is moved > > > > > > > > > from the second to the first half. > > > > > > > > > It should increase performance on most systems having 64-= =20 > > byte =20 > > > > cache line, =20 > > > > > > > > =20 > > > > > > > > > i.e. mbuf is split in two cache lines. =20 > > > > > > > > > > > > > > > > But In any event, Tx needs to touch the pool to freeing =20 > > back to =20 > > > > the =20 > > > > > > > > pool upon Tx completion. Right? > > > > > > > > Not able to understand the motivation for moving it to the = =20 > > > > first 64B cache line? =20 > > > > > > > > The gain varies from driver to driver. For example, a =20 > > Typical =20 > > > > > > > > ARM-based NPU does not need to > > > > > > > > touch the pool in Rx and its been filled by HW. Whereas it = =20 > > > > needs to =20 > > > > > > > > touch in Tx if the reference count is implemented. =20 > > > > > > > > > > > > See below. > > > > > > =20 > > > > > > > > =20 > > > > > > > > > Due to this change, tx_offload is moved, so some vector = =20 > > data =20 > > > > paths =20 > > > > > > > > > may need to be adjusted. Note: OCTEON TX2 check is =20 > > removed =20 > > > > temporarily! =20 > > > > > > > > > > > > > > > > It will be breaking the Tx path, Please just don't remove = =20 > > the =20 > > > > static =20 > > > > > > > > assert without adjusting the code. =20 > > > > > > > > > > > > > > Of course not. > > > > > > > I looked at the vector Tx path of OCTEON TX2, > > > > > > > it's close to be impossible to understand :) > > > > > > > Please help! =20 > > > > > > > > > > > > Off course. Could you check the above section any share the =20 > > > > rationale =20 > > > > > > for this change > > > > > > and where it helps and how much it helps? =20 > > > > > > > > > > It has been concluded in the techboard meeting you were part of. > > > > > I don't understand why we restart this discussion again. > > > > > I won't have the energy to restart this process myself. > > > > > If you don't want to apply the techboard decision, then please > > > > > do the necessary to request another quick decision. =20 > > > > > > > > Yes. Initially, I thought it is OK as we have 128B CL, After =20 > > looking =20 > > > > into Thomas's change, I realized > > > > it is not good for ARM64 64B catchlines based NPU as > > > > - A Typical ARM-based NPU does not need to touch the pool in Rx =20 > > and =20 > > > > its been filled by HW. Whereas it needs to > > > > touch in Tx if the reference count is implemented. =20 > > > > > > Jerin, I don't understand what the problem is here... > > > > > > Since RX doesn't touch m->pool, it shouldn't matter for RX which =20 > > cache line m->pool resides in. I get that. =20 > > > > > > You are saying that TX needs to touch m->pool if the reference count = =20 > > is implemented. I get that. But I don't understand why it is worse > > having m->pool in the first cache line than in the second cache line; > > can you please clarify? =20 > > > =20 > > > > - Also it will be effecting exiting vector routines =20 > > > > > > That is unavoidable if we move something from the second to the first= =20 > > cache line. =20 > > > > > > It may require some rework on the vector routines, but it shouldn't = =20 > > be too difficult for whoever wrote these vector routines. =20 > > > =20 > > > > > > > > I request to reconsider the tech board decision. =20 > > > > > > I was on the techboard meeting as an observer (or whatever the =20 > > correct term would be for non-members), and this is my impression of > > the decision on the meeting: =20 > > > > > > The techboard clearly decided not to move any dynamic fields in the = =20 > > first cache line, on the grounds that if we move them away again in a > > later version, DPDK users utilizing a dynamic field in the first cache > > line might experience a performance drop at that later time. And this > > will be a very bad user experience, causing grief and complaints. To > > me, this seemed like a firm decision, based on solid arguments. =20 > > > > > > Then the techboard discussed which other field to move to the freed = =20 > > up space in the first cache line. There were no performance reports > > showing any improvements by moving the any of the suggested fields (m- = =20 > > >pool, m->next, m->tx_offload), and there was a performance report =20 > > showing no improvements by moving m->next in a test case with large > > segmented packets. The techboard decided to move m->pool as originally > > suggested. To me, this seemed like a somewhat random choice between A, > > B and C, on the grounds that moving one of them is probably better than > > moving none of them. =20 > > > =20 > >=20 > > This largely tallies with what I remember of the discussion too. > >=20 > > I'd also add though that the choice between the next pointer and the > > pool > > pointer came down to the fact that the next pointer is only used for > > chained, multi-segment, packets - which also tend to be larger packets > > - > > while the pool pointer is of relevance to all packets, big and small, > > single and multi-segment. =20 >=20 > I wish that was the case, but I am not so sure... >=20 > It is true that m->next is NULL for non-segmented packets. Yes m->next is NULL for non-segmented packets. Do we need to modify rte_mbuf_check() to enforce these checks?