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 3144FA0A02; Mon, 17 May 2021 16:15:26 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 9DD6A4014E; Mon, 17 May 2021 16:15:25 +0200 (CEST) Received: from NAM12-DM6-obe.outbound.protection.outlook.com (mail-dm6nam12on2076.outbound.protection.outlook.com [40.107.243.76]) by mails.dpdk.org (Postfix) with ESMTP id DD11940041 for ; Mon, 17 May 2021 16:15:24 +0200 (CEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=ZfE4OJUOgGqztAGasW0R49g0+1LhX9vtFq4FB9oN5g+Ba+NSQD9I20aBUXZmyPDuqWvIu2DgAs2wuXNGoncTE/FA5FT3KAyEFwB1lxHd1eV2eXldF+64b818YExo3LL596gksy6o6nJVXgZNej/eNh0YsXTefOFd7HzcT2GM5vGztoWfemSYfogayocrcLIctW5R6/gWqHW58prq0UG/GurykFjCqNReu6tcDiNjhScP2e8CKxUOTDBeppFbhmK5Quo8iAQz79mpuHASYm1jfH9bff+mc5c/2gFw0OguHcxsbOK/Yh1q2PrWAFmrP4v+3Ayj22s02LAipDlSdJq5mw== 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=Xcq/c5dqhxaBfTahVnbqUCR2zFlEUzUc+H+ZwaJQap0=; b=n74fPOUlJ8otYzjyw7TA4lmtqHqT1kd9P2MjcOIw5cbqzWrKkYS8dxJqw5F8npQeGmMW+2KDAaOxM/aApUx0rODmh6TRpGmd9u2YOoMj4+HxrjFsanaEiKGrpmjmmCTduy4QbuR7BpAWuD7zcJovDo0Mf1LdKS/nUdOgMeiuYTQRl93c7b7Iv7JN8u3y9FKufm0jbLxVmaPeeZiVtfmS7mLwgFZIYkN75oZYRLgO4HRHV1qEwcGvd8LS13gEQ3N2SJIBsWar2tr7sKg7yr1B+P5AnbAlMysCsWluNdLruZglEcejXDvw1R7PE1ByqmWGTOg+bl/tQhPTCg5Xjc/fGQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=nvidia.com; dmarc=pass action=none header.from=nvidia.com; dkim=pass header.d=nvidia.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=Nvidia.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=Xcq/c5dqhxaBfTahVnbqUCR2zFlEUzUc+H+ZwaJQap0=; b=nLueIzxy53jQHPKvAUOHwsrRJUxaQdBL2p1/X0/y3hHABwqhX1Rv//N6pRGe3hWsGYi8HKQ7a9pjA/ycbGzfDsR+aaQidAPx0AiWJCjy0Z8Cv3yjX3POM9tyoOEzxbpX+RauMDY/tXidUYgCu2YqHwRmpAY8oHU8QAHDfrSHtMU5u/onrTQ5Bi5fgru8OgfVFLlqbdPIRDxIfYb2VSLw1i7wby4i0VoORLzPWS9dad7oBr5ncrInkmQGqjijFyUjG1ucWKKwBcaDWj3kLoS+f4p4qEyJx+BQcn1trwAafC59JihfBVJbLUxZ7Y4SUKyA50Y4Y3/BC8XNhROvzrvfkQ== Received: from DM6PR12MB3753.namprd12.prod.outlook.com (2603:10b6:5:1c7::18) by DM5PR12MB1306.namprd12.prod.outlook.com (2603:10b6:3:6c::13) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4129.25; Mon, 17 May 2021 14:15:22 +0000 Received: from DM6PR12MB3753.namprd12.prod.outlook.com ([fe80::c595:e3bc:686c:16a4]) by DM6PR12MB3753.namprd12.prod.outlook.com ([fe80::c595:e3bc:686c:16a4%5]) with mapi id 15.20.4129.031; Mon, 17 May 2021 14:15:22 +0000 From: Slava Ovsiienko To: Feifei Wang , Matan Azrad , Shahaf Shuler CC: "dev@dpdk.org" , "nd@arm.com" , Ruifeng Wang Thread-Topic: [PATCH v2 2/2] net/mlx5: remove unnecessary wmb for Memory Region cache Thread-Index: AQHXSwNxEZyrvNnNEUWs9CCvmpnuF6rnsmow Date: Mon, 17 May 2021 14:15:22 +0000 Message-ID: References: <20210318071840.359957-1-feifei.wang2@arm.com> <20210517100002.19905-1-feifei.wang2@arm.com> <20210517100002.19905-3-feifei.wang2@arm.com> In-Reply-To: <20210517100002.19905-3-feifei.wang2@arm.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: arm.com; dkim=none (message not signed) header.d=none;arm.com; dmarc=none action=none header.from=nvidia.com; x-originating-ip: [188.163.75.124] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 95043c63-1799-4290-f203-08d9193e3597 x-ms-traffictypediagnostic: DM5PR12MB1306: x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:7219; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: vDAJG1TvYWnqEerdjCQFS/DG6Vp3ou+xrllwdUckVBn9niWwQmwgTC4Ps7vVdrM1dq4IAVsQHT9mKr3zXWi4Z5mwsgvbDxvykL6tqEAQ7BYi+9ll6VWzk73S1xOCrmA0IEt6E4Rk15Ijp5HFNINzg5Kp+EbCMgqlY8LpbJJzu/7COEg49ipCNryraSIJn55gh8e1XagBXsQTCLFZav4RAcqameTiHEpQ//V7jL4KVO/+8B4+p4TwY3RzDatq3oIIBv3S0u05FdANZ9qC2QpwJ0IsItGCeNXICw7VKZ1zT19iGLkYA3blXWq7hJObIdqiuhakar7f42dHLw6wbBng8ava1xbk0FiebN2RbekVC2yX/PMJ5UclrAdmT3Eqs6psRnV3LpN4EscvGETNavRQVHydtKEK/fWxPvIQnPiqZAKDztAMTgRx66gjGU+TpObME4FDJtPSiEdflbmbDyWjeDw6omrxpq6m+aGlyRNv3135GCPkixthZB38UdSx4o1uuFT0Zij5El9k/7reStnwWvBccUPCz11KrNPkBYidy5UxaBNNO/VZRDZmyaWTsz62klbPS/0B6y/UJP8DvDTyIh2LVrdmj45ZI0jfF+uHVaJbsJzoINngf40GGemHdJmNwBSpFVfwH+3MbriDCxotpnh8r6SYCyE263QUcuHacWrQ/+VxoiXYDSPWzTyqArSY x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:DM6PR12MB3753.namprd12.prod.outlook.com; PTR:; CAT:NONE; SFS:(4636009)(396003)(136003)(376002)(346002)(366004)(39860400002)(316002)(8676002)(9686003)(55016002)(186003)(8936002)(6636002)(54906003)(5660300002)(2906002)(38100700002)(26005)(110136005)(66476007)(966005)(66556008)(71200400001)(76116006)(86362001)(122000001)(53546011)(66946007)(6506007)(52536014)(7696005)(4326008)(64756008)(478600001)(55236004)(33656002)(66446008)(83380400001); DIR:OUT; SFP:1101; x-ms-exchange-antispam-messagedata: =?us-ascii?Q?RHzLGDOdjf7qvnyL/BXWQ3phMxN5BEGK4ky10OsDVTk1KgmG3yfERfgdbSZ6?= =?us-ascii?Q?6iFlJyIdSxV+KzWfWUkkpzSEcL8yYmOC4EN//+qnc/MOTsJfV9AyRnwY4sSx?= =?us-ascii?Q?FbIX0ew4RYLzxXEV4PYBQqYIcQt8gwyFEk56Y+r82Zyhx+Fmt9hLjDBPs1y0?= =?us-ascii?Q?KsUkgYepOAxMz62APdAU2g8SpU40+s9oRUZPgTlJrLoirxfzq+h4oC5vhh/i?= =?us-ascii?Q?UgTpR4KiA8jEkaPXYX+7b+5/3Gc/j1yYAe6Jpnn5MJQPFrCiEATnqwMK8lSe?= =?us-ascii?Q?2SHPt/Vr5BRtmErbjWhBc2ewFf4/gzQYbPqUFgfsNXFwDL/OZWdbj53k2Gux?= =?us-ascii?Q?2OTemVmyNCEzhu1StgNxzh2VFfMBDs/e5kStNOlGf8oucgtxElceu4WvUdhx?= =?us-ascii?Q?mZpcU+k36o3nmYTlxKgIM44q9Ju84R9VWekvy8zLjPPU1XKqwSarE94A07Bl?= =?us-ascii?Q?FSKCE2s9Vmorff8x/a3M0kZH/zHk7EjnTkeRvrB0HEPfKDvE8mMzeC2Vrwpk?= =?us-ascii?Q?uZmu/+qMryOaSph+xijhwYKw913F1ndEhgJcOy89aeyhtLngAzafd7xbBdxs?= =?us-ascii?Q?H7cSbfDavpvBWjlcdWui4BLvIN+8/SlHr/HuiKrADxKXqTpJkUYViJ7KPH75?= =?us-ascii?Q?oyc7/64+GVw+a7Et/a5WRDD1jHYaNBg+ayaycq7GUZ2lokxVtXAN1si4N/B4?= =?us-ascii?Q?+aBKy7O5EE3h6A3JR/6kBcbSqpJ23cNg7sAORNn8J5me/ObLZc2+WyQf8oiq?= =?us-ascii?Q?LP6JqIc8FwBYkpZdoaDgdLNiAVSOXpgtMX0vUVLN/CaSQM+0JnfIbHTvEN0c?= =?us-ascii?Q?C0Ka+bpyTuD6OZtnrg6NpfZW9HZawQ0DHv0uxmAZGsDmDfUtxwB1dkePeu9Q?= =?us-ascii?Q?yJ7Wxr/banUQmqCmTbttG8adliklfQoRRugp2Qky1mW5MQ2OoiUPOIDzi9l5?= =?us-ascii?Q?POhkW9b/FdXqAyS66dweYNm1uNOk6bxS/EW4PJW73w4h9mqkmVX5QRuVInNd?= =?us-ascii?Q?52LzFcKfh9aSNdwy+g3SOWDnlg8TwVLxOosmOPmsjF90reke8rt5WetVu5ud?= =?us-ascii?Q?uCR6Z0JPJARmtoTxPpmGtT+Q5B+80xWhhmzFNFa/ETbN0REOIWkDbvS3BcXs?= =?us-ascii?Q?45wdvhVJ4CwfWJ8TipEgZqmWUWm/07AR5ugO4JeETkkxoS5qa1+CKJYY68T9?= =?us-ascii?Q?CF3lwYgB/yyGgNvhc70IyjIKaPTBMyDQavj9SFaxOsoe7C8k8VWBk9YRrgom?= =?us-ascii?Q?G228/lEmDSARTx4yQLfK45wf1JzTTo/HFaqtlVdqu6OzDlMgeMl/jqQCi1j3?= =?us-ascii?Q?d0mVjs8k1zfK+a+/XZLfQC2o?= Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: Nvidia.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: DM6PR12MB3753.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 95043c63-1799-4290-f203-08d9193e3597 X-MS-Exchange-CrossTenant-originalarrivaltime: 17 May 2021 14:15:22.5753 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 43083d15-7273-40c1-b7db-39efd9ccc17a X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: 73VOXtDuvdwXQ86wxaYGoKEnbIH7fnKApNRiZdDBMwauf/24KjWoCFpkYl8QPKN/UnQU4B3S/SvRdcqMhmYB0g== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR12MB1306 Subject: Re: [dpdk-dev] [PATCH v2 2/2] net/mlx5: remove unnecessary wmb for Memory Region cache 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 Sender: "dev" Hi, Feifei Thanks you for the patch. Please, see my notes below about typos and minor commit message rewording. > -----Original Message----- > From: Feifei Wang > Sent: Monday, May 17, 2021 13:00 > To: Matan Azrad ; Shahaf Shuler > ; Slava Ovsiienko > Cc: dev@dpdk.org; nd@arm.com; Feifei Wang ; > Ruifeng Wang > Subject: [PATCH v2 2/2] net/mlx5: remove unnecessary wmb for Memory > Region cache >=20 > 'dev_gen' is a variable to inform other cores to flush their local cache = when > global cache is rebuilt. It is unnecessary to add write memory barrier (w= mb) > before or after its updating for synchronization. >=20 Would it be better "to trigger all cores to flush their local caches once t= he global MR cache has been rebuilt" ?=20 > This is due to MR cache's R/W lock can maintain synchronization between > threads: I would add empty line here. > 1. dev_gen and global cache update ordering inside the lock protected > section does not matter. Because other threads cannot take the lock until > global cache has been updated. Thus, in out of order platform, even if ot= her > agents firstly observed updated dev_gen but global does not update, they > also needs to wait the lock. As a result, it is unnecessary to add a wmb Type: "need" (no S) -> "have to" would be better ?=20 > between rebuiling global cache and updating dev_gen to keep the order rebuiling -> rebuilDing And let's reword a little bit? "wmb between global cache rebuilding and updating the dev_gen to keep the m= emory store order." > rebuilding global cache and updating dev_gen. >=20 > 2. Store-Release of unlock can provide the implicit wmb at the level visi= ble by can provide -> provides > software. This makes 'rebuiling global cache' and 'updating dev_gen' be Typo: rebuiling -> rebuilDing > observed before local_cache starts to be updated by other agents. Thus, > wmb after 'updating dev_gen' can be removed. >=20 > Suggested-by: Ruifeng Wang > Signed-off-by: Feifei Wang > Reviewed-by: Ruifeng Wang > --- > drivers/net/mlx5/mlx5_mr.c | 26 ++++++++++---------------- > 1 file changed, 10 insertions(+), 16 deletions(-) >=20 > diff --git a/drivers/net/mlx5/mlx5_mr.c b/drivers/net/mlx5/mlx5_mr.c inde= x > e791b6338d..85e5865050 100644 > --- a/drivers/net/mlx5/mlx5_mr.c > +++ b/drivers/net/mlx5/mlx5_mr.c > @@ -107,18 +107,15 @@ mlx5_mr_mem_event_free_cb(struct > mlx5_dev_ctx_shared *sh, > if (rebuild) { > mlx5_mr_rebuild_cache(&sh->share_cache); > /* > - * Flush local caches by propagating invalidation across cores. > - * rte_smp_wmb() is enough to synchronize this event. If > one of > - * freed memsegs is seen by other core, that means the > memseg > - * has been allocated by allocator, which will come after this > - * free call. Therefore, this store instruction (incrementing > - * generation below) will be guaranteed to be seen by other > core > - * before the core sees the newly allocated memory. > + * No wmb is needed after updating dev_gen due to store- > release of > + * unlock can provide the implicit wmb at the level visible by > + * software. This makes rebuilt global cache and updated > dev_gen > + * be observed when local_cache starts to be updating by > other > + * agents. > */ Let's make comment a less wordy (and try to keep source code concise), what= about this? "No explicit wmb is needed after updating dev_gen due to store-release orde= ring in unlock that provides the implicit barrier at the software visible level.= " > ++sh->share_cache.dev_gen; > DRV_LOG(DEBUG, "broadcasting local cache flush, gen=3D%d", > sh->share_cache.dev_gen); > - rte_smp_wmb(); > } > rte_rwlock_write_unlock(&sh->share_cache.rwlock); > } > @@ -411,18 +408,15 @@ mlx5_dma_unmap(struct rte_pci_device *pdev, > void *addr, > (void *)mr); > mlx5_mr_rebuild_cache(&sh->share_cache); > /* > - * Flush local caches by propagating invalidation across cores. > - * rte_smp_wmb() is enough to synchronize this event. If one of > - * freed memsegs is seen by other core, that means the memseg > - * has been allocated by allocator, which will come after this > - * free call. Therefore, this store instruction (incrementing > - * generation below) will be guaranteed to be seen by other core > - * before the core sees the newly allocated memory. > + * No wmb is needed after updating dev_gen due to store-release of > + * unlock can provide the implicit wmb at the level visible by > + * software. This makes rebuilt global cache and updated dev_gen > + * be observed when local_cache starts to be updating by other > + * agents. The same as previous comment above. Please, apply the same comments to the mlx4 patch: http://patches.dpdk.org/project/dpdk/patch/20210517100002.19905-2-feifei.wa= ng2@arm.com/ With best regards, Slava