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 C78CEA04AD; Fri, 1 May 2020 16:22:00 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id A28F71DA62; Fri, 1 May 2020 16:22:00 +0200 (CEST) Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id ADBA31D99E; Fri, 1 May 2020 16:21:57 +0200 (CEST) IronPort-SDR: q0unIlAFbkqpluLYMAzEBd7MPPodnchyFHjOeGhMO0R1sGBB0q7WRl6kzKjq6EkDGWBxTXAEUA sCzXCchDeQvA== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 01 May 2020 07:21:56 -0700 IronPort-SDR: vECN9gn4vNwOKy/s8LydHLnzEQB5f/dl71f5KXw406Jb+8Hzj6ds9BDBCu4BLv3bcO1vie3zhz X4jt1G1HGv/g== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.73,339,1583222400"; d="scan'208";a="460274701" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by fmsmga006.fm.intel.com with ESMTP; 01 May 2020 07:21:56 -0700 Received: from fmsmsx161.amr.corp.intel.com (10.18.125.9) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.439.0; Fri, 1 May 2020 07:21:56 -0700 Received: from FMSEDG001.ED.cps.intel.com (10.1.192.133) by FMSMSX161.amr.corp.intel.com (10.18.125.9) with Microsoft SMTP Server (TLS) id 14.3.439.0; Fri, 1 May 2020 07:21:55 -0700 Received: from NAM04-BN3-obe.outbound.protection.outlook.com (104.47.46.55) by edgegateway.intel.com (192.55.55.68) with Microsoft SMTP Server (TLS) id 14.3.439.0; Fri, 1 May 2020 07:21:55 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=OQltfjk9qZFkWBqnY2GiDBXgVBAr3aT1xgXV+JRPlpjgo3m95p+K37YFjE2MdlFpKMZ7kh/fhlsUvuXSx97L0jVV0Fzc4L6vAaiB/rfkFk6r0lnOkJx3emam++u+ahRVfrrmMk3EBhp4+Tmros6D6Ym2MfzW/43/OUn+d4tvJF9qqtgN01ocxcirqkUKDRRQcDRa12cWqFPpJSO2mZIYyqr7vX27wuC3gEvfOzrC3dO0dknoj8Smo3zaVfmENvDjQW+T7W2KA3alTX3Wp6RCC+GNRso6b33WhXDHPDVb9sfpXQbqGuVe8WUzqkvYDkC7SK+4rJYPGHpX62lOkoC7Ng== 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=/PDpHqn52V7DzQV8HTCDONPMugWERQI28fJEQ87KQGA=; b=A25Xjn9/Kq8W5vMhBGzMjzffnnRmuOs5/xJpnYL462KiBA8UICj1KiZ4Ky455C6/7x/ELPCHZSYj4hhfWcYlW5Q4ImkMcrj6KWy/DesoTp6Rm7DYWlYZEFSukIsCqxikrXAXY97E5Lgu0PUQnm8CVCsPelx4uwCfKNFJCmSy72IItC/BxgvIq2pi0UEIanWih5oawPIzoy0g8b19SaPi5r13XAsvnn5wkWIKmpDgWKfVYlsL+immDi7vOC32gT61LjKj7HS89nWxNUm0dzUZ0YP49ual9F7rMg5sRXTajRqC1JIc2dH6518oUtwnbbwIy6qpMtk4Rqvmfouk21gyQQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel.onmicrosoft.com; s=selector2-intel-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=/PDpHqn52V7DzQV8HTCDONPMugWERQI28fJEQ87KQGA=; b=GnGQ2eSvV0XRUSU80RqiQbSRDNO3PwNi5cNBz7DOpBRyC85evWL+VOHX3GINKe9LBhjWodps7U6/Kwek26yeasY0I0TSkWHQFS7GUTZomWw0WMtjR717koqSCF4eKqRz5MC2z56DmQI+1sKEQocyaX9rV1MY/C8bzu9oGQ2CpUE= Received: from BYAPR11MB3143.namprd11.prod.outlook.com (2603:10b6:a03:92::32) by BYAPR11MB3142.namprd11.prod.outlook.com (2603:10b6:a03:85::25) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2937.13; Fri, 1 May 2020 14:21:53 +0000 Received: from BYAPR11MB3143.namprd11.prod.outlook.com ([fe80::5917:9757:49b0:3c49]) by BYAPR11MB3143.namprd11.prod.outlook.com ([fe80::5917:9757:49b0:3c49%5]) with mapi id 15.20.2937.023; Fri, 1 May 2020 14:21:53 +0000 From: "Van Haaren, Harry" To: Honnappa Nagarahalli , Phil Yang , "dev@dpdk.org" CC: "thomas@monjalon.net" , "david.marchand@redhat.com" , "Ananyev, Konstantin" , "jerinj@marvell.com" , "hemant.agrawal@nxp.com" , Gavin Hu , nd , "stable@dpdk.org" , "Eads, Gage" , "Richardson, Bruce" , nd Thread-Topic: [PATCH v2 1/6] service: fix race condition for MT unsafe service Thread-Index: AQHWGYzR0pQ9nE/SWEGnc1eZ3IqbiqiQVgOQgABnoYCAApK+kA== Date: Fri, 1 May 2020 14:21:53 +0000 Message-ID: References: <1584407863-774-8-git-send-email-phil.yang@arm.com> <1587659482-27133-1-git-send-email-phil.yang@arm.com> <1587659482-27133-2-git-send-email-phil.yang@arm.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-reaction: no-action dlp-version: 11.2.0.6 authentication-results: arm.com; dkim=none (message not signed) header.d=none;arm.com; dmarc=none action=none header.from=intel.com; x-originating-ip: [192.198.151.170] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: f3986c33-6d3f-4770-5fec-08d7eddaff25 x-ms-traffictypediagnostic: BYAPR11MB3142: x-ld-processed: 46c98d88-e344-4ed4-8496-4ed7712e255d,ExtAddr x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:10000; x-forefront-prvs: 0390DB4BDA x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:BYAPR11MB3143.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFTY:; SFS:(396003)(366004)(376002)(346002)(136003)(39860400002)(186003)(316002)(26005)(7696005)(478600001)(8676002)(4326008)(33656002)(7416002)(8936002)(2906002)(55016002)(110136005)(76116006)(6506007)(5660300002)(66476007)(71200400001)(66946007)(64756008)(66446008)(54906003)(86362001)(52536014)(66556008)(53546011)(9686003); DIR:OUT; SFP:1102; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: LAm13wunsO4ETfH8nMKwCjbnBCEMwKDNgzb4QbekQ08aLUtJk0zmzw2WmOvj88w6S7uW6s3ageyW6wElTBVgsYlg7p+j+4r8i5c2sRRKSmv0K+JvCSSk4DTgYJI57kNHETXibvpe5DFoXxCkuhqq1lB8FW0lcDejElsO/7fzdtf+ptzUDovrFGv7bTxTF2oMnhWbDz6QuJ+kj2i6ISNPMab2TKKFkXbb2XE2wUN5BX2lne6sfRtx6sz72ImeRTtNzWv7pBJ5w1v2Av6O0t8gKFA+oG3Dn9udb3wxhE8GFE7ZEd5v8LWPL6tJUoT7rcfCgcXMp18W6eFCyBdYUl0mDHrzrofl4287rsjIaIih2EXJf+RPz6V+0LVD4wOop46r4G2FNWZJltrjeLXcPUcrP5faQjMcG6gc4JTp4mw+IqHE8V1F+1ANKIVeZOMjO6hP x-ms-exchange-antispam-messagedata: qVTSeP8x6eIQ5CNaYH4CbIxRevaQP+00sVyBVC6pIFH0cmLkT2/qVsbBdQK0NASpnT9ZRntDXYClj0ABoyn47iuAP7VBRvUx5m3BCEhIQber4qesRDIOuORZKnDRqdL/fRR4ikxrfwIimDKIi1PQd3WVzFFC4+kNr+IzAw45Ze+oxk3T1o12cP5+BpFd6gSVDv/7FJR1S5ut7Q1N6JVUWVNC1jlxJYT1+OxvHtr/Nund2Gj1xIUQuoUvo/B+qz6PLMpMtQtNCb2Eswgiv2ynH4AOzHuqLPExVyoYf8g2alXU4xcIg+TtUFhnsaw+PO7xwl+ZRSNY3N/nlVK0sOTqx/CCsTYuS1z88UODXfZwaLO4c59UkRUMitFpVuVNvHnIFxTJr4zPvE+nqF0/cxncMlQV4fOm9267w9zIT9sRqNWXBvxgJ0/CuVQVt+W1TceiotHDxQEzdZREEv1ABFwj1nlbyckMXrAl21r/F8bDEgsgVOhbFnsacm6GbwSU/SHzbZJWq44uJxq393DM1LnAa6TZA9tA2x0joQ3ymegperpYrRBE6yptw+0yki4Fns8CpuQ91WsKnEVl1jCmAcCRcewvz6u6JtXGd9BVdJfuGBAW/oFPH3fJ6ksLwg640T3g9WG0Knb+ucWzCdLfRNL1l0eMjBf9I5N6+uDV9xxxiHgSyyjnJRNoKGeNxYtaCczVtGNyU3IfrsM9WHQEktQRvwlH/QPA/kkiql6hr5ob1CVELH+1/WllGMWSKyvyWkvkJlOAXz1IwzEOkzxQQtbEZ/bY7qI/xG7vNZPVWh6l5GOEXA0wxNah+JIrdROswAcH Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-CrossTenant-Network-Message-Id: f3986c33-6d3f-4770-5fec-08d7eddaff25 X-MS-Exchange-CrossTenant-originalarrivaltime: 01 May 2020 14:21:53.3768 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: Y1Cf6UyERIfALHl0Sx1SLJGg2NMytwhq2cw2rzr4BbA4HXPB7Wmoq02ZtZL14DaTbNijo0VtzPZg2q7ZEPNgrLjuZJqq/txTHN7YNZVe4iE= X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR11MB3142 X-OriginatorOrg: intel.com Subject: Re: [dpdk-dev] [PATCH v2 1/6] service: fix race condition for MT unsafe service 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" > -----Original Message----- > From: Honnappa Nagarahalli > Sent: Wednesday, April 29, 2020 11:49 PM > To: Van Haaren, Harry ; Phil Yang > ; dev@dpdk.org > Cc: thomas@monjalon.net; david.marchand@redhat.com; Ananyev, Konstantin > ; jerinj@marvell.com; > hemant.agrawal@nxp.com; Gavin Hu ; nd > ; stable@dpdk.org; Eads, Gage ; > Richardson, Bruce ; Honnappa Nagarahalli > ; nd > Subject: RE: [PATCH v2 1/6] service: fix race condition for MT unsafe ser= vice >=20 > Hi Harry, > Thanks for getting back on this. >=20 > >=20 > > > Subject: [PATCH v2 1/6] service: fix race condition for MT unsafe > > > service > > > > > > From: Honnappa Nagarahalli > > > > > > The MT unsafe service might get configured to run on another core > > > while the service is running currently. This might result in the MT > > > unsafe service running on multiple cores simultaneously. Use > > > 'execute_lock' always when the service is MT unsafe. > > > > > > Fixes: e9139a32f6e8 ("service: add function to run on app lcore") > > > Cc: stable@dpdk.org > > > > > > Signed-off-by: Honnappa Nagarahalli > > > Reviewed-by: Phil Yang > > > --- > > > > Thanks for spinning a new revision - based on ML discussion previously,= it > > seems like the "use service-run-count" to avoid this race would be a co= mplex > > solution. > > > > Suggesting the following; > > 1) Take the approach as per this patch, to always take the atomic, fixi= ng the > > race condition. > Ok I've micro-benchmarked this code change inside the service cores autotest, = and it introduces around 35 cycles of overhead per service call. This is not idea= l, but given it's a bugfix, and by far the simplest method to fix this race-condition. H= aving discussed and investigated multiple other solutions, I believe this is the = right solution. Thanks Honnappa and Phil for identifying and driving a solution. I suggest to post the benchmarking unit-test addition patch, and integrate = that *before* the series under review here gets merged? This makes benchmarking of the "before bugfix" performance in future easier should it be required. > > 2) Add an API to service-cores, which allows "committing" of mappings. > > Committing the mapping would imply that the mappings will not be change= d > > in future. With runtime-remapping being removed from the equation, the > > existing branch-over-atomic optimization is valid again. > Ok. Just to make sure I understand this: > a) on the data plane, if commit API is called (probably a new state varia= ble) and > num_mapped_cores is set to 1, there is no need to take the lock. > b) possible implementation of the commit API would check if > num_mapped_cores for the service is set to 1 and set a variable to indica= te that > the lock is not required. >=20 > What do you think about asking the application to set the service capabi= lity to > MT_SAFE if it knows that the service will run on a single core? This woul= d > require us to change the documentation and does not require additional co= de. That's a nice idea - I like that if applications want to micro-optimize aro= und the atomic, that they have a workaround/solution to do so, particularly tha= t it doesn't require code-changes and backporting. Will send review and send feedback on the patches themselves. Regards, -Harry > > So this would offer applications two situations > > A) No application change: possible performance regression due to atomic > > always taken. > > B) Call "commit" API, and regain the performance as per previous DPDK > > versions. > > > > Thoughts/opinions on the above? I've flagged the rest of the patchset = for > > review ASAP. Regards, -Harry > > > > > lib/librte_eal/common/rte_service.c | 11 +++++------ > > > 1 file changed, 5 insertions(+), 6 deletions(-)