From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
by inbox.dpdk.org (Postfix) with ESMTP id B3B5DA0562;
Fri, 3 Apr 2020 13:58:32 +0200 (CEST)
Received: from [92.243.14.124] (localhost [127.0.0.1])
by dpdk.org (Postfix) with ESMTP id A9EA81C114;
Fri, 3 Apr 2020 13:58:20 +0200 (CEST)
Received: from mga07.intel.com (mga07.intel.com [134.134.136.100])
by dpdk.org (Postfix) with ESMTP id 9D6CB1C0DC;
Fri, 3 Apr 2020 13:58:19 +0200 (CEST)
IronPort-SDR: H9QaZ9GUJ8SLHiuDzrgbVz75J/YOmY/Kq5DEeZlmQFiqdm0mJZ8ZU3UWzX7VndDDeaafndoQcp
mPMREDdU6ROA==
X-Amp-Result: SKIPPED(no attachment in message)
X-Amp-File-Uploaded: False
Received: from orsmga004.jf.intel.com ([10.7.209.38])
by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384;
03 Apr 2020 04:58:18 -0700
IronPort-SDR: 6ExvzDiTBdjJ454WanF6Ap32Zbg6uJpbV7mQjeWSl0zZrMf2IAciV3amE7AeotC2xEFBfzg1az
Jm8835kG3LBQ==
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.72,339,1580803200"; d="scan'208";a="396715198"
Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205])
by orsmga004.jf.intel.com with ESMTP; 03 Apr 2020 04:58:18 -0700
Received: from fmsmsx602.amr.corp.intel.com (10.18.126.82) by
fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS)
id 14.3.439.0; Fri, 3 Apr 2020 04:58:18 -0700
Received: from fmsmsx602.amr.corp.intel.com (10.18.126.82) by
fmsmsx602.amr.corp.intel.com (10.18.126.82) with Microsoft SMTP Server
(version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id
15.1.1713.5; Fri, 3 Apr 2020 04:58:17 -0700
Received: from FMSEDG001.ED.cps.intel.com (10.1.192.133) by
fmsmsx602.amr.corp.intel.com (10.18.126.82) with Microsoft SMTP Server
(version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) id 15.1.1713.5
via Frontend Transport; Fri, 3 Apr 2020 04:58:17 -0700
Received: from NAM11-DM6-obe.outbound.protection.outlook.com (104.47.57.175)
by edgegateway.intel.com (192.55.55.68) with Microsoft SMTP Server (TLS) id
14.3.439.0; Fri, 3 Apr 2020 04:58:17 -0700
ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none;
b=YIMyQbRdndx7QS6n6+BCRUwucarjaXyYJPMXeZF7DlX7z+LNvvjI11CSo1YJVmUFBFqIUG+p2PFX8deyZ7Fc8bsCrMgmh5xBmOTSseQ0+Z95NzdOaI/HmVdlM+hw8uA1eNazdnUELjVo1WCQxFeOzE3jxEOXO41J6B45XUJTj2x2AaBL/OTR5DZiIhzTm0sR3zhH2ULZSBiZjaNibA2QYc7tbOt7dnlq1XVw56/78jGYeXAh5XB5cK3Gi3aCdUhMeIRoUUUsss2T0u6YFgZkIUSV2W6N5FTkoYVsdZ4I/UZqoSGddDphFGw6f2o9clZIFZpTFPzVL6v72mDJyXwBCw==
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=syx0oOPmPVnajRjTBfNZAhHrPdA4LpgjwoU3kC0DTk8=;
b=lu2jJBF1QJfTRN3RW5AQ7/hfvAY08t8gTJ6HB7C8n/RJoyhaBsGhiszuXjEc+E6+3+KsWVrd3jS9FMzyNa5SyKkTCqOmqpQDKbs+U3roVMyIoP3+5+D3mUHVZgO4oxFh98tqVcNpr+KnqxQ9bOtrLsewXvzKbB7HoBxXd+uoRSiKTo3LIbxaf5egJRYA+EWDhK7T1rpWUZpdAdVueaWB121Ff2DpS2mOgpiHbxcTs3AwCPquLNRzTQgRj3WmcgaA+VfX8GkL3R9CbBoanr56csJHA2eB6Av0YIpw9FXMTooUfTf6/y3IC8cUaunjwAlEA08qIy7lUVkGRdxOORbwjw==
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=syx0oOPmPVnajRjTBfNZAhHrPdA4LpgjwoU3kC0DTk8=;
b=KfkdFEwA3zE415PGJkiZd1ljcndeOQZB7s5TaVR59++xZmvYVGTDNzYunrkqqlm8ut1S4XCoBeVHRvf9sLpsdLGW6ncfxMr+SN4DqtElLsket17xndNNJOwR9S8JNe/npLLoJ7jq6Xl6mFbLeJLz7JarLS4JWZoEmR86cRt9c8w=
Received: from BYAPR11MB3143.namprd11.prod.outlook.com (2603:10b6:a03:92::32)
by BYAPR11MB3541.namprd11.prod.outlook.com (2603:10b6:a03:f5::16)
with Microsoft SMTP Server (version=TLS1_2,
cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2878.15; Fri, 3 Apr
2020 11:58:16 +0000
Received: from BYAPR11MB3143.namprd11.prod.outlook.com
([fe80::5917:9757:49b0:3c49]) by BYAPR11MB3143.namprd11.prod.outlook.com
([fe80::5917:9757:49b0:3c49%5]) with mapi id 15.20.2878.014; Fri, 3 Apr 2020
11:58:16 +0000
From: "Van Haaren, Harry" <harry.van.haaren@intel.com>
To: Phil Yang <phil.yang@arm.com>, "thomas@monjalon.net"
<thomas@monjalon.net>, "Ananyev, Konstantin" <konstantin.ananyev@intel.com>,
"stephen@networkplumber.org" <stephen@networkplumber.org>,
"maxime.coquelin@redhat.com" <maxime.coquelin@redhat.com>, "dev@dpdk.org"
<dev@dpdk.org>
CC: "david.marchand@redhat.com" <david.marchand@redhat.com>,
"jerinj@marvell.com" <jerinj@marvell.com>, "hemant.agrawal@nxp.com"
<hemant.agrawal@nxp.com>, "Honnappa.Nagarahalli@arm.com"
<Honnappa.Nagarahalli@arm.com>, "gavin.hu@arm.com" <gavin.hu@arm.com>,
"ruifeng.wang@arm.com" <ruifeng.wang@arm.com>, "joyce.kong@arm.com"
<joyce.kong@arm.com>, "nd@arm.com" <nd@arm.com>, Honnappa Nagarahalli
<honnappa.nagarahalli@arm.com>, "stable@dpdk.org" <stable@dpdk.org>
Thread-Topic: [PATCH v3 10/12] service: identify service running on another
core correctly
Thread-Index: AQHV+/orBSL3IBQhjESAbL8UIwVqvqhnYzZA
Date: Fri, 3 Apr 2020 11:58:16 +0000
Message-ID: <BYAPR11MB314354D146833D997E65E287D7C70@BYAPR11MB3143.namprd11.prod.outlook.com>
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: <1584407863-774-11-git-send-email-phil.yang@arm.com>
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=harry.van.haaren@intel.com;
x-originating-ip: [192.198.151.168]
x-ms-publictraffictype: Email
x-ms-office365-filtering-correlation-id: 7cc48c5b-0cf0-4320-6857-08d7d7c64b57
x-ms-traffictypediagnostic: BYAPR11MB3541:
x-ms-exchange-transport-forked: True
x-microsoft-antispam-prvs: <BYAPR11MB35410E9A5DCC9AE418AB9706D7C70@BYAPR11MB3541.namprd11.prod.outlook.com>
x-ms-oob-tlc-oobclassifiers: OLM:1051;
x-forefront-prvs: 0362BF9FDB
x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:;
IPV:NLI; SFV:NSPM; H:BYAPR11MB3143.namprd11.prod.outlook.com; PTR:; CAT:NONE;
SFTY:;
SFS:(10019020)(396003)(136003)(376002)(346002)(366004)(39860400002)(76116006)(53546011)(8676002)(478600001)(9686003)(7696005)(66476007)(81156014)(81166006)(55016002)(5660300002)(64756008)(4326008)(66556008)(186003)(52536014)(66446008)(2906002)(66946007)(26005)(8936002)(33656002)(86362001)(110136005)(54906003)(6506007)(71200400001)(7416002)(316002);
DIR:OUT; SFP:1102;
x-ms-exchange-senderadcheck: 1
x-microsoft-antispam: BCL:0;
x-microsoft-antispam-message-info: ndyO4flpD59U45VthRxUC1CwGMtbUvRNH+fdmWmwMQbHsgY4t8sFq02EYbvKDqa4bOJaV7CnffHszl86/sLddMMde0bboNUnTkzn1KKomF7JKpBteuPnBHbj+EUjGXx7QtdSSwbaGMrKOTiAGaA4mnkAfnyzSgQf5ozvq7vkuJix/cGVANmrzDRuJTBpkQ7xfiASmv2rAdBihItgmrOv2JYTnKsFLir5vYUEPouZA/+2qTc9MO/KbIA3M8OH93L6aRO3Eo1ZOH6neqVeqwWM7azSJ/9SJ7FMpRERgFKcmcd+XgptPyDNy+OiRX6wf5wGCiZrsdEeRj2qpzRtRLswpU8ZJYdsSO+js5VRTb/qUsgKC8ZqEJkSKXFupM16FD2z7YbdXRYNbQrZEzchissM0udPf1H5cMEYyYr78o6QyCGieGA5emRgJ8+XRaXT/cCk
x-ms-exchange-antispam-messagedata: NO4o9cfUH1B6Y9NEI0fJ6guZkTH5WRVxj7YOjwiixw67LiXXG2x2hK4bJwY1vKWmL0/hMRScTQHOocQGBT3JNNhQdWn5DgkLU9Cndf8JIElu5p/v4EL1n3W8QEems7Y3AfZVFUNeeubJ0+qmVkkW9Q==
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 7cc48c5b-0cf0-4320-6857-08d7d7c64b57
X-MS-Exchange-CrossTenant-originalarrivaltime: 03 Apr 2020 11:58:16.2331 (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: zQhSBTvBwQbLbtgWj7qrJc0kvvu9+u7DhV6KCecx4tdEvp9DMNLuOoRVOPyjJLX0Q5RG17Wbe4b9AkLXPj8k+nI2AWWKZE1FoAUmDOdq5Z8=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR11MB3541
X-OriginatorOrg: intel.com
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 <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>
> From: Phil Yang <phil.yang@arm.com>
> Sent: Tuesday, March 17, 2020 1:18 AM
> To: thomas@monjalon.net; Van Haaren, Harry <harry.van.haaren@intel.com>;
> Ananyev, Konstantin <konstantin.ananyev@intel.com>;
> stephen@networkplumber.org; maxime.coquelin@redhat.com; dev@dpdk.org
> Cc: david.marchand@redhat.com; jerinj@marvell.com; hemant.agrawal@nxp.com=
;
> Honnappa.Nagarahalli@arm.com; gavin.hu@arm.com; ruifeng.wang@arm.com;
> joyce.kong@arm.com; nd@arm.com; Honnappa Nagarahalli
> <honnappa.nagarahalli@arm.com>; stable@dpdk.org
> Subject: [PATCH v3 10/12] service: identify service running on another co=
re
> correctly
>=20
> From: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
>=20
> 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.
>=20
> Cc: stable@dpdk.org
>=20
> Signed-off-by: Honnappa Nagarahalli <honnappa.nagarahalli@arm.com>
> Reviewed-by: Phil Yang <phil.yang@arm.com>
> Reviewed-by: Gavin Hu <gavin.hu@arm.com>
Is this fixing broken functionality, or does it aim to only "optimize"?
Lack of "fixes" tag suggests optimization.
I'm cautious about the commit phrase "Assume that the application ...",
if the code was previously checking things, we must not stop checking
them now, this may introduce race-conditions in existing applications?
It seems like the "serialize_mt_unsafe" branch is being pushed
further down the callgraph, and instead of branching over atomics
this patch forces always executing 2 atomics?
This feels like too specific an optimization/tradeoff, without data to
backup that there are no regressions on any DPDK supported platforms.
DPDK today doesn't have a micro-benchmark to gather such perf data,=20
but I would welcome one and we can have a data-driven decision.
Hope this point-of-view makes sense, -Harry
> ---
> lib/librte_eal/common/rte_service.c | 26 ++++++++------------------
> 1 file changed, 8 insertions(+), 18 deletions(-)
>=20
> 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_im=
pl
> *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,
>=20
> cs->service_active_on_lcore[i] =3D 1;
>=20
> - 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;
>=20
> @@ -412,24 +412,14 @@ rte_service_run_iter_on_app_lcore(uint32_t id, uint=
32_t
> serialize_mt_unsafe)
>=20
> SERVICE_VALID_GET_OR_ERR_RET(id, s, -EINVAL);
>=20
> - /* Atomically add this core to the mapped cores first, then examine if
> - * 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);
>=20
> - 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);
>=20
> - if (serialize_mt_unsafe)
> - rte_atomic32_dec(&s->num_mapped_cores);
> + rte_atomic32_dec(&s->num_mapped_cores);
>=20
> 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);
> }
>=20
> cs->loops++;
> --
> 2.7.4