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 2FED6A0519; Thu, 2 Jul 2020 23:30:18 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 035AF1D933; Thu, 2 Jul 2020 23:30:17 +0200 (CEST) Received: from EUR03-VE1-obe.outbound.protection.outlook.com (mail-eopbgr50068.outbound.protection.outlook.com [40.107.5.68]) by dpdk.org (Postfix) with ESMTP id 758571D91B; Thu, 2 Jul 2020 23:30:16 +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=Vg6Hrai3IT85NV0VlGpayBCbAbrLPQWRFBwVlGhVBTw=; b=jSNoJdyVxZESS6w/t4M0zJ7l7FaK5I3/Bk7CWqL8aFha6vJPsrBFmjHXELu03jZiWAGj7ZSoEhIkWPerU/aFr+4pXDHVA3u5JmnHdvoejonKENm56F87DCCfY3YE9PEfGMOsVBj7MqdHI8NqdSicnkHQgvhhzspQ7AUS4Ffuzoo= Received: from AM6P192CA0071.EURP192.PROD.OUTLOOK.COM (2603:10a6:209:82::48) by DB6PR08MB2663.eurprd08.prod.outlook.com (2603:10a6:6:21::28) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3131.20; Thu, 2 Jul 2020 21:30:15 +0000 Received: from AM5EUR03FT030.eop-EUR03.prod.protection.outlook.com (2603:10a6:209:82:cafe::b6) by AM6P192CA0071.outlook.office365.com (2603:10a6:209:82::48) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3153.21 via Frontend Transport; Thu, 2 Jul 2020 21:30:15 +0000 X-MS-Exchange-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 AM5EUR03FT030.mail.protection.outlook.com (10.152.16.117) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3153.24 via Frontend Transport; Thu, 2 Jul 2020 21:30:15 +0000 Received: ("Tessian outbound 8239f48e56bd:v62"); Thu, 02 Jul 2020 21:30:14 +0000 X-CR-MTA-TID: 64aa7808 Received: from bbaadc33c48d.3 by 64aa7808-outbound-1.mta.getcheckrecipient.com id A7607426-45DE-413F-B3F2-2A1DB4E9A905.1; Thu, 02 Jul 2020 21:30:09 +0000 Received: from EUR05-AM6-obe.outbound.protection.outlook.com by 64aa7808-outbound-1.mta.getcheckrecipient.com with ESMTPS id bbaadc33c48d.3 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384); Thu, 02 Jul 2020 21:30:09 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=gCwGqj8Hd2dokJDjDM6l4ZVIF/76YEDlY8e2FtCLHETnfgrmEFRhlQEiM/Ij4LjiHLviNfcF68iy9hVreMULktvce92PbZefrxj+oFapnvHSTlm2+rR8FftI+c0RYqtenCZcMHPhm/CMm962XNQiwVLvNab75RIWxBhRR3/WfXWnj7zh4EQeT5zufIZFoqggOWcHqtJK6O1b9UjSjpu2c53mBySF0c3huuTZHl/RSGeY3sSyuG79HsXIVfw9aEKNI/sAdaqWjW8qTR9z7IN6eBKG8qp8huXE+lGk2GGl5XTQPU2Hzqq8ICBCHp5s4AlHdL3poiuAUqqncCGI9XDAxg== 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=Vg6Hrai3IT85NV0VlGpayBCbAbrLPQWRFBwVlGhVBTw=; b=ZnfVcu0hODRm5CUVPWUfaHe/qwyXQHtWqMWozXFkj1CUkjDCPI1atyCoKyFDWwjwGtLaBUl5msnj9+9Cl85DLpX8T0ou5FmeKFJ5cncfRBvk0qCTzcYs8xXX6rpyovhckGnFSU2FFmPfEdjcsTZD9xP5ch33a0vlZwINSnNXOt2U54xQxsDuBXzYvxzX/CWjpT55Y8fABgmxnlCq1UNl6ENji9CDSWkcdGepGfaV4nPfbQeBO40kVrhi5kHJ6SPJ00grLXe/cRiE4xO60Xy1Pma0NiLaoQx0atUp9z+dFgTKciy2p+qAH5m102F6XltIFY9uxGPug2EbUoP7yNe3qA== 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=Vg6Hrai3IT85NV0VlGpayBCbAbrLPQWRFBwVlGhVBTw=; b=jSNoJdyVxZESS6w/t4M0zJ7l7FaK5I3/Bk7CWqL8aFha6vJPsrBFmjHXELu03jZiWAGj7ZSoEhIkWPerU/aFr+4pXDHVA3u5JmnHdvoejonKENm56F87DCCfY3YE9PEfGMOsVBj7MqdHI8NqdSicnkHQgvhhzspQ7AUS4Ffuzoo= Received: from DB6PR0802MB2216.eurprd08.prod.outlook.com (2603:10a6:4:85::9) by DB6PR0801MB1734.eurprd08.prod.outlook.com (2603:10a6:4:31::8) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3131.20; Thu, 2 Jul 2020 21:30:07 +0000 Received: from DB6PR0802MB2216.eurprd08.prod.outlook.com ([fe80::9d1d:207b:e89d:199d]) by DB6PR0802MB2216.eurprd08.prod.outlook.com ([fe80::9d1d:207b:e89d:199d%10]) with mapi id 15.20.3153.023; Thu, 2 Jul 2020 21:30:07 +0000 From: Honnappa Nagarahalli To: "Carrillo, Erik G" , Phil Yang , "dev@dpdk.org" CC: "drc@linux.vnet.ibm.com" , Ruifeng Wang , Dharmik Thakkar , "stable@dpdk.org" , nd , "jerinj@marvell.com" , Honnappa Nagarahalli , nd Thread-Topic: [PATCH 1/3] eventdev: fix race condition on timer list counter Thread-Index: AQHWQKuA3yJ7DVsy8kmXELnLFbhrvKjeheQAgAABwBCAFTgsAIAAB3KAgAEjN4CAAAO2kA== Date: Thu, 2 Jul 2020 21:30:07 +0000 Message-ID: References: <1591960798-24024-1-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: c354d33e-cf57-41de-815c-e0f2fa4dddbc.0 x-checkrecipientchecked: true Authentication-Results-Original: intel.com; dkim=none (message not signed) header.d=none;intel.com; dmarc=none action=none header.from=arm.com; x-originating-ip: [217.140.111.135] x-ms-publictraffictype: Email X-MS-Office365-Filtering-HT: Tenant X-MS-Office365-Filtering-Correlation-Id: 4f6abb78-4892-4887-7d6d-08d81ecf1c1f x-ms-traffictypediagnostic: DB6PR0801MB1734:|DB6PR08MB2663: 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:883;OLM:883; x-forefront-prvs: 0452022BE1 X-MS-Exchange-SenderADCheck: 1 X-Microsoft-Antispam-Untrusted: BCL:0; X-Microsoft-Antispam-Message-Info-Original: wmuMqD5wdMiqeK6jKe4/nHCKCtAUp7QgIQ3JMNsCG+nw/TyLrVs7dwQhAjaGMiqQPVlsIYz8CtuZFiaymRsY2Fnp9yhZ9g1H42DNZgkYF49dsXH4LRjQQrfrjMi1ZS1LEUtID7n50r9IcFpZjH662BujXmlKaPsQEfslQBiQdHu02v8RE+r8aiXlNSIXlPxnhOInKg8qx/eFKmtm/ES/pIPg4lTk+pvuRLXxLzmbaGh7HBOK6eRd3Q/XxZV0N0OxibtOnGaIV+rWI/lvG7Hhrf8fXEY3GuEheQQSmgEk7OaG7s7VJlAiXQWc+iQx+3PoVfFhdOgfHMoAtj9svGzKTHCujLdz3kfdOB6H65BFBwZYzOGQ8qQJc/59HK6fCMfi X-Forefront-Antispam-Report-Untrusted: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:DB6PR0802MB2216.eurprd08.prod.outlook.com; PTR:; CAT:NONE; SFTY:; SFS:(4636009)(376002)(366004)(396003)(346002)(39860400002)(136003)(478600001)(186003)(54906003)(110136005)(316002)(9686003)(71200400001)(55016002)(66476007)(5660300002)(86362001)(66556008)(66946007)(83380400001)(66446008)(64756008)(76116006)(52536014)(2906002)(4326008)(7696005)(6506007)(33656002)(26005)(8936002)(53546011)(8676002)(21314003); DIR:OUT; SFP:1101; x-ms-exchange-antispam-messagedata: RA9FbE0Hc1FQSfR3SjHXpfIbZYmSth7UCUq24fPfP8hW45hB1NnL6mR2iI8n41YzOuThrPgBeqVlGJJBXoj9kX9D9Ta7b1bEvTMdaigyhse8tIAsCNHHr1QjXU5EufyaMscqhNPqGi6sAF9DZkWfAmS1yPWJguIOR66q8rjBvysp/BK60ZskYf6GNKLR47vfOIlIqyvuHtVDOB7ApqaCaS4qEd0gsaZAn+Y7EanSeLLaqnquFuriU/5H6n/ARA+gjg4F08O9XbfAhb7vXWRuWfocb81iRJ3a6ZtjNbtUWwFk96dJ+B+mkp+mTmT3N+m0fsKPa8AQQ30DmxqLUIYA58VZ0L2f6PAmOG7/1haZUkQwM04zmzC9h/VdUc+uKaxIo1Vma/wBHwCikqiWicvnmke7HZZbu6Zjf54WCSTfZ6pdjCeAelmedR0nfhihJpPh05UIrIWfRrOu89NkWRjWsjFc4++lxdL+ouhoOXGd1HAqG2c8kWBK21SyAmADmIPf Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB6PR0801MB1734 Original-Authentication-Results: intel.com; dkim=none (message not signed) header.d=none;intel.com; dmarc=none action=none header.from=arm.com; X-EOPAttributedMessage: 0 X-MS-Exchange-Transport-CrossTenantHeadersStripped: AM5EUR03FT030.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:(4636009)(39860400002)(376002)(136003)(346002)(396003)(46966005)(2906002)(26005)(55016002)(8676002)(8936002)(356005)(478600001)(82310400002)(81166007)(83380400001)(86362001)(33656002)(47076004)(82740400003)(5660300002)(52536014)(186003)(70586007)(110136005)(70206006)(54906003)(336012)(4326008)(53546011)(7696005)(6506007)(36906005)(450100002)(9686003)(316002)(21314003); DIR:OUT; SFP:1101; X-MS-Office365-Filtering-Correlation-Id-Prvs: 1277d26b-5308-4b46-a314-08d81ecf17a1 X-Forefront-PRVS: 0452022BE1 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: Y637mspGSqjJHVZRUjaQKkEE6CUOxlkYc9uZzTwfaJbI45nUelJm0BoRd4EwvsLx/3LmyZWA/usCrM8U2T9pLp5IX6EBc+KAISzpdAWV+zGXD8ZvCj6T98DmyJK1aMBu4uwxO7hY0waWW/J3KX3hmlc/z8bMXtrN/nJpuIYFpNInRWNrluHU4bHALiDxKZvkdGt602n4FkwBh9kaPqaj/eJ7Cr+AEPenzgWe5EjJRR5TA3rYKnpd5OmBJrhGAuCBRl95O/1jwzKMCdxjCxtoxrBYWABYXRHqdm6qWHUy10N7kmyU32yXyj5CFsXqASxHGwaxXkT6b5/JkMALXw2JE5bbNB/sVDxxZdjaJ5hNKrBjSNp/IpTukP+8OYwSPdfUibgJMYqfubn/mczC+Tz9/q5ppTGEJJwB8DTQq1C3bu4= X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 Jul 2020 21:30:15.1024 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: 4f6abb78-4892-4887-7d6d-08d81ecf1c1f 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-AuthSource: AM5EUR03FT030.eop-EUR03.prod.protection.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: DB6PR08MB2663 Subject: Re: [dpdk-dev] [PATCH 1/3] eventdev: fix race condition on timer list counter 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" > > > > > > > > > > > > > > Hi Phil, > > > > > > > > > > Good catch - thanks for the fix. I've commented in-line: > > > > > > > > > > > -----Original Message----- > > > > > > From: Phil Yang > > > > > > Sent: Friday, June 12, 2020 6:20 AM > > > > > > To: dev@dpdk.org; Carrillo, Erik G > > > > > > Cc: drc@linux.vnet.ibm.com; honnappa.nagarahalli@arm.com; > > > > > > ruifeng.wang@arm.com; dharmik.thakkar@arm.com; nd@arm.com; > > > > > > stable@dpdk.org > > > > > > Subject: [PATCH 1/3] eventdev: fix race condition on timer > > > > > > list counter > > > > > > > > > > > > The n_poll_lcores counter and poll_lcore array are shared > > > > > > between lcores and the update of these variables are out of > > > > > > the protection of spinlock on each lcore timer list. The > > > > > > read-modify-write operations of the counter are not atomic, so > > > > > > it has the potential of race condition > > > > > between lcores. > > > > > > > > > > > > Use c11 atomics with RELAXED ordering to prevent confliction. > > > > > > > > > > > > Fixes: cc7b73ea9e3b ("eventdev: add new software timer > > > > > > adapter") > > > > > > Cc: erik.g.carrillo@intel.com > > > > > > Cc: stable@dpdk.org > > > > > > > > > > > > Signed-off-by: Phil Yang > > > > > > Reviewed-by: Dharmik Thakkar > > > > > > Reviewed-by: Ruifeng Wang > > > > > > --- > > > > > > lib/librte_eventdev/rte_event_timer_adapter.c | 16 > > > > > > ++++++++++++---- > > > > > > 1 file changed, 12 insertions(+), 4 deletions(-) > > > > > > > > > > > > diff --git a/lib/librte_eventdev/rte_event_timer_adapter.c > > > > > > b/lib/librte_eventdev/rte_event_timer_adapter.c > > > > > > index 005459f..6a0e283 100644 > > > > > > --- a/lib/librte_eventdev/rte_event_timer_adapter.c > > > > > > +++ b/lib/librte_eventdev/rte_event_timer_adapter.c > > > > > > @@ -583,6 +583,7 @@ swtim_callback(struct rte_timer *tim) > > > > > > uint16_t nb_evs_invalid =3D 0; uint64_t opaque; int ret; > > > > > > +int n_lcores; > > > > > > > > > > > > opaque =3D evtim->impl_opaque[1]; adapter =3D (struct > > > > > > rte_event_timer_adapter *)(uintptr_t)opaque; @@ > > > > > > -605,8 +606,12 @@ swtim_callback(struct rte_timer *tim) > > > > > > "with immediate expiry value"); } > > > > > > > > > > > > -if (unlikely(rte_atomic16_test_and_set(&sw- > > > > > > >in_use[lcore].v))) > > > > > > -sw->poll_lcores[sw->n_poll_lcores++] =3D lcore; > > > > > > +if (unlikely(rte_atomic16_test_and_set(&sw- > > > > > > >in_use[lcore].v))) { > > > > > > +n_lcores =3D __atomic_fetch_add(&sw->n_poll_lcores, > > > > > > 1, > > > > > > +__ATOMIC_RELAXED); > > > > Since this commit will be back ported, we should prefer to use > > > > rte_atomic APIs for this commit. Otherwise, we will have a mix of > > > > rte_atomic and C11 APIs. > > > > My suggestion is to fix this bug using rte_atomic so that > > > > backported code will have only rte_atomic APIs. Add another commit > > > > (if > > > > required) in this series to make the bug fix use C11 APIs (this > > > > commit will not be > > > backported). > > > > > > Hi Honnappa, > > > > > > It doesn't have an applicable rte_atomic_XXX API to fix this issue. > > > The rte_atomic32_inc doesn't return the original value of the input > > > parameter and rte_atomic32_add_return can only return the new value. > > Ok, understood. > > > > > > > > Meanwhile, the rte_timer_alt_manage & rte_timer_stop_all API not > > > support rte_atomic type parameters. We might need to rewrite these > > > two APIs if we want to use rte_atomic operations for n_pol_lcores > > > and > > poll_lcores array. > > > > > > So, a better solution could be to backport the entire c11 solution > > > to stable releases. > > I am ok with the approach. > > Erik, are you ok with this? > > >=20 > Yes, I'm OK with that as well. Thanks Erik. I think we need to add stable@dpdk.org to Cc for all the commi= ts in the series. >=20 > Thanks, > Erik >=20 > > > > > > Thanks, > > > Phil > > > > > > > > > > > > > > > > > Just a nit, but let's align the continued line with the opening > > > > > parentheses in this location and below. With these changes: > > > > > > > > > > Acked-by: Erik Gabriel Carrillo > > > > > > > > > > > +__atomic_store_n(&sw->poll_lcores[n_lcores], > > > > > > lcore, > > > > > > +__ATOMIC_RELAXED); > > > > > > +} > > > > > > } else { > > > > > > EVTIM_BUF_LOG_DBG("buffered an event timer expiry event"); > > > > > > > > > > > > @@ -1011,6 +1016,7 @@ __swtim_arm_burst(const struct > > > > > > rte_event_timer_adapter *adapter, uint32_t lcore_id =3D > > > > > > rte_lcore_id(); struct rte_timer *tim, *tims[nb_evtims]; > > > > > > uint64_t cycles; > > > > > > +int n_lcores; > > > > > > > > > > > > #ifdef RTE_LIBRTE_EVENTDEV_DEBUG > > > > > > /* Check that the service is running. */ @@ -1033,8 +1039,10 > > > > > > @@ __swtim_arm_burst(const struct rte_event_timer_adapter > > > > > > *adapter, if > > > > > > (unlikely(rte_atomic16_test_and_set(&sw->in_use[lcore_id].v))) > > > > > > { EVTIM_LOG_DBG("Adding lcore id =3D %u to list of lcores to > > > > > poll", > > > > > > lcore_id); > > > > > > -sw->poll_lcores[sw->n_poll_lcores] =3D lcore_id; > > > > > > -++sw->n_poll_lcores; > > > > > > +n_lcores =3D __atomic_fetch_add(&sw->n_poll_lcores, 1, > > > > > > +__ATOMIC_RELAXED); __atomic_store_n(&sw- > > >poll_lcores[n_lcores], > > > > > > +lcore_id, __ATOMIC_RELAXED); > > > > > > } > > > > > > > > > > > > ret =3D rte_mempool_get_bulk(sw->tim_pool, (void **)tims, > > > > > > -- > > > > > > 2.7.4 > > > >