From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
	by inbox.dpdk.org (Postfix) with ESMTP id EDFEAA0543;
	Wed, 14 Dec 2022 10:39:01 +0100 (CET)
Received: from mails.dpdk.org (localhost [127.0.0.1])
	by mails.dpdk.org (Postfix) with ESMTP id DBC04400D6;
	Wed, 14 Dec 2022 10:39:01 +0100 (CET)
Received: from mx0b-0016f401.pphosted.com (mx0a-0016f401.pphosted.com
 [67.231.148.174])
 by mails.dpdk.org (Postfix) with ESMTP id 0AB394003F
 for <dev@dpdk.org>; Wed, 14 Dec 2022 10:38:59 +0100 (CET)
Received: from pps.filterd (m0045849.ppops.net [127.0.0.1])
 by mx0a-0016f401.pphosted.com (8.17.1.19/8.17.1.19) with ESMTP id
 2BE6Uboh013706; Wed, 14 Dec 2022 01:38:59 -0800
Received: from nam10-mw2-obe.outbound.protection.outlook.com
 (mail-mw2nam10lp2106.outbound.protection.outlook.com [104.47.55.106])
 by mx0a-0016f401.pphosted.com (PPS) with ESMTPS id 3mf6tj18yh-1
 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT);
 Wed, 14 Dec 2022 01:38:54 -0800
ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none;
 b=bvfO23V28eAFu5NnYIPWAMPc17QBMTkQelbJX3FrPdCCJS38SL9W/Bhc6mutWyNTU6CixmnTtUQiDMS3xmn+60r6gReiMCtJM4cNoxxJnSESrjwe8VNd++TyO4f+9uZ2e7MRTUOFsn1Azj7F+OdTY49qzal8ZJDwfGcLyd2YFNFtdu2w6a32lVnFct7g+VqTsl+MUiozj0YFN5aSlshxnhccaJ3+4wnuYzr7ZniXkTFdZ5Y8uV0OLZCPWCXtoxZJWkKkANzlyeuR+pG/0KVohM7FI6g9WPHthcjguoESjESARFfvBkEmfrXGm1ddvj1chLafwCVHmqy5OyEvMVj1eg==
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-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1;
 bh=NJPkPEOARO2/oEds7y2CMLrMl8OSLJrGjPTQQVHm2FA=;
 b=mAs/YK93HN4G2bWeH9ZyW3Uka0RPlDEwoayXn0NmryU2APIRfXb4naEPi+82MBLOFWDXaeTn1+ADalx9Wd4GeSDjd6ndL6XPW2Kr3L+bB6eI/36ap6ZehvWYkARENmxffZnJqIx2KbbvsuhtQXswbSh1LN23x9Ga9w2FM1hRUPCbPs43qiOKMdpiCO7aAurssnBfP+METKDLtTAS5Gpp+jLeCfTwY6mfYWB7p4jwypDv6GLsObno0nYrbStxjFMc4DtPY9P/siADIVeOW5LbyOnAvUkqslNDg9q53Gk0HnTKRZ+ESl5XNJOxK010CHZBfNYdEcyoj5I8dQ6gqFrMzQ==
ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass
 smtp.mailfrom=marvell.com; dmarc=pass action=none header.from=marvell.com;
 dkim=pass header.d=marvell.com; arc=none
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;
 d=marvell.onmicrosoft.com; s=selector1-marvell-onmicrosoft-com;
 h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck;
 bh=NJPkPEOARO2/oEds7y2CMLrMl8OSLJrGjPTQQVHm2FA=;
 b=ISE9eED/F9Syo7B8Wx34ksBBR7o3OG4gthRE92fFaIou7wezGNVlmTP5WKi5HD0ooqwWoi34DSJkrxq9voXUj0uraGBEVYxLIFEwSS8GbUcBS2BmgfYCpbuIj6xbXwGKQnSi74ctPt8PWcB7kOxKUHrKHC0bbXWMrT5VDZpP1vc=
