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 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 ; 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 To: =?iso-8859-1?Q?Morten_Br=F8rup?= , "dev@dpdk.org" CC: "thomas@monjalon.net" , Jerin Jacob Kollanukkaran , "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: 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-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