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 D173FA0583; Fri, 20 Mar 2020 16:35:49 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 797502BF2; Fri, 20 Mar 2020 16:35:48 +0100 (CET) Received: from EUR03-VE1-obe.outbound.protection.outlook.com (mail-eopbgr50073.outbound.protection.outlook.com [40.107.5.73]) by dpdk.org (Postfix) with ESMTP id 0E8F23B5; Fri, 20 Mar 2020 16:35:47 +0100 (CET) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=WOAmqTjafiTg0foQBZbVn44HbghV2A4vpdlvmafSRwYQgHq1KO41PsVlOVSFt5eXori401dlmGuXvlbzF8Gvbb23GseSLej0OIUjSysOU9b8SxQ0gIbSqXGa0jq6JNTU16kMVqUBKq++yzEPFVhDlfJRpESrVv3NELU3HsT/CM0Koeoja2KmkqaQaSJV0vXcjr9ZPZFCWGdKjvwrcJGAcrd5S/FFZ4MQUA9TYfMG/Hm901M30J/SnW7UCLaXQ2tWnqSrk1b6378503aw5AFfO0Mdr55RJn8tN0F78gU0X8hVSsyFjAnCYa/JYd1FEAFhuBUjywv8NXbP3EY121+bZw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=uCTnxIXSouVW01Rx0pgEkeikjrTxnIEmfFe3VliF0MM=; b=DDJHP1whBnkdqBGdVaoyoFnrtbGVJboClAdLpj5MHYCoMP2Ibq6a25EMHW/NyPCH3Kd5IQ+3cY8rPs9zCogtCoqTswtTOVWeQ2pibdB50AcUwBvrP+PNyvzGtbKp/rfWd8/qx1gvT1Ou+YkKovK/VBrvxKaphpMkNfcqq/O1cGUkKAqatVcrHzjSYWG7oTGb6CZu/VPZV27SFTo94MMylGj8rgA06Bicnbbwg8c+V9OnyUdCKnZsPuOwFI9kKAhm/Te4PgvNb/8JRNqas+C6lU4I8bjfIBrwUejHxxA5IBPPV3SII2nsFs+Xd1dDvXegSXLHOsL3dltZXCZ01XtXcw== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=mellanox.com; dmarc=pass action=none header.from=mellanox.com; dkim=pass header.d=mellanox.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Mellanox.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=uCTnxIXSouVW01Rx0pgEkeikjrTxnIEmfFe3VliF0MM=; b=IPoSUOvpROXjmd4m45/sdAJbojTnGFduLw8RSnfuFDzeJkwZWeypj07AP31EIDlzZB13+zuy/fQny/6Nkw4/3rQY3KVPGGaslnfFbXxvDCAAQttTNhtOP9n6MHukFFWoe6krSGyOnXEy/qfttbQsqqHjr2CkUrAst8Tb3s1uPro= Received: from AM0PR05MB4561.eurprd05.prod.outlook.com (52.133.54.148) by AM0PR05MB5730.eurprd05.prod.outlook.com (20.178.115.152) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2814.16; Fri, 20 Mar 2020 15:35:46 +0000 Received: from AM0PR05MB4561.eurprd05.prod.outlook.com ([fe80::20a9:df2a:69cf:1494]) by AM0PR05MB4561.eurprd05.prod.outlook.com ([fe80::20a9:df2a:69cf:1494%6]) with mapi id 15.20.2835.017; Fri, 20 Mar 2020 15:35:46 +0000 From: Alexander Kozyrev To: Olivier Matz CC: "dev@dpdk.org" , Slava Ovsiienko , Matan Azrad , Thomas Monjalon , "stable@dpdk.org" Thread-Topic: [PATCH] mbuf: optimize memory loads during mbuf freeing Thread-Index: AQHV/dEIZE5jF3Mhn0y/C1A8GeFax6hRnfQg Date: Fri, 20 Mar 2020 15:35:45 +0000 Message-ID: References: <1584383500-27482-1-git-send-email-akozyrev@mellanox.com> <20200319093025.GT17125@platinum> In-Reply-To: <20200319093025.GT17125@platinum> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=akozyrev@mellanox.com; x-originating-ip: [2607:fea8:e380:d8e0:bdd8:bb49:1061:7593] x-ms-publictraffictype: Email x-ms-office365-filtering-ht: Tenant x-ms-office365-filtering-correlation-id: 2593a6b7-a335-4e3f-6b9f-08d7cce45bcf x-ms-traffictypediagnostic: AM0PR05MB5730:|AM0PR05MB5730: x-ld-processed: a652971c-7d2e-4d9b-a6a4-d149256f461b,ExtAddr x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:10000; x-forefront-prvs: 03484C0ABF x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(4636009)(39860400002)(396003)(376002)(366004)(346002)(136003)(199004)(4326008)(55016002)(81166006)(86362001)(8676002)(9686003)(33656002)(52536014)(71200400001)(6506007)(64756008)(66946007)(66446008)(478600001)(53546011)(81156014)(66476007)(66556008)(316002)(54906003)(8936002)(7696005)(2906002)(5660300002)(76116006)(186003)(6916009); DIR:OUT; SFP:1101; SCL:1; SRVR:AM0PR05MB5730; H:AM0PR05MB4561.eurprd05.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; A:1; received-spf: None (protection.outlook.com: mellanox.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: N0euTwdoGf+WkZLJuf5KHicQRdUD1C13itqXnRw/YttBdEywl14MFKEJVsmHtoDxXOiBwoDIoPO4201jp3+IUbiaJXzgSHv2vcyvIdBRiMVcWlqahvlorJHPS0KjcPTkO9hJoM+vNqQ5awC9woKpu7OnXBbGP4UpvtKIrxmU4TWHfEKdlhF9JdgZk0d48zoCTr9Y+ENCVnfugmAWeYQF+JrvR98PlKLkv5ZIIjp7rAYE/cnkOgAjdEw0XJZxpweFM4djY/i9LMH6urqdkR/51fcHmbSqKzyUlG8BumWe958eTCH4rZs4USHx8LF8Qyu5VYTisDeikiIqlk3QNaBU8Kk2QutROSSsO21OZ3cjfS/UXLW3Irp6ie9DBUP52C4cr6vGKNBSsxZ6Bkaj4U9EM1uZn6y2gHSyoNzaa/orrMdNdSynG4TGgODdKvuBR6A4 x-ms-exchange-antispam-messagedata: YcP+VKWAUDY54GDr7Bb1jMPmadoHbivjPnvzc/1xu1TzSWj5t66PQ3HSt6SDL27oRVApIfsGx0P13cNHO46lK+BYHMC+1qMDG6sQQRoCTzecufONYrK7lI4B2Ie0zG+0oW93XW7YVwgBmDflSbvBOEznhea+u7nOCU3rEFxcwMLaK6LfPEpFOMayHnjDKN4xBsheK4iTUYb3Agzp/J9yzw== Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: Mellanox.com X-MS-Exchange-CrossTenant-Network-Message-Id: 2593a6b7-a335-4e3f-6b9f-08d7cce45bcf X-MS-Exchange-CrossTenant-originalarrivaltime: 20 Mar 2020 15:35:45.8925 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: a652971c-7d2e-4d9b-a6a4-d149256f461b X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: qwg6Pb/njoKd7w4FqEDquuq+dQ3WuvrpFr0e8cTURlnCRzhW/ghZI7Jg385K2fZFMxcnRbz1KcXVAbTJj8QpRQ== X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM0PR05MB5730 Subject: Re: [dpdk-dev] [PATCH] mbuf: optimize memory loads during mbuf freeing 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" You are right, Olivier, thanks for your suggestion - it looks even better. I've tested this version and the performance is great - will send a v2 shor= tly. Regards, Alex > -----Original Message----- > From: Olivier Matz > Sent: Thursday, March 19, 2020 5:30 > To: Alexander Kozyrev > Cc: dev@dpdk.org; Slava Ovsiienko ; Matan > Azrad ; Thomas Monjalon > ; stable@dpdk.org > Subject: Re: [PATCH] mbuf: optimize memory loads during mbuf freeing >=20 > Hi, >=20 > On Mon, Mar 16, 2020 at 06:31:40PM +0000, Alexander Kozyrev wrote: > > Introduction of pinned external buffers doubled memory loads in the > > rte_pktmbuf_prefree_seg() function. Analysis of the generated assembly > > code shows unnecessary load of the pool field of the rte_mbuf structure= . > > Here is the snippet of the assembly for "if (!RTE_MBUF_DIRECT(m))": > > Before the change the code was: > > movq 0x18(%rbx), %rax // load the ol_flags field > > test %r13, %rax // check if ol_flags equals to 0x60...0 > > jz 0x9a8718 // jump out to "if (m->next !=3D NULL)" > > After the change the code becomed: > > movq 0x18(%rbx), %rax // load ol_flags > > test %r14, %rax // check if ol_flags equals to 0x60...0 > > jnz 0x9bea38 // jump in to "if > (!RTE_MBUF_HAS_EXTBUF(m)" > > movq 0x48(%rbx), %rax // load the pool field > > jmp 0x9bea78 // jump out to "if (m->next !=3D NULL)" > > Look like this absolutely unneeded memory load of the pool field is an > > optimization for the external buffer case in GCC (4.8.5), since Clang > > generates the same assembly for both before and after the chenge > versions. > > Plus, GCC favors the extrnal buffer case over the simple case. > > This assembly code layout causes the performance degradation because > > the > > rte_pktmbuf_prefree_seg() function is a part of a very hot path. > > Workaround this compilation issue by moving the check for pinned > > buffer apart from the check for external buffer and restore the > > initial code flow that favors the direct mbuf case over the external on= e. > > > > Fixes: 6ef1107ad4c6 ("mbuf: detach mbuf with pinned external buffer") > > Cc: stable@dpdk.org > > > > Signed-off-by: Alexander Kozyrev > > Acked-by: Viacheslav Ovsiienko > > --- > > lib/librte_mbuf/rte_mbuf.h | 14 ++++++-------- > > 1 file changed, 6 insertions(+), 8 deletions(-) > > > > diff --git a/lib/librte_mbuf/rte_mbuf.h b/lib/librte_mbuf/rte_mbuf.h > > index 34679e0..ab9d3f5 100644 > > --- a/lib/librte_mbuf/rte_mbuf.h > > +++ b/lib/librte_mbuf/rte_mbuf.h > > @@ -1335,10 +1335,9 @@ static inline int > __rte_pktmbuf_pinned_extbuf_decref(struct rte_mbuf *m) > > if (likely(rte_mbuf_refcnt_read(m) =3D=3D 1)) { > > > > if (!RTE_MBUF_DIRECT(m)) { > > - if (!RTE_MBUF_HAS_EXTBUF(m) || > > - !RTE_MBUF_HAS_PINNED_EXTBUF(m)) > > - rte_pktmbuf_detach(m); > > - else if (__rte_pktmbuf_pinned_extbuf_decref(m)) > > + rte_pktmbuf_detach(m); > > + if (RTE_MBUF_HAS_PINNED_EXTBUF(m) && > > + __rte_pktmbuf_pinned_extbuf_decref(m)) > > return NULL; > > } > > > [...] >=20 > Reading the previous code again, it was correct but not easy to understan= d, > especially the: >=20 > if (!RTE_MBUF_HAS_EXTBUF(m) || !RTE_MBUF_HAS_PINNED_EXTBUF(m)) >=20 > Knowing we already checked it is not a direct mbuf, it is equivalent to: >=20 > if (!RTE_MBUF_HAS_PINNED_EXTBUF(m)) >=20 > I think the objective was to avoid an access to the pool flags if not nec= essary. >=20 > Completely removing the test as you did is also functionally OK, because > rte_pktmbuf_detach() also does the check, and the code is even clearer. >=20 > I wonder however if doing this wouldn't avoid an access to the pool flags= for > mbufs which have the IND_ATTACHED flags: >=20 > if (!RTE_MBUF_DIRECT(m)) { > rte_pktmbuf_detach(m); > if (RTE_MBUF_HAS_EXTBUF(m) && > RTE_MBUF_HAS_PINNED_EXTBUF(m) && > __rte_pktmbuf_pinned_extbuf_decref(m)) > return NULL; > } >=20 > What do you think? >=20 > Nit: if you wish to send a v2, there are few english fixes that could be = done > (becomed, chenge, extrnal) >=20 > Thanks