Received: from DM4PR18MB4368.namprd18.prod.outlook.com (2603:10b6:5:39d::6) by
 SN7PR18MB4062.namprd18.prod.outlook.com (2603:10b6:806:10f::11) with
 Microsoft SMTP Server (version=TLS1_2,
 cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.5880.10; Wed, 14 Dec
 2022 09:38:51 +0000
Received: from DM4PR18MB4368.namprd18.prod.outlook.com
 ([fe80::3117:f51c:37c2:fa05]) by DM4PR18MB4368.namprd18.prod.outlook.com
 ([fe80::3117:f51c:37c2:fa05%5]) with mapi id 15.20.5924.011; Wed, 14 Dec 2022
 09:38:51 +0000
From: Tomasz Duszynski <tduszynski@marvell.com>
To: =?iso-8859-1?Q?Morten_Br=F8rup?= <mb@smartsharesystems.com>,
 "dev@dpdk.org" <dev@dpdk.org>
CC: "thomas@monjalon.net" <thomas@monjalon.net>, Jerin Jacob Kollanukkaran
 <jerinj@marvell.com>, "zhoumin@loongson.cn" <zhoumin@loongson.cn>
Subject: RE: [PATCH v4 1/4] eal: add generic support for reading PMU events
Thread-Topic: [PATCH v4 1/4] eal: add generic support for reading PMU events
Thread-Index: AQHZDt/QEhRtFHSga0yHSOMIy5Zzs65rtOuAgAFVOCA=
Date: Wed, 14 Dec 2022 09:38:51 +0000
Message-ID: <DM4PR18MB4368461EC42603F77A7DC1BCD2E09@DM4PR18MB4368.namprd18.prod.outlook.com>
References: <20221129092821.1304853-1-tduszynski@marvell.com>
 <20221213104350.3218167-1-tduszynski@marvell.com>
 <20221213104350.3218167-2-tduszynski@marvell.com>
 <98CBD80474FA8B44BF855DF32C47DC35D8758C@smartserver.smartshare.dk>
In-Reply-To: <98CBD80474FA8B44BF855DF32C47DC35D8758C@smartserver.smartshare.dk>
Accept-Language: en-US
Content-Language: en-US
X-MS-Has-Attach: 
X-MS-TNEF-Correlator: 
x-ms-publictraffictype: Email
x-ms-traffictypediagnostic: DM4PR18MB4368:EE_|SN7PR18MB4062:EE_
x-ms-office365-filtering-correlation-id: f0900d41-3d0f-427e-57fc-08daddb7025b
x-ms-exchange-senderadcheck: 1
x-ms-exchange-antispam-relay: 0
x-microsoft-antispam: BCL:0;
x-microsoft-antispam-message-info: 6dhtuJqkWP/Kyt9RU4PCy2fnfKgno5nAq89L9qoYmwltD6b9J6Wuu9S9CkHVKVpmI7AkshsAGr1h+JVcLobSTJJlvy503aG3EBI52MIJqhHQ92RHk6SesFS4bW/xD6T9XIuFu5S3T9cTK3bH/aBN5wI03RQv6ovqV3CH1wgFwta97+ph4y9GJcm5OUamSK3ldGACutLVQEEo/l1RItdpIC8302Mgu7lFbhB4aDELV6umXCSL66rzB/T5IQ8AorjDWEdnFuS5sLE7w2tRvy4GiynR0THct57wHAS6uQfF+/Hob011NzSDNI0BDH7gbzqeih0ax25kGnNMn3kLvhMmyHtwib4SvuDYPQjT3CAdGzc/d00WmiIzi8Yzdnrdyxb4ph7UhPzZGx8p24uPwysNuDt9wu9a7f5DEdcl45chwWL8iPOAi8s3W4FAYDaTTmBZdiJrHiaMWo/uIgF165W7ykdb1KyTC5B6lyYG2XylZZuiHsFcrpvPzoYkG0PiO7VGyf/i6pqfL/K2P2jjfNdWP71kttXX0ogBInbM4tbvnuV5M1HR+RnX9fyVFiH2QqUi4aPv3pt8mTSCXMQNdpQ2c3ifhgCcj3/ESYShPUaH8/B/PuqyipmmEwZ3biVsiS5b5ImbaFGAG3V1jHtH/L5D6RaIR7onZm6f7n4U76X5NXUgqinKz4F+3XF4AtZqmpf1bZppuXw+uXzSc0G/VLyzW3SWWCRW3GRB0KLBFpBRAd8=
x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:;
 IPV:NLI; SFV:NSPM; H:DM4PR18MB4368.namprd18.prod.outlook.com; PTR:; CAT:NONE;
 SFS:(13230022)(4636009)(39850400004)(346002)(136003)(396003)(376002)(366004)(451199015)(122000001)(41300700001)(38100700002)(52536014)(8936002)(66556008)(66946007)(5660300002)(66446008)(64756008)(66476007)(4326008)(76116006)(8676002)(83380400001)(6506007)(7696005)(478600001)(2906002)(110136005)(54906003)(38070700005)(316002)(66574015)(26005)(9686003)(71200400001)(33656002)(55016003)(186003)(86362001)(41533002);
 DIR:OUT; SFP:1101; 
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?iso-8859-1?Q?sS2WH1g01zAU0bC4cR5xZIn8Eh8oUGgr0J/Ccq5AnkS37tHLbqECp4Bo4E?=
 =?iso-8859-1?Q?Nj7bDBzz/f082k3kGd6Zxa8DRcyhuxelvOzFyKGIwdNDKHkGAoW1knv5aD?=
 =?iso-8859-1?Q?mhHAwbWFhp/e6CB2P03735mUk0LZfZZlUmymfHNDLE7dN/hxO6fsUcXeMR?=
 =?iso-8859-1?Q?S5PmGuWGwVyuFN+SyOjg7U0YuFsXAb4te/2qlyLlonBZUyZcy/5vx6/spb?=
 =?iso-8859-1?Q?gnpw/omD2UBqiAzHsAzgRwcPTZ/dpYXBxtyPrO1rGt300p69UhT5txcxOH?=
 =?iso-8859-1?Q?38z0fvz+PLUjML29YxfWSKIy+IjUQ/lXeXZ6o+nLrApM7XaVOMdsShIIMB?=
 =?iso-8859-1?Q?d2NF/gsoTlPBhPne4sBrL7zz1RoiMlLR14lP3pRIRPNx9T4I0FVNrd+AUH?=
 =?iso-8859-1?Q?XOKAyyjm5E5VN5GCzbP9WYva/Kbb9gxD+uU4bZoSQgiAcT/cNQIKQRJVuD?=
 =?iso-8859-1?Q?g/TTwkboj8nDA6uojhli99UzPz69L16Xp+uJNcTiHSacgfQwANzgrqh9gb?=
 =?iso-8859-1?Q?xKS8XlqDIzlzeFUleYiWbAUva7gOHznOGcnfZ47AcbMfITBC7sniAXsNbl?=
 =?iso-8859-1?Q?rHTtSMGSfqT6QFJPgxewqZfx1xD5CkM175YjpeM+93MPDYeI/F3PjfsM4E?=
 =?iso-8859-1?Q?yff5ZvgEbrTrWNLTtHyLE+bcCqmKYYTMV5uveacIXqtzhvBaM+CYcL93EC?=
 =?iso-8859-1?Q?dZItNCDTPrIHaDA8WVJ1LK0R6U745t1eFnMxjSrr1KwKB8XZcizav7GGWC?=
 =?iso-8859-1?Q?CMT1wnTImhd6434Lju7KG8nU2W/CGgRj+mLY99N0NQj1Zi/NCFAvwf7bnO?=
 =?iso-8859-1?Q?c3HnXo/iLjL/zELrUFcUzPlvjB01Vy/36aqHYTcN5mGR1xDfgoCu7h+G5P?=
 =?iso-8859-1?Q?prym/NSARP26SdHv8vmsyXTUKdOnLKoPV7GXhuOHlDSA4nkyIyXp24eSlu?=
 =?iso-8859-1?Q?Xod+WhGmurSrEKx8elmto5oYxy803KMccr8F3za5pO4zE/MfcGBhYp60Yt?=
 =?iso-8859-1?Q?hSN/zTe2bw4eY+LGINeCT+09ag5wdnxLNeOzQABXU33RePip2c6B72lE1o?=
 =?iso-8859-1?Q?Kzp9qLNz7JWl14gSnGAk/boEcnUnHEtqcTS7qwKEtscoWnhGQ1brVewUFa?=
 =?iso-8859-1?Q?3wF09vUrQNnVtWjM4f+6Eg0GMzi/kS0XAoryPMpypYZD10QHphuLY20F1s?=
 =?iso-8859-1?Q?MXcY584RWzZAPoYer8lGQrIdtGWd/o/9GzgH4iMdaeQyrB9l+68s1+qmGV?=
 =?iso-8859-1?Q?TVLmwkW/5TL/j7wx1QXB3slEBfrc67Rb07Ss23IW/oo04SHOeeIQwJclAf?=
 =?iso-8859-1?Q?p+oJwZbGXT08lNL1o37ChKS2vMIYI22fUjl+pJqQ7aUJULcXXeIgRprC5Q?=
 =?iso-8859-1?Q?vncxiLgkyGqhrCkB5v4FGLBT65E8zjmKofeWcwxCDNdHs8i9zBEaGnT53J?=
 =?iso-8859-1?Q?Qp5wIfx8sqvCANh1mOsfOG/oLNb4Vx6gOJw8v7gW8/w9Q6v2bkOqicYdZ1?=
 =?iso-8859-1?Q?yuqJO+5Ao5WjoJRnubKCoiiODAUjK3GYonQSlGaHq67TCAl8BKjMSR2Hkn?=
 =?iso-8859-1?Q?QbRujAtZCXP07z4v8wZCi4SUd1maUdz1XwrxQkknL5mPg7CuGQz2/PYGed?=
 =?iso-8859-1?Q?Cgv0usuWfsg5kLugUigy+s3iKP8vwVscIe?=
Content-Type: text/plain; charset="iso-8859-1"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: marvell.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: DM4PR18MB4368.namprd18.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: f0900d41-3d0f-427e-57fc-08daddb7025b
X-MS-Exchange-CrossTenant-originalarrivaltime: 14 Dec 2022 09:38:51.4484 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 70e1fb47-1155-421d-87fc-2e58f638b6e0
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: ccTgfBcneqXktd+JWro0BtF6ziPjUJeNvsxPTydMnuAd3maU7aUJCs3udM+J5K1eH63Y3fm4A9xfcrKBlFVBFg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN7PR18MB4062
X-Proofpoint-ORIG-GUID: bvbzWv8Qr6enZWEHtYa8iIFjuruKHgBz
X-Proofpoint-GUID: bvbzWv8Qr6enZWEHtYa8iIFjuruKHgBz
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.205,Aquarius:18.0.923,Hydra:6.0.545,FMLib:17.11.122.1
 definitions=2022-12-14_04,2022-12-13_01,2022-06-22_01
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
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

Hello Morten,=20

Thanks for review. Answers inline.=20

[...]

> > +/**
> > + * @file
> > + *
> > + * PMU event tracing operations
> > + *
> > + * This file defines generic API and types necessary to setup PMU and
> > + * read selected counters in runtime.
> > + */
> > +
> > +/**
> > + * A structure describing a group of events.
> > + */
> > +struct rte_pmu_event_group {
> > +	int *fds; /**< array of event descriptors */
> > +	void **mmap_pages; /**< array of pointers to mmapped
> > perf_event_attr structures */
>=20
> There seems to be a lot of indirection involved here. Why are these array=
s not statically sized,
> instead of dynamically allocated?
>=20

Different architectures/pmus impose limits on number of simultaneously enab=
led counters. So in order
relief the pain of thinking about it and adding macros for each and every a=
rch I decided to allocate
the number user wants dynamically. Also assumption holds that user knows ab=
out tradeoffs of using
too many counters hence will not enable too many events at once.=20

> Also, what is the reason for hiding the type struct perf_event_mmap_page =
**mmap_pages opaque by
> using void **mmap_pages instead?

I think, that part doing mmap/munmap was written first hence void ** was ch=
osen in the first place.=20

>=20
> > +	bool enabled; /**< true if group was enabled on particular lcore
> > */
> > +};
> > +
> > +/**
> > + * A structure describing an event.
> > + */
> > +struct rte_pmu_event {
> > +	char *name; /** name of an event */
> > +	int index; /** event index into fds/mmap_pages */
> > +	TAILQ_ENTRY(rte_pmu_event) next; /** list entry */ };
> > +
> > +/**
> > + * A PMU state container.
> > + */
> > +struct rte_pmu {
> > +	char *name; /** name of core PMU listed under
> > /sys/bus/event_source/devices */
> > +	struct rte_pmu_event_group group[RTE_MAX_LCORE]; /**< per lcore
> > event group data */
> > +	int num_group_events; /**< number of events in a group */
> > +	TAILQ_HEAD(, rte_pmu_event) event_list; /**< list of matching
> > events */
> > +};
> > +
> > +/** Pointer to the PMU state container */ extern struct rte_pmu
> > +*rte_pmu;
>=20
> Again, why not just extern struct rte_pmu, instead of dynamic allocation?
>=20

No strong opinions here since this is a matter of personal preference. Can =
be removed
in the next version.=20

> > +
> > +/** Each architecture supporting PMU needs to provide its own version
> > */
> > +#ifndef rte_pmu_pmc_read
> > +#define rte_pmu_pmc_read(index) ({ 0; }) #endif
> > +
> > +/**
> > + * @internal
> > + *
> > + * Read PMU counter.
> > + *
> > + * @param pc
> > + *   Pointer to the mmapped user page.
> > + * @return
> > + *   Counter value read from hardware.
> > + */
> > +__rte_internal
> > +static __rte_always_inline uint64_t
> > +rte_pmu_read_userpage(struct perf_event_mmap_page *pc) {
> > +	uint64_t offset, width, pmc =3D 0;
> > +	uint32_t seq, index;
> > +	int tries =3D 100;
> > +
> > +	for (;;) {
> > +		seq =3D pc->lock;
> > +		rte_compiler_barrier();
> > +		index =3D pc->index;
> > +		offset =3D pc->offset;
> > +		width =3D pc->pmc_width;
> > +
> > +		if (likely(pc->cap_user_rdpmc && index)) {
> > +			pmc =3D rte_pmu_pmc_read(index - 1);
> > +			pmc <<=3D 64 - width;
> > +			pmc >>=3D 64 - width;
> > +		}
> > +
> > +		rte_compiler_barrier();
> > +
> > +		if (likely(pc->lock =3D=3D seq))
> > +			return pmc + offset;
> > +
> > +		if (--tries =3D=3D 0) {
> > +			RTE_LOG(DEBUG, EAL, "failed to get
> > perf_event_mmap_page lock\n");
> > +			break;
> > +		}
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +/**
> > + * @internal
> > + *
> > + * Enable group of events for a given lcore.
> > + *
> > + * @param lcore_id
> > + *   The identifier of the lcore.
> > + * @return
> > + *   0 in case of success, negative value otherwise.
> > + */
> > +__rte_internal
> > +int
> > +rte_pmu_enable_group(int lcore_id);
> > +
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice
> > + *
> > + * Add event to the group of enabled events.
> > + *
> > + * @param name
> > + *   Name of an event listed under
> > /sys/bus/event_source/devices/pmu/events.
> > + * @return
> > + *   Event index in case of success, negative value otherwise.
> > + */
> > +__rte_experimental
> > +int
> > +rte_pmu_add_event(const char *name);
> > +
> > +/**
> > + * @warning
> > + * @b EXPERIMENTAL: this API may change without prior notice
> > + *
> > + * Read hardware counter configured to count occurrences of an event.
> > + *
> > + * @param index
> > + *   Index of an event to be read.
> > + * @return
> > + *   Event value read from register. In case of errors or lack of
> > support
> > + *   0 is returned. In other words, stream of zeros in a trace file
> > + *   indicates problem with reading particular PMU event register.
> > + */
> > +__rte_experimental
> > +static __rte_always_inline uint64_t
> > +rte_pmu_read(int index)
> > +{
> > +	int lcore_id =3D rte_lcore_id();
> > +	struct rte_pmu_event_group *group;
> > +	int ret;
> > +
> > +	if (!rte_pmu)
> > +		return 0;
> > +
> > +	group =3D &rte_pmu->group[lcore_id];
> > +	if (!group->enabled) {
> > +		ret =3D rte_pmu_enable_group(lcore_id);
> > +		if (ret)
> > +			return 0;
> > +
> > +		group->enabled =3D true;
> > +	}
>=20
> Why is the group not enabled in the setup function, rte_pmu_add_event(), =
instead of here, in the
> hot path?
>=20

When this is executed for the very first time then cpu will have obviously =
more work to do=20
but afterwards setup path is not taken hence much less cpu cycles are requi=
red.

Setup is executed by main lcore solely, before lcores are executed hence so=
me info passed to
SYS_perf_event_open ioctl() is missing, pid (via rte_gettid()) being an exa=
mple here.=20

> > +
> > +	if (index < 0 || index >=3D rte_pmu->num_group_events)
> > +		return 0;
> > +
> > +	return rte_pmu_read_userpage((struct perf_event_mmap_page
> > *)group->mmap_pages[index]);
>=20
> Using fixed size arrays instead of multiple indirections via pointers is =
faster. It could be:
>=20
> return rte_pmu_read_userpage((struct perf_event_mmap_page
> *)rte_pmu.group[lcore_id].mmap_pages[index]);
>=20
> With our without suggested performance improvements...
>=20
> Series-acked-by: Morten Br=F8rup <mb@smartsharesystems.com>