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 DFDDCA0519; Thu, 2 Jul 2020 23:15:34 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 2815C1D8F4; Thu, 2 Jul 2020 23:15:34 +0200 (CEST) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 9ADF61D8F3; Thu, 2 Jul 2020 23:15:31 +0200 (CEST) IronPort-SDR: 1G6DcsXRsg9QZmmIR7NZWHFAt5am9WlpS8X5uDaXVYcL1hzP5KMBxd8bPE+bPlRhiN4ubjpHJM Q7fFKDo+4GPA== X-IronPort-AV: E=McAfee;i="6000,8403,9670"; a="231893690" X-IronPort-AV: E=Sophos;i="5.75,305,1589266800"; d="scan'208";a="231893690" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 02 Jul 2020 14:15:30 -0700 IronPort-SDR: +9+UQXasHhxjEXKJB/UgzpkpBoafL2I+cAwVhIO6hhv8oBHJGTQdL1E7+EjLFb2y5wEVdvltyF GtDFjtM27PsQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.75,305,1589266800"; d="scan'208";a="314258649" Received: from orsmsx101.amr.corp.intel.com ([10.22.225.128]) by fmsmga002.fm.intel.com with ESMTP; 02 Jul 2020 14:15:30 -0700 Received: from orsmsx609.amr.corp.intel.com (10.22.229.22) by ORSMSX101.amr.corp.intel.com (10.22.225.128) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 2 Jul 2020 14:15:30 -0700 Received: from orsmsx606.amr.corp.intel.com (10.22.229.19) by ORSMSX609.amr.corp.intel.com (10.22.229.22) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Thu, 2 Jul 2020 14:15:29 -0700 Received: from ORSEDG002.ED.cps.intel.com (10.7.248.5) by orsmsx606.amr.corp.intel.com (10.22.229.19) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) id 15.1.1713.5 via Frontend Transport; Thu, 2 Jul 2020 14:15:29 -0700 Received: from NAM11-BN8-obe.outbound.protection.outlook.com (104.47.58.171) by edgegateway.intel.com (134.134.137.101) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 2 Jul 2020 14:15:29 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=fo8aZaHCKGDYp3PzlAWF8FZ25sJPbPYzdkoG19P4bgY7/PvVdZipMxQasMX7CoJXC94gagiTVj7v24n3gUpswa6kKdAzQEscU2ysFJ3hleCZDAlwDETfRaoRKQ78uPC533+78+fDkL3D3GK3c0Qb4IWi94Vo6sQR6C2a4jqBSSb5qtO4GPpAm5LefMVquowNPZKoPb4QIakZJXEhPFsv2eD8yUOur4/QjNgZYVYFP6GMykks9q4jH7d0ErsGsZbglV5Akez3IYESKs3uzLQYpc+gtHXHPxIp4AX04viYRIIFlJolQlSoBH8qjwyN07m8YqUaT4km7h93dAp6aU/t+g== 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=SVBhyxDt6CXfD9VgnEwZs9lwf2RXH7dXTdQ1750CSz0=; b=C2r2Iatc4iWbl3s5z4dwN+UUDo9lifFq8/LAboQR7SUceiHffUHuoGLAIv4NDLs6YxcHs3jtSP3nO42DRLCyckH+VEaSZR5+f2NUBVCmsg7d6U5FJVNgiVjycqA3OqscqJkLG/mw42XN9EDUH1pD0+db7MkCUVLXxzJCvyI/+4bkPHWagrnz+KhS5XYikePq59brKsOvHUnu+1LmJ4p8cQgTemndz8wjzYZ3Vag4WkY7w5/dSCB0n4OORydYkqWEiXl7w1VXw8jiycuxw8uh38xSG38g+Vds/Ou4neFXhaiVY3aJAgaWYJyfpUhfELKJ6bRXtBpv/e1M5TK4CI/VwA== 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=SVBhyxDt6CXfD9VgnEwZs9lwf2RXH7dXTdQ1750CSz0=; b=vkxngFT0LiWAj1DNz/5KZWzzTqZ5q5bv5mBB9TDjRWZobU0nWgR1AXBLgXWpJHwqeACP/EG01kN3HBSr5kld+d5GUQvrvv+K+sBtzO+7RgNaI5KU7EgQtgvjl9D9zX7UDIOUVCiuQrQ4pa+5qJ7jeYSH14rxv7GU+eNzH4vMynI= Received: from MWHPR1101MB2127.namprd11.prod.outlook.com (2603:10b6:301:58::20) by MWHPR11MB1712.namprd11.prod.outlook.com (2603:10b6:300:29::23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3131.21; Thu, 2 Jul 2020 21:15:27 +0000 Received: from MWHPR1101MB2127.namprd11.prod.outlook.com ([fe80::cd90:bf78:d3ce:abc6]) by MWHPR1101MB2127.namprd11.prod.outlook.com ([fe80::cd90:bf78:d3ce:abc6%11]) with mapi id 15.20.3153.027; Thu, 2 Jul 2020 21:15:27 +0000 From: "Carrillo, Erik G" To: Honnappa Nagarahalli , Phil Yang , "dev@dpdk.org" CC: "drc@linux.vnet.ibm.com" , Ruifeng Wang , Dharmik Thakkar , "stable@dpdk.org" , nd , "jerinj@marvell.com" , nd Thread-Topic: [PATCH 1/3] eventdev: fix race condition on timer list counter Thread-Index: AQHWQKt/5wmus6+dYkqGj9dsFfyoo6jehGawgAA2CICAFQViAIAACFCAgAEhj8A= Date: Thu, 2 Jul 2020 21:15:27 +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: 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.55.52.205] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 52fb5ff2-df06-4d92-154a-08d81ecd0b40 x-ms-traffictypediagnostic: MWHPR11MB1712: x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:883; x-forefront-prvs: 0452022BE1 x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: /7zARJek416NDIr+fDdKB0FkjF/+OGq5Lr/3FvXn1PXyHXj30P1Cpqm++YXxQU0AH/0osvLSX1TDhiY18K70ZZdpEQRjRwMqV1ZDmTMJQux8dSulrFru8QCag7bPHjjsy14sxjBJvW0ZvR+XyKuxDWOgUyygz5LBRcWrmEJA6ZBMgfa0uV1SDaCFncPZJXD7gxsZx9zIi1h91YEUfaW+5lWrEyi6o6rqsFGJn5kO+iC0GjCIFSrEOxUul/LedRs+4NIfS8gkSYHjFMqqx2SMdLWNxIAH744Pu20i8nLXXYuUZjJ9/y/0LnqpR9zRbfGhYoiwAOV8II7E6E65SsNgW1NvDkd30uystc1a/ghjQZmPIH7jBIQAXROORyBvAg5e x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:MWHPR1101MB2127.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFTY:; SFS:(4636009)(39860400002)(396003)(346002)(136003)(376002)(366004)(110136005)(55016002)(54906003)(83380400001)(71200400001)(316002)(33656002)(86362001)(2906002)(66946007)(8676002)(76116006)(64756008)(66556008)(66476007)(8936002)(66446008)(9686003)(5660300002)(26005)(53546011)(6506007)(7696005)(52536014)(4326008)(478600001)(186003)(21314003); DIR:OUT; SFP:1102; x-ms-exchange-antispam-messagedata: uri5pt/K8SK1VWUUrO+iMWwfw+OHoqXTwxGVg6u9acQLzpJ6WWQdII+Oa3ThD9htuu7I38+NZgq14mnLNmbC7/a8RErDVe/hHSbzGHyEcPgJCM3fn83SFUeZQu1nzzn0TIQh/0cuiHEOOs7t2hvWwG3LyTgREsRWb57sOt1nmuhGLNo84Tc/UYUGLR0iGbIHML/gP29b/mjVPOVqdViuAvloFdlx33uXNd/iQEb9sCCGdYI/uqDF54wZGQfX0I/RUtsHhYfoP1wR8zN7lSF465l+Bp4J567DDpz5G57bopYhyTDMCqitoaU34lKiFJZ/yc5a5JTsX/60UhxMPDIofwDhmJ+KJQjdnjSuMQyLKA9+b4VFCBt9E7lerAVyAQBJR5q5goEv+6AE1W4FzOxmNkTHccSWPFE6gGqTfp3zNXcOv3iYczcWsRjNRKXA4TOXz3oYgZjAxXK/MkFS5RLbUq54Y3or1B2yPLCAzKoBurk= x-ms-exchange-transport-forked: True Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: MWHPR1101MB2127.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 52fb5ff2-df06-4d92-154a-08d81ecd0b40 X-MS-Exchange-CrossTenant-originalarrivaltime: 02 Jul 2020 21:15:27.3735 (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: NPtw8GQYk3yliJ82zvPRvpy5PCp4MAXmhO5NS1aoIbW+8LkTSGerkR6Pue0CGNQ/e81C9bpgq+eXNqO5IanjuacNFG0E304uVQ+mGqaeaE8= X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR11MB1712 X-OriginatorOrg: intel.com 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" > -----Original Message----- > From: Honnappa Nagarahalli > Sent: Wednesday, July 1, 2020 10:56 PM > To: Phil Yang ; Carrillo, Erik G > ; dev@dpdk.org > Cc: drc@linux.vnet.ibm.com; Ruifeng Wang ; > Dharmik Thakkar ; stable@dpdk.org; nd > ; jerinj@marvell.com; Honnappa Nagarahalli > ; nd > Subject: RE: [PATCH 1/3] eventdev: fix race condition on timer list count= er >=20 > > > > > > > > > > > > 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. >=20 > > > > 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 > > > > 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 > > >