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 5652DA00C3; Fri, 24 Apr 2020 03:27:05 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 9255B28EE; Fri, 24 Apr 2020 03:27:04 +0200 (CEST) Received: from mga17.intel.com (mga17.intel.com [192.55.52.151]) by dpdk.org (Postfix) with ESMTP id 770C31C192 for ; Fri, 24 Apr 2020 03:27:02 +0200 (CEST) IronPort-SDR: ESuaHdtaV6nlx8Nc03eeR+ZEjtslK52WBMr1FZqgzChvCIHRW/TZDSiXciegpOYjfYmyYxKije jzKCVo+oLd2Q== X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 23 Apr 2020 18:27:01 -0700 IronPort-SDR: 17EREcK1nTrFbZqPp0zAouUDDFZ8JbhkHMiZ1Jb1Bb5N8k4BfGaRIPmp4XQf0dkuIPGelxXdkh xvGrMeSEWHuQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.73,309,1583222400"; d="scan'208";a="457149284" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by fmsmga005.fm.intel.com with ESMTP; 23 Apr 2020 18:27:01 -0700 Received: from fmsmsx157.amr.corp.intel.com (10.18.116.73) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 23 Apr 2020 18:26:42 -0700 Received: from FMSEDG002.ED.cps.intel.com (10.1.192.134) by FMSMSX157.amr.corp.intel.com (10.18.116.73) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 23 Apr 2020 18:26:41 -0700 Received: from NAM02-CY1-obe.outbound.protection.outlook.com (104.47.37.51) by edgegateway.intel.com (192.55.55.69) with Microsoft SMTP Server (TLS) id 14.3.439.0; Thu, 23 Apr 2020 18:26:41 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Ws2u74vfNYqbAmZ0mUfHVMbWqvKHJBwWhzvY+ZMP/qGjjBtUWzFrW8//69SRRl3d3rPsl/WoxDpECSP2XppGYdZNybcflR6imDUHRXjn31DEUJzXyAt6qavlBDb/vRF6TNHT3pmHh0CqPzsIiyIdLCQqEvFLKwAT1VRZSdo4CD82s0ctN0kuQs5uyLcKoGib5c9VgVF1NjRl0ege6i3dNnyHHvMRRRm1yhcf2aR7UqNGZbFoohTNkYbvUuXxrFdjqQfbiHQnbCpS5k9qcPhzhSj1A/LKr9EMMbjs0YkmpMpSf2eYdlqLiu3E++O/v6xCOGd4JSB7Mif0C2PXV+K1PA== 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=3e3B8+Ae7v7iB7DfUblyGzbOuldSa+hplXi6LrYks1Q=; b=RalPnkTs7knCubstdFmwvV9ZbSK0iUvKja+pNPrlDQCU1TG67R9Twc459lXBrKDF9OZUjB0W27+6J4HvPeP/Xth946P0X1JxkTc6YeSTAeh/rVkkDK/5vcnLM5IcJe1qyUPKAXaTcU6cyW+zyFb9kTGis9J3TBkdHB1PWeCv+zV1XEyjM7Ixr+l3AnziGrWYER5d1YgXlCxFZkHuH/xf+rZ18enK2VQRIkQgrDd9tZJx2JD9R/bjjfgezNNge0mgavaPwL/o1/pSRSvnGw/Aae1ngWNdu4DIKF9eRSa06VUFDoCz56VqK9TUUVPtxKYHZ7TrHHXP89SYA0fCZRXUqQ== 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=3e3B8+Ae7v7iB7DfUblyGzbOuldSa+hplXi6LrYks1Q=; b=pyOaGAIjnNE4iUmkGDuMWYtNejQorNE4GrF2RH2whnO1H31H8Olh4jfPaFbm0N6W18ENKcPtJeDFwrAQF6iv98SThwb9tK8Lyv3wa5esXyWMo7iqGMqRFxTMq4xkhrEStoz4DYWYJy7uvyD2jXESvHP0Z6v0KAuv+QWBtVb6BZc= Received: from SA0PR11MB4656.namprd11.prod.outlook.com (2603:10b6:806:96::23) by SA0PR11MB4637.namprd11.prod.outlook.com (2603:10b6:806:97::16) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2921.27; Fri, 24 Apr 2020 01:26:40 +0000 Received: from SA0PR11MB4656.namprd11.prod.outlook.com ([fe80::6400:b873:7752:50b4]) by SA0PR11MB4656.namprd11.prod.outlook.com ([fe80::6400:b873:7752:50b4%4]) with mapi id 15.20.2937.012; Fri, 24 Apr 2020 01:26:40 +0000 From: "Carrillo, Erik G" To: Honnappa Nagarahalli , Phil Yang , "rsanford@akamai.com" , "dev@dpdk.org" CC: "thomas@monjalon.net" , "david.marchand@redhat.com" , "Ananyev, Konstantin" , "jerinj@marvell.com" , "hemant.agrawal@nxp.com" , Gavin Hu , nd , nd Thread-Topic: [PATCH v2] lib/timer: relax barrier for status update Thread-Index: AQHWFy3P3RvZBS5Al0in0ZaJ5Nm9D6iHJu4AgABWvYA= Date: Fri, 24 Apr 2020 01:26:40 +0000 Message-ID: References: <1582526539-14360-2-git-send-email-phil.yang@arm.com> <1587398752-9345-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: spf=none (sender IP is ) smtp.mailfrom=erik.g.carrillo@intel.com; x-originating-ip: [136.49.135.17] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 87bd30bf-62f3-4b72-3fd1-08d7e7ee8a8c x-ms-traffictypediagnostic: SA0PR11MB4637: x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:10000; x-forefront-prvs: 03838E948C x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:SA0PR11MB4656.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFTY:; SFS:(10019020)(39860400002)(396003)(136003)(346002)(376002)(366004)(478600001)(4326008)(86362001)(186003)(966005)(76116006)(66476007)(52536014)(64756008)(66556008)(66946007)(66446008)(5660300002)(33656002)(7696005)(8936002)(110136005)(81156014)(316002)(6506007)(53546011)(7416002)(26005)(9686003)(54906003)(2906002)(15650500001)(71200400001)(8676002)(55016002); DIR:OUT; SFP:1102; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: J+q83s4+BFRPSS6CCjnXufZWykt5ybRYBSu3rzagP6kDqcWgBqvdA+k/pHBVH3tF31y74h4oHkh6vEX+2T73mQVAWECbHCZ2BuqDMq7NdiW0jWIqPHxYmVUBjtlJIch0kNaolDzRyq7yUxZYYFW8SVXcxXNht/zWygMd3QSEGh5BToZg6uUy2sPzCziWb2FexaAmrckewoIpLtitL9IWvSEKIqabZUl6auHxhHCh/UrdUhftpSmYyqsVStsBIDv7d28hdzKOhqh/DxLgk+CnTjRor/2V3mnBhbV8hr2hzDlviyKCoXVX8p/NwIEuFW/UpBUEg3OpmGKdF4FU089C5yaoNsXLxzY+aeaezn4qJyZ6h7RudFHdkwcho2S0tk7tprX30V0mwsjXM+G2hvtSiNpe3H0eh8VjU6kJW9wFbzHGG5C5FF9INOclySa9S/3Tvb9tNO60ELCyN6tpmL6g5rLEXvETQr8Rri61MdHCX+E2l1V4xUSA7IfpyKkglczfQ1k2c52WcD/j4omLd61MNA== x-ms-exchange-antispam-messagedata: SwJlw1WnvbSWpgMveXbwieAAMJaTv0MeN5efHAM0DqYad3s+63B8opb81Y5chNqSY0XuRt7YFWKd3UiBXGP1vRGbSPVcN6n+IQ2RBHAxLpszQa1mXmHKWztyqkB29oEUC+FCjGIx/g1AMo+UzyEitQ== Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-CrossTenant-Network-Message-Id: 87bd30bf-62f3-4b72-3fd1-08d7e7ee8a8c X-MS-Exchange-CrossTenant-originalarrivaltime: 24 Apr 2020 01:26:40.6027 (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: MStSXjNJZNf006icurA75a+/4BIhNECtocRRk/qIh11Bz97/nF4PxaPh4qGl8cvurXvbvr7r7ODlWvCE1dmkPl6m/+qglugybC5WnNV6cUI= X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA0PR11MB4637 X-OriginatorOrg: intel.com Subject: Re: [dpdk-dev] [PATCH v2] lib/timer: relax barrier for status update 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 Honnappa, > -----Original Message----- > From: Honnappa Nagarahalli > Sent: Thursday, April 23, 2020 3:06 PM > To: Phil Yang ; Carrillo, Erik G > ; rsanford@akamai.com; dev@dpdk.org > Cc: thomas@monjalon.net; david.marchand@redhat.com; Ananyev, > Konstantin ; jerinj@marvell.com; > hemant.agrawal@nxp.com; Gavin Hu ; nd > ; Honnappa Nagarahalli ; > nd > Subject: RE: [PATCH v2] lib/timer: relax barrier for status update >=20 > Hi Erik, >=20 > > Subject: [PATCH v2] lib/timer: relax barrier for status update > > > > Volatile has no ordering semantics. The rte_timer structure defines > > timer status as a volatile variable and uses the rte_r/wmb barrier to > > guarantee inter-thread visibility. > > > > This patch optimized the volatile operation with c11 atomic operations > > and one-way barrier to save the performance penalty. According to the > > timer_perf_autotest benchmarking results, this patch can uplift > > 10%~16% timer appending performance, 3%~20% timer resetting > > performance and 45% timer callbacks scheduling performance on aarch64 > > and no loss in performance for x86. > > > > Suggested-by: Honnappa Nagarahalli > > Signed-off-by: Phil Yang > > Reviewed-by: Gavin Hu > > > > --- > > This patch depends on patch: > > http://patchwork.dpdk.org/patch/65997/ > > > > v2: > > 1. Changed the memory ordering comment in timer_set_config_state. > > 2. It is still using built-ins as the wrapper functions for C11 > > built-ins are not defined yet. > It is too late to get the wrapper functions done for 20.05. It was decide= d in > yesterday's tech board meeting to go ahead with C11 atomic built-ins (sin= ce > there is lot of code in DPDK that uses C11 built-ins). If there are no fu= rther > comments, can you please provide your ack? >=20 Ok, thanks for letting me know. Based on that decision, I've taken anothe= r look=20 and done some testing and it looks good to me. I've made one comment in-li= ne below and acked it. <... snipped ...> > > @@ -258,9 +257,15 @@ timer_set_config_state(struct rte_timer *tim, > > * mark it atomically as being configured */ > > status.state =3D RTE_TIMER_CONFIG; > > status.owner =3D (int16_t)lcore_id; > > - success =3D rte_atomic32_cmpset(&tim->status.u32, > > - prev_status.u32, > > - status.u32); > > + /* CONFIG states are acting as locked states. If the > > + * timer is in CONFIG state, the state cannot be changed > > + * by other threads. So, we should use ACQUIRE here. > > + */ > > + success =3D __atomic_compare_exchange_n(&tim- > >status.u32, > > + &prev_status.u32, > > + status.u32, 0, > > + __ATOMIC_ACQUIRE, > > + __ATOMIC_RELAXED); > > } > > > > ret_prev_status->u32 =3D prev_status.u32; @@ -279,20 +284,27 @@ > > timer_set_running_state(struct rte_timer *tim) > > > > /* wait that the timer is in correct status before update, > > * and mark it as running */ > > - while (success =3D=3D 0) { > > - prev_status.u32 =3D tim->status.u32; > > + prev_status.u32 =3D __atomic_load_n(&tim->status.u32, > > __ATOMIC_RELAXED); > > > > + while (success =3D=3D 0) { > > /* timer is not pending anymore */ > > if (prev_status.state !=3D RTE_TIMER_PENDING) > > return -1; > > > > /* here, we know that timer is stopped or pending, We know that the timer will be pending at this point... Since we're correct= ing the comment below, we can correct this part too. With that change: Acked-by: Erik Gabriel Carrillo > > - * mark it atomically as being configured */ > > + * mark it atomically as being running > > + */ > > status.state =3D RTE_TIMER_RUNNING; > > status.owner =3D (int16_t)lcore_id; > > - success =3D rte_atomic32_cmpset(&tim->status.u32, > > - prev_status.u32, > > - status.u32); > > + /* RUNNING states are acting as locked states. If the > > + * timer is in RUNNING state, the state cannot be changed > > + * by other threads. So, we should use ACQUIRE here. > > + */ > > + success =3D __atomic_compare_exchange_n(&tim- > >status.u32, > > + &prev_status.u32, > > + status.u32, 0, > > + __ATOMIC_ACQUIRE, > > + __ATOMIC_RELAXED); > > } > > > > return 0; Thanks, Erik