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 8B69FA0577; Sun, 5 Apr 2020 04:43:52 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id A605FF12; Sun, 5 Apr 2020 04:43:51 +0200 (CEST) Received: from EUR05-DB8-obe.outbound.protection.outlook.com (mail-db8eur05on2073.outbound.protection.outlook.com [40.107.20.73]) by dpdk.org (Postfix) with ESMTP id B0B88F12; Sun, 5 Apr 2020 04:43:50 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector2-armh-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=nDXkWKUcNQjTQ+ovXFM+9qQscHMfQJrFr1LDbNSjWK8=; b=y1vn46t5bD+xWq9jvSqpStJ55ZSA/lvE8ZwovyJnFAHVwmqAg/L7AamZQlxQp6iWltJv4vU4B8DSUf1DN2uX4g3K4Cwqd+2ScLYoH7UBs1k6allV/rUa1H9Ntuivn3kIiUzWBSTc5W9uT49YYj6GLxTXpb7okzMDjdNhV8Jokzc= Received: from AM0PR0202CA0021.eurprd02.prod.outlook.com (2603:10a6:208:1::34) by AM0PR08MB4531.eurprd08.prod.outlook.com (2603:10a6:208:13f::11) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2878.20; Sun, 5 Apr 2020 02:43:49 +0000 Received: from VE1EUR03FT042.eop-EUR03.prod.protection.outlook.com (2603:10a6:208:1:cafe::62) by AM0PR0202CA0021.outlook.office365.com (2603:10a6:208:1::34) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2878.16 via Frontend Transport; Sun, 5 Apr 2020 02:43:49 +0000 Authentication-Results: spf=pass (sender IP is 63.35.35.123) smtp.mailfrom=arm.com; dpdk.org; dkim=pass (signature was verified) header.d=armh.onmicrosoft.com;dpdk.org; dmarc=bestguesspass action=none header.from=arm.com; Received-SPF: Pass (protection.outlook.com: domain of arm.com designates 63.35.35.123 as permitted sender) receiver=protection.outlook.com; client-ip=63.35.35.123; helo=64aa7808-outbound-1.mta.getcheckrecipient.com; Received: from 64aa7808-outbound-1.mta.getcheckrecipient.com (63.35.35.123) by VE1EUR03FT042.mail.protection.outlook.com (10.152.19.62) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2856.17 via Frontend Transport; Sun, 5 Apr 2020 02:43:49 +0000 Received: ("Tessian outbound af37c2b81632:v50"); Sun, 05 Apr 2020 02:43:48 +0000 X-CR-MTA-TID: 64aa7808 Received: from 81b0b78872c5.1 by 64aa7808-outbound-1.mta.getcheckrecipient.com id 32E69A6C-67D9-4822-A50C-24D36DDC62BE.1; Sun, 05 Apr 2020 02:43:43 +0000 Received: from EUR03-DB5-obe.outbound.protection.outlook.com by 64aa7808-outbound-1.mta.getcheckrecipient.com with ESMTPS id 81b0b78872c5.1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384); Sun, 05 Apr 2020 02:43:43 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=IOIuRwtmjvfAYybIUXJ0SH87e1N2UEvDsDbGWsMdo+4Vrq0Q6eFXlREN/9/2TYGHl8lq0fvX9Uf1u7fiJf2EGNNRuMZB5bsBLC/Sd0W85C1y3YQMxEmlZMElI2Zrb0qJJd2BmYQ1//O++f9zTbn6I+otf21wvaWJtUw69hbT25+5Kx9cTl3F/8hXZYLmHVUGBWaJnGqP+N7MMa0o/VRo4Ubx8viIuy+yubigRl90rUQ1HHdsDEQsLwabv3ULIPA9wBj7JaIzBXRqzpY7qcjB43ALMoRL1yIiSqtJQN1cxUeRGQzi1nzQdWfsaccRA3ZTuZtDF0xVCny9/vXeRHXM0Q== 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=nDXkWKUcNQjTQ+ovXFM+9qQscHMfQJrFr1LDbNSjWK8=; b=BMuPWGLYoXCvBPk1Ve669GdMnyMWwxdZqhN6Ufb2m+InsnQJOcnZinbJ55hjPIdei040QaEJRm5/G1S5pBZRSBuvmHdUiGnqwqI+kDvck668YLNi2treSnMIFTMZup1ggG6ygHapdZz/6vWgtA3X6kGVATeZGN24NS5r6Np+Q9r/FIUzPUcjeYbHAkxc4iZvzBf2jxJ5qMPX4JIpos41lV96eFS95TNwWMl0rzjXrJo/aPe3y1M+oY2zKmFeZ3Ffbhq5apXIv47ZKeQ4hdb0CQYkMGZMnwsg33z3hOelu2ty3aGPHd5tIdqe14sHgvDDqiyGQn8POs4Fi+M+aH9usA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector2-armh-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=nDXkWKUcNQjTQ+ovXFM+9qQscHMfQJrFr1LDbNSjWK8=; b=y1vn46t5bD+xWq9jvSqpStJ55ZSA/lvE8ZwovyJnFAHVwmqAg/L7AamZQlxQp6iWltJv4vU4B8DSUf1DN2uX4g3K4Cwqd+2ScLYoH7UBs1k6allV/rUa1H9Ntuivn3kIiUzWBSTc5W9uT49YYj6GLxTXpb7okzMDjdNhV8Jokzc= Received: from DBBPR08MB4646.eurprd08.prod.outlook.com (10.255.79.144) by DBBPR08MB4332.eurprd08.prod.outlook.com (20.179.44.13) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2878.20; Sun, 5 Apr 2020 02:43:41 +0000 Received: from DBBPR08MB4646.eurprd08.prod.outlook.com ([fe80::1870:afc4:b90f:609d]) by DBBPR08MB4646.eurprd08.prod.outlook.com ([fe80::1870:afc4:b90f:609d%5]) with mapi id 15.20.2878.018; Sun, 5 Apr 2020 02:43:40 +0000 From: Honnappa Nagarahalli To: "Van Haaren, Harry" , Phil Yang , "thomas@monjalon.net" , "Ananyev, Konstantin" , "stephen@networkplumber.org" , "maxime.coquelin@redhat.com" , "dev@dpdk.org" CC: "david.marchand@redhat.com" , "jerinj@marvell.com" , "hemant.agrawal@nxp.com" , Gavin Hu , Ruifeng Wang , Joyce Kong , nd , "stable@dpdk.org" , Honnappa Nagarahalli , nd Thread-Topic: [PATCH v3 10/12] service: identify service running on another core correctly Thread-Index: AQHWCa8s1Z09TC3qMk29Ua7/hSq6qqhpxk+g Date: Sun, 5 Apr 2020 02:43:40 +0000 Message-ID: References: <1583999071-22872-1-git-send-email-phil.yang@arm.com> <1584407863-774-1-git-send-email-phil.yang@arm.com> <1584407863-774-11-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: x-ts-tracking-id: 90b6e600-71f1-453e-bf00-4922d5933731.0 x-checkrecipientchecked: true Authentication-Results-Original: spf=none (sender IP is ) smtp.mailfrom=Honnappa.Nagarahalli@arm.com; x-originating-ip: [70.113.25.165] x-ms-publictraffictype: Email X-MS-Office365-Filtering-HT: Tenant X-MS-Office365-Filtering-Correlation-Id: 657cc1e4-3407-45c7-f7a9-08d7d90b2b6e x-ms-traffictypediagnostic: DBBPR08MB4332:|DBBPR08MB4332:|AM0PR08MB4531: x-ld-processed: f34e5979-57d9-4aaa-ad4d-b122a662184d,ExtAddr x-ms-exchange-transport-forked: True X-Microsoft-Antispam-PRVS: x-checkrecipientrouted: true nodisclaimer: true x-ms-oob-tlc-oobclassifiers: OLM:5516;OLM:5516; x-forefront-prvs: 03648EFF89 X-Forefront-Antispam-Report-Untrusted: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:DBBPR08MB4646.eurprd08.prod.outlook.com; PTR:; CAT:NONE; SFTY:; SFS:(10009020)(4636009)(39850400004)(366004)(396003)(376002)(136003)(346002)(110136005)(54906003)(9686003)(86362001)(8936002)(316002)(5660300002)(55016002)(71200400001)(81166006)(81156014)(8676002)(7696005)(7416002)(33656002)(186003)(52536014)(64756008)(66946007)(66446008)(66556008)(66476007)(76116006)(6506007)(478600001)(26005)(2906002)(4326008); DIR:OUT; SFP:1101; received-spf: None (protection.outlook.com: arm.com does not designate permitted sender hosts) X-MS-Exchange-SenderADCheck: 1 X-Microsoft-Antispam-Untrusted: BCL:0; X-Microsoft-Antispam-Message-Info-Original: p5VbLs8NmB1ZJ4XVhiwp8gLNbhHTTC/PJXSlnqmQCoD9DKQnS+7c1DWN8FoZERYaH+7bcZQS+zJeM8nn4Bqp4140ceKmOSB6DDH6U3+REXPH4HRp5tIeDfWv/eg7Zq58UMCzB0J79xzVInA/CUYUHczOae/p03XowCglE2zYCBmGjVpYW0u0sq4Yo/kAVfmqDW55cdxEy/wp8BkcyyK0EIerDnsCyqGrCIoQM71uvZuxQFZB+s9onRPpoJcCE/r+vuFBCquvjycE6baHfxrjVCG6UZO7xrIJMTP+j6sTstYvPxZqZWQAmWJANTlkae7kZMDV4tjZZkxpxgyVMMcEaMg20jdH/W23t3qLvzOovOkXIM6Fo9IIGvrOPExVe2X4eFo6qHLKrSBQIj8CO7vp/OzLYits4DtO5Pw3gpSQJYnbnPcJ83iA2OlUltEHdGZR x-ms-exchange-antispam-messagedata: fV/WX1VToCg1uRU1ndn3NFOp0PEHdsXqYc5fu13+Vd2ifiAC/gD3xDQI1q/wcNXKJ9liaU/xhVyeAu/3teBxSWP6QBdjc2wFNku8cJiz5um9P/N6LJv66oWCZZ2xXmjJXAMiAFdErRIRpQVhy3N5AQ== Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-Transport-CrossTenantHeadersStamped: DBBPR08MB4332 Original-Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=Honnappa.Nagarahalli@arm.com; X-EOPAttributedMessage: 0 X-MS-Exchange-Transport-CrossTenantHeadersStripped: VE1EUR03FT042.eop-EUR03.prod.protection.outlook.com X-Forefront-Antispam-Report: CIP:63.35.35.123; CTRY:IE; LANG:en; SCL:1; SRV:; IPV:CAL; SFV:NSPM; H:64aa7808-outbound-1.mta.getcheckrecipient.com; PTR:ec2-63-35-35-123.eu-west-1.compute.amazonaws.com; CAT:NONE; SFTY:; SFS:(10009020)(4636009)(376002)(396003)(136003)(346002)(39850400004)(46966005)(6506007)(5660300002)(52536014)(86362001)(70586007)(316002)(26005)(8676002)(54906003)(2906002)(81156014)(70206006)(7696005)(110136005)(36906005)(81166006)(8936002)(55016002)(450100002)(9686003)(82740400003)(186003)(26826003)(33656002)(4326008)(356004)(478600001)(336012)(47076004); DIR:OUT; SFP:1101; X-MS-Office365-Filtering-Correlation-Id-Prvs: 76a866a2-2485-4c5f-5e6b-08d7d90b266b X-Forefront-PRVS: 03648EFF89 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: drnVfP+CNo/cDJ13Fy1Ywk7gVna4H9R0NfKkpNdEyVDT8c6Oah8j25tLkweWUvdDd/akyhDholaYN+hXn7x/U2NB/foxFeVHF23yYb6OI1D1GYpilzsoXliwKwmRlyASmzbachY1lu5Mb2yvehjVwCyfbOFhJLTzVdN9j1OT6XNrZVkO7Gu2chGhGqv/xcr1xQR+Ihgu3UyoPCdMn4h7aGfP3mbNx6diJN1d/IAxaKTP9O767OkIg+nbA3VCRpWdBn8OEEEMOuev4T4zQG6oYR2Gl/VM9pHzNQVMXEwylJFz9HAFXdIk2aMmsfeq8edHZddI30f8UQx0+lo5qY2cVurx1d9/rnQEJyeQX/YrkLCxtAuacoaDHODk/LCFbYpadXxpv8LjcyzQeTwREoasjFdqbmiCleD7OZ53meCHEncvCqLV4+eftj+4mW2bEWBn0OGkhBD0A+YFz+VnFAynAnF/Kcd4E5BEqSub8/pTCrAwP/F1ibThK/oSN56JZyL0E1rNURMwK/CtxRq4W/kF1A== X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 05 Apr 2020 02:43:49.1183 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 657cc1e4-3407-45c7-f7a9-08d7d90b2b6e X-MS-Exchange-CrossTenant-Id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=f34e5979-57d9-4aaa-ad4d-b122a662184d; Ip=[63.35.35.123]; Helo=[64aa7808-outbound-1.mta.getcheckrecipient.com] X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM0PR08MB4531 Subject: Re: [dpdk-dev] [PATCH v3 10/12] service: identify service running on another core correctly 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" > > Subject: [PATCH v3 10/12] service: identify service running on another > > core correctly > > > > From: Honnappa Nagarahalli > > > > The logic to identify if the MT unsafe service is running on another > > core can return -EBUSY spuriously. In such cases, running the service > > becomes costlier than using atomic operations. Assume that the > > application passes the right parameters and reduces the number of > > instructions for all cases. > > > > Cc: stable@dpdk.org > > > > Signed-off-by: Honnappa Nagarahalli > > Reviewed-by: Phil Yang > > Reviewed-by: Gavin Hu >=20 > Is this fixing broken functionality, or does it aim to only "optimize"? > Lack of "fixes" tag suggests optimization. Good point, it is missing the 'Fixes' tag. Relooking at the code, following= are few problems that I observe, please correct me if I am wrong: 1) Looking at the API rte_service_map_lcore_set, 'num_mapped_cores' keeps t= rack of the number of cores the service is mapped to. However, in the funct= ion 'rte_service_run_iter_on_app_lcore', the 'num_mapped_cores' is incremen= ted only if 'serialize_mt_unsafe' is set. This will return incorrect result= in 'rte_service_runstate_get' API when 'serialize_mt_unsafe' is not set. 2) Since incrementing 'num_mapped_cores' and checking its value is not an a= tomic operation, it introduces race conditions. Consider the current code s= nippet from the function 'rte_service_run_iter_on_app_lcore'. 1) if (serialize_mt_unsafe) 2) rte_atomic32_inc(&s->num_mapped_cores); 3) if (service_mt_safe(s) =3D=3D 0 && rte_atomic32_read(&s->num_mapped_core= s) > 1) { 4) if (serialize_mt_unsafe) 5) rte_atomic32_dec(&s->num_mapped_cores); 6) return -EBUSY; 7) } It is possible that more than 1 thread is executing line 3) concurrently, i= n which case all of them will hit line 6). Due to this, it is possible that= the service might never run or it might take multiple attempts to run the = service wasting cycles. If you agree these are bugs, I can add 'fixes' tag. >=20 > I'm cautious about the commit phrase "Assume that the application ...", i= f the > code was previously checking things, we must not stop checking them now, > this may introduce race-conditions in existing applications? What I meant here is, let us believe in what the application says. i.e. if = the applications sets 'serialize_mt_unsafe' to 1, let us assume that the se= rvice is infact 'unsafe'. >=20 > It seems like the "serialize_mt_unsafe" branch is being pushed further do= wn > the callgraph, and instead of branching over atomics this patch forces al= ways > executing 2 atomics? Yes, that is correct. Please see explanation in 1) above. >=20 > This feels like too specific an optimization/tradeoff, without data to ba= ckup > that there are no regressions on any DPDK supported platforms. Apologies for missing the 'Fixes' tag, this patch is a bug fix. >=20 > DPDK today doesn't have a micro-benchmark to gather such perf data, but I > would welcome one and we can have a data-driven decision. When this was developed, was the logic to avoid taking the lock measured to= provide any improvements? >=20 > Hope this point-of-view makes sense, -Harry >=20 > > --- > > lib/librte_eal/common/rte_service.c | 26 ++++++++------------------ > > 1 file changed, 8 insertions(+), 18 deletions(-) > > > > diff --git a/lib/librte_eal/common/rte_service.c > > b/lib/librte_eal/common/rte_service.c > > index 32a2f8a..0843c3c 100644 > > --- a/lib/librte_eal/common/rte_service.c > > +++ b/lib/librte_eal/common/rte_service.c > > @@ -360,7 +360,7 @@ service_runner_do_callback(struct > > rte_service_spec_impl *s, > > /* Expects the service 's' is valid. */ static int32_t > > service_run(uint32_t i, struct core_state *cs, uint64_t service_mask, > > - struct rte_service_spec_impl *s) > > + struct rte_service_spec_impl *s, uint32_t serialize_mt_unsafe) > > { > > if (!s) > > return -EINVAL; > > @@ -374,7 +374,7 @@ service_run(uint32_t i, struct core_state *cs, > > uint64_t service_mask, > > > > cs->service_active_on_lcore[i] =3D 1; > > > > - if (service_mt_safe(s) =3D=3D 0) { > > + if ((service_mt_safe(s) =3D=3D 0) && (serialize_mt_unsafe =3D=3D 1)) = { > > if (!rte_atomic32_cmpset((uint32_t *)&s->execute_lock, 0, 1)) > > return -EBUSY; > > > > @@ -412,24 +412,14 @@ rte_service_run_iter_on_app_lcore(uint32_t id, > > uint32_t > > serialize_mt_unsafe) > > > > SERVICE_VALID_GET_OR_ERR_RET(id, s, -EINVAL); > > > > - /* Atomically add this core to the mapped cores first, then examine i= f > > - * we can run the service. This avoids a race condition between > > - * checking the value, and atomically adding to the mapped count. > > + /* Increment num_mapped_cores to indicate that the service > > + * is running on a core. > > */ > > - if (serialize_mt_unsafe) > > - rte_atomic32_inc(&s->num_mapped_cores); > > + rte_atomic32_inc(&s->num_mapped_cores); > > > > - if (service_mt_safe(s) =3D=3D 0 && > > - rte_atomic32_read(&s->num_mapped_cores) > 1) { > > - if (serialize_mt_unsafe) > > - rte_atomic32_dec(&s->num_mapped_cores); > > - return -EBUSY; > > - } > > - > > - int ret =3D service_run(id, cs, UINT64_MAX, s); > > + int ret =3D service_run(id, cs, UINT64_MAX, s, serialize_mt_unsafe); > > > > - if (serialize_mt_unsafe) > > - rte_atomic32_dec(&s->num_mapped_cores); > > + rte_atomic32_dec(&s->num_mapped_cores); > > > > return ret; > > } > > @@ -449,7 +439,7 @@ service_runner_func(void *arg) > > if (!service_valid(i)) > > continue; > > /* return value ignored as no change to code flow */ > > - service_run(i, cs, service_mask, service_get(i)); > > + service_run(i, cs, service_mask, service_get(i), 1); > > } > > > > cs->loops++; > > -- > > 2.7.4