From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id C9881A09E4; Thu, 28 Jan 2021 15:44:00 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 6670C4067A; Thu, 28 Jan 2021 15:44:00 +0100 (CET) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by mails.dpdk.org (Postfix) with ESMTP id F391740395 for ; Thu, 28 Jan 2021 15:43:58 +0100 (CET) IronPort-SDR: 8KdbCMhh/mNj1pRcPqxaeXmTmBYhm5uKeiV4s9Y6iK54jeJepTBhDhWp9z83O2Pdc9CAdiSYB6 AeHeNkf+BtQQ== X-IronPort-AV: E=McAfee;i="6000,8403,9877"; a="241768251" X-IronPort-AV: E=Sophos;i="5.79,382,1602572400"; d="scan'208";a="241768251" Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 28 Jan 2021 06:43:57 -0800 IronPort-SDR: uyuohWAzyuElKQ2Md/BTlj2sDPqr+j9dh8LQlXN+Kwu5kZ6GasXK7A+vMvrEZmiIWnG/9RZifA qxFM5araudyg== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.79,382,1602572400"; d="scan'208";a="369877169" Received: from orsmsx606.amr.corp.intel.com ([10.22.229.19]) by orsmga002.jf.intel.com with ESMTP; 28 Jan 2021 06:43:57 -0800 Received: from orsmsx607.amr.corp.intel.com (10.22.229.20) by ORSMSX606.amr.corp.intel.com (10.22.229.19) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2106.2; Thu, 28 Jan 2021 06:43:56 -0800 Received: from orsmsx603.amr.corp.intel.com (10.22.229.16) by ORSMSX607.amr.corp.intel.com (10.22.229.20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2106.2; Thu, 28 Jan 2021 06:43:56 -0800 Received: from ORSEDG602.ED.cps.intel.com (10.7.248.7) by orsmsx603.amr.corp.intel.com (10.22.229.16) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2106.2 via Frontend Transport; Thu, 28 Jan 2021 06:43:56 -0800 Received: from NAM10-BN7-obe.outbound.protection.outlook.com (104.47.70.109) by edgegateway.intel.com (134.134.137.103) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.1713.5; Thu, 28 Jan 2021 06:43:56 -0800 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=mZlaAhbgGOSragOh3IdWDvVzbsGhhEJ2MYn5Aa5ngDK6/PC8sYN3A1jJHX0QfCLyO5JJeYwL/6JzrFJqkKgfihISI3Hlcv6j2HQCIHbcONNy9xe3KPAZt3TZSfhROe+7j2CMLkE0dovHE8/tltdn0LWHkfwO5PJHY6iOvqyU3cDtJUEYdQ7901OA3+HQfEQDGZ3W+lNhAE+oOzYqf7iG8Wbbhgt1a1RlhzVVUYhxFhUhITwe0E/AWsrwChqyiBOw38pe0wWrvCzvYE3krOi/DPMZ0wakd2sfR9FUY1ZdhQHeSLCgBTmwC3SRrlhiVpZPzG0Ef6vJsXO3NVkmpibT5Q== 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=QiejF9wCFXIPp/QzqROxtLGY/6MrEOyGrqeQiYs0JpI=; b=lz40nsTl4NlEkCs01SSlPh5zRQdEAZGiih4s4Yu5X88RnSn0+eUhukV2MTYZK0q/Y4/8ot7ShSM8HEFR2K88JnwyFmvTR3rZgBtUkO7rppKe//PuTx07OueLLTMmrrLM9qsiOfTQHPFz1gavqaDU13kdYx7ohBFRyHMJ0wcqsAVrVj37J0bkLm0oVhupRz6QlaLHDr362eSxms2CM6CbaIFh5rJK54imX6FVD9dQzc0LTbRBAWQ92V/UiDy1+ZUjqn6kWyxqpHAA06tH/89u89CfNvQ0ThQam9sGp4k6CRdVqDhqAWQ7V1HKCvadNgOvkheTNJVlWGXFTPqc2MZbow== 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=QiejF9wCFXIPp/QzqROxtLGY/6MrEOyGrqeQiYs0JpI=; b=ynzEwz7L3+L+ydmSgRjn8unQA8PjPWYsQBnK6TumDP151H8vJLGHIQKQX+aWF0nRNtNY2BKhQ1bcPkxs04hn/KDMLEWQ3oHAVqPHNvKD2k9VsjTUBnebUK/qaN8M78Vxh+fsE/54GKRvNUicCuZAyl6xKQO0citYkt9Yv0F8+eQ= Received: from BYAPR11MB3301.namprd11.prod.outlook.com (2603:10b6:a03:7f::26) by SJ0PR11MB4800.namprd11.prod.outlook.com (2603:10b6:a03:2af::14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3805.17; Thu, 28 Jan 2021 14:43:55 +0000 Received: from BYAPR11MB3301.namprd11.prod.outlook.com ([fe80::1152:1426:8a4f:c755]) by BYAPR11MB3301.namprd11.prod.outlook.com ([fe80::1152:1426:8a4f:c755%4]) with mapi id 15.20.3784.019; Thu, 28 Jan 2021 14:43:54 +0000 From: "Ananyev, Konstantin" To: Honnappa Nagarahalli , Feifei Wang CC: "dev@dpdk.org" , nd , Ruifeng Wang , nd Thread-Topic: [PATCH v1 1/2] app/test: remove unnecessary barriers for ring stress test Thread-Index: AQHW2CwQ96/sxptInkmsLdK5Uf3RAKoDBLrQgDlLZACAAQV0AA== Date: Thu, 28 Jan 2021 14:43:54 +0000 Message-ID: References: <20201222063054.44429-1-feifei.wang2@arm.com> <20201222063054.44429-2-feifei.wang2@arm.com> In-Reply-To: Accept-Language: en-GB, 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.5.1.3 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: [46.7.39.127] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 44e6d073-9e08-4445-9875-08d8c39b2324 x-ms-traffictypediagnostic: SJ0PR11MB4800: x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:8882; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: iMGXQ85Ed52pbhfm4O7j09XzUh7bshe26rl9UHFmz2W5uXwNqB/siAk+jgUzPkk9Ky4CnMwSbmoFn785/+M9eLOPK9W2j3MoAXK3LW0WzrC41V1TOxpm9Y7jro0UAGteiGcJki27w4wuN3Q9uqvMaL8qzjV62hujW2/ldFRmx6ZWiD+V8USY4XLzy5/+gJQnykS0JO7KYhpSUpx2R/uAqijKWeDgE8C525phFgBZsCMB/dfrw/0jXa2/aVYYKzdr/sPCIroNLLLso7WouG39VbmoRMj77y0/SQ5UePZU4HvftlK2qnGNQ9zE7LF5NX9Db37cmWNDjGjBfFkkkD2k5Q1ugw3rLoxKd0FfSITTq2ZeZBvxHoCY+1qt14yci8kFwYbDufb3j8wwuo9haI2F7M1uZKvv9OpnrvUjfaXCXG8rjbZ7nRVPUUI8RP0ws7wgeVlAznHRtRGL2/mGA2BNB1HFxzBWu4xA4FbFAA+LhxXebhXSjDtXI/8D9At9zlBY8xbrbHTeRCvQGwCdLeQUdWRIU8oPyl8jgMvOYVOpvqzEMDyx8QIsfOMBAMpd94DuM9zXripPni+v3+wDzaKnVZkb54oQlXh6B2ivLzxLhKo= x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:BYAPR11MB3301.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFS:(4636009)(136003)(346002)(366004)(396003)(376002)(39860400002)(71200400001)(52536014)(6506007)(5660300002)(7696005)(86362001)(9686003)(8936002)(66946007)(2906002)(33656002)(66446008)(66476007)(66556008)(4326008)(966005)(478600001)(110136005)(316002)(55016002)(76116006)(54906003)(83380400001)(26005)(8676002)(186003)(64756008); DIR:OUT; SFP:1102; x-ms-exchange-antispam-messagedata: =?us-ascii?Q?+8YpSoUqfpZqGTQOkeymbtediqDuvXUew/KNDnZjm1epDnbpLGUtxy8YRliO?= =?us-ascii?Q?df+1ZtCv7MtvD3J1dnwp89zTf1NPVshKntJdzFAcKctw9thau2SwjaRNzPxU?= =?us-ascii?Q?enBjyTmbrqMDwMoqMD31bbs/nR28RZiqY1JMuk364uAMBbE+yM+m3ITdMqQG?= =?us-ascii?Q?xNjus6VgD3Gk9j7BoBZzC9EY6dtCy45jtyQMiF9vrP2ahGyyblul85k8SCew?= =?us-ascii?Q?I3GSREZB55vgn12xRhAXgi+1Xcn4I1yshieq8GPNOHHXh3j5aXyGkJ34ubpc?= =?us-ascii?Q?C0JCDb7nd8yhC7ha8SL2ITgi+w3VVLYK+DxMyyHLCuYRh489mk/2V7cneWbm?= =?us-ascii?Q?k3Ec5o0nJp7tsiYEQ8mmDVhkrOBKJUBkKJbvW+x/B8Yxa1hgyZMwttPntYb/?= =?us-ascii?Q?5MPwUVd6bZedkvjcrVhSXZdl1/jueGCpKkWo50ulQRDg1/ZwVgqtW2XDW9pT?= =?us-ascii?Q?4j3wzQC7ioe/h0s9gGwlb+Dbf1mhrFTPXDsUWa34HRZBmKnw21i3ZO+uVP4T?= =?us-ascii?Q?gFYJznEfpbN0KgLwuU3ys0iuC4AK/hClMdV/NDaNnI9ctJBPyeJUFVEDbGdS?= =?us-ascii?Q?LvON+ok8fXMy3n8TZPfYqiZrn4hD9E0w4W/zE+3ax0MUobrNDm/h+WmujhrC?= =?us-ascii?Q?1g3ITrEW8TECq3exB948GeUn3da/59xOsvZV5wrnrz7GUU79unFdrqva/jaJ?= =?us-ascii?Q?nnmI7e6dq5QQ8Lh0Yu3QCJp3E8Z1kw5t3FyI8Tw1Sw/U9vQcUCC2wF30WCNd?= =?us-ascii?Q?H9kyz0Y65eFfEZFYUhnqYrsWBFStAPXm08nSrGAgC6HT5+o3NpNkDkicWZm/?= =?us-ascii?Q?9sQe9WTrqYLIws1JdHmPgTdkdlFzGog1MzHOxxZ42oYlyXmo76ghemqMsHOk?= =?us-ascii?Q?RnKr9Y7oHe8tjMNv+fMBzUeQwuKA4BU/ea304sX5hFJ8df/hLfvOqw8emAsY?= =?us-ascii?Q?/gB8JNUs71kC5eOdtwA8ZcrL+2uS+tReyw8UpFDnDzIQpOD7fyJJtJQGbSFo?= =?us-ascii?Q?l/qiozqSftgtd1AZEl3015NJUZdwXgyafmxfPJElUuPnl1o=3D?= 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: BYAPR11MB3301.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 44e6d073-9e08-4445-9875-08d8c39b2324 X-MS-Exchange-CrossTenant-originalarrivaltime: 28 Jan 2021 14:43:54.8853 (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: 46Npq5yckyeSFMjCi2D7EkN8QQlkx2L44j8FpJQkLp7yJ0Zxt9K+MAxngrbkI5NPmczCVt86Gg7VQTpGv+7wYoJN38OYRkZzBl0NlvTEYJs= X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR11MB4800 X-OriginatorOrg: intel.com Subject: Re: [dpdk-dev] [PATCH v1 1/2] app/test: remove unnecessary barriers for ring stress test X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 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 Feifei, > > > > > > > > The variable "wrk_cmd" is a signal to control threads from running an= d > > > stopping. When worker lcores load "wrk_cmd =3D=3D WRK_CMD_RUN", they > > start > > > running and when worker lcores load "wrk_cmd =3D=3D WRK_CMD_STOP", > > they > > > stop. > > > > > > For the wmb in test_mt1, no storing operations must keep the order > > > after storing "wrk_cmd". Thus the wmb is unnecessary. > > > > I think there is a bug in my original code, we should do smp_wmb() *bef= ore* > > setting wrk_cmd, not after: > > > > /* launch on all workers */ > > RTE_LCORE_FOREACH_WORKER(lc) { > > arg[lc].rng =3D r; > > arg[lc].stats =3D init_stat; > > rte_eal_remote_launch(test, &arg[lc], lc); > > } > > > > /* signal worker to start test */ > > + rte_smp_wmb(); > > wrk_cmd =3D WRK_CMD_RUN; > > - rte_smp_wmb(); > > > > usleep(run_time * US_PER_S); > > > > > > I still think we'd better have some synchronisation here. > > Otherwise what would prevent compiler and/or cpu to update wrk_cmd out > > of order (before _init_ phase is completed)? > > We probably can safely assume no reordering from the compiler here, as = we > > have function calls straight before and after 'wrk_cmd =3D WRK_CMD_RUN;= ' > > But for consistency and easier maintenance, I still think it is better = to have > > something here, after all it is not performance critical pass. > Agree that this is not performance critical. >=20 > This is more about correctness (as usually people refer to code to unders= tand the concepts). You can refer to video [1]. Essentially, the > pthread_create has 'happens-before' behavior. i.e. all the memory operati= ons before the pthread_create are visible to the new thread. The > rte_smp_rmb() barrier in the thread function is not required as it reads = the data that was set before the thread was launched. rte_eal_remote_launch() doesn't call pthread_create(). All it does - updates global variable (lcore_config) and writes/reads to/f= rom the pipe. >=20 > I do not know why rte_smp_wmb is required. The update to 'wrk_cmd' is see= n by the thread eventually. rte_smp_wmb does not result in > update being seen sooner/immediately. We don't need it sooner. We need to make sure it wouldn't be seen by any worker thread before all wo= rkers are launched. To make sure all workers start the test at approximately same moment. That's why I think wmb() should be before 'wrk_cmd =3D WRK_CMD_RUN;' in my = original code. >=20 > [1] https://www.youtube.com/watch?t=3D4170&v=3DKeLBd2EJLOU&feature=3Dyout= u.be > > > > > For the rmb in test_worker, the parameters have been prepared when > > > worker lcores call "test_worker". It is unnessary to wait wrk_cmd to > > > be loaded, then the parameters can be loaded, So the rmb can be > > removed. > > > > It is not only about parameters loading, it is to prevent worker core = to start > > too early. > Because 'pthread_launch' provides the 'happens-before' behavior, the work= er core will see the updates that happened before the worker > was launched. >=20 > I suggest changing the commit log to provide the reasoning around pthread= _create. >=20 > > > > As I understand, your goal is to get rid of rte_smp_*() calls. > > Might be better to replace such places here with _atomic_ semantics. > > Then, as I can see, we also can get rid of 'volatile' fo wrk_cmd. > > > > > In the meanwhile, fix a typo. The note above storing "stop" into > > > "wrk_cmd" should be "stop test" rather than "start test". > > > > > > Signed-off-by: Feifei Wang > > > Reviewed-by: Honnappa Nagarahalli > > > Reviewed-by: Ruifeng Wang > > > --- > > > app/test/test_ring_stress_impl.h | 5 +---- > > > 1 file changed, 1 insertion(+), 4 deletions(-) > > > > > > diff --git a/app/test/test_ring_stress_impl.h > > > b/app/test/test_ring_stress_impl.h > > > index f9ca63b90..384555ef9 100644 > > > --- a/app/test/test_ring_stress_impl.h > > > +++ b/app/test/test_ring_stress_impl.h > > > @@ -198,7 +198,6 @@ test_worker(void *arg, const char *fname, int32_t > > prcs) > > > fill_ring_elm(&loc_elm, lc); > > > > > > while (wrk_cmd !=3D WRK_CMD_RUN) { > > > - rte_smp_rmb(); > > > rte_pause(); > > > } > > > > > > @@ -357,13 +356,11 @@ test_mt1(int (*test)(void *)) > > > > > > /* signal worker to start test */ > > > wrk_cmd =3D WRK_CMD_RUN; > > > - rte_smp_wmb(); > > > > > > usleep(run_time * US_PER_S); > > > > > > - /* signal worker to start test */ > > > + /* signal worker to stop test */ > > > wrk_cmd =3D WRK_CMD_STOP; > > > - rte_smp_wmb(); > > > > > > /* wait for workers and collect stats. */ > > > mc =3D rte_lcore_id(); > > > -- > > > 2.17.1