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 9B483423AC; Wed, 11 Jan 2023 10:31:11 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 5F71D40691; Wed, 11 Jan 2023 10:31:11 +0100 (CET) Received: from smartserver.smartsharesystems.com (smartserver.smartsharesystems.com [77.243.40.215]) by mails.dpdk.org (Postfix) with ESMTP id 7CB4840E25 for ; Wed, 11 Jan 2023 10:31:09 +0100 (CET) Content-class: urn:content-classes:message MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Subject: RE: [PATCH v5 0/4] add support for self monitoring Date: Wed, 11 Jan 2023 10:31:08 +0100 X-MimeOLE: Produced By Microsoft Exchange V6.5 Message-ID: <98CBD80474FA8B44BF855DF32C47DC35D8764E@smartserver.smartshare.dk> In-Reply-To: <20230111003222.GA24460@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: [PATCH v5 0/4] add support for self monitoring Thread-Index: AdklVC/Q59aD8UxMQS6KHkACy3BMlQAR+XiQ References: <20221213104350.3218167-1-tduszynski@marvell.com> <20230110234642.1188550-1-tduszynski@marvell.com> <20230111003222.GA24460@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> From: =?iso-8859-1?Q?Morten_Br=F8rup?= To: "Tyler Retzlaff" , "Tomasz Duszynski" , Cc: , , , , , 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 > From: Tyler Retzlaff [mailto:roretzla@linux.microsoft.com] > Sent: Wednesday, 11 January 2023 01.32 >=20 > On Wed, Jan 11, 2023 at 12:46:38AM +0100, Tomasz Duszynski wrote: > > This series adds self monitoring support i.e allows to configure and > > read performance measurement unit (PMU) counters in runtime without > > using perf utility. This has certain adventages when application = runs > on > > isolated cores with nohz_full kernel parameter. > > > > Events can be read directly using rte_pmu_read() or using dedicated > > tracepoint rte_eal_trace_pmu_read(). The latter will cause events to > be > > stored inside CTF file. > > > > By design, all enabled events are grouped together and the same = group > > is attached to lcores that use self monitoring funtionality. > > > > Events are enabled by names, which need to be read from standard > > location under sysfs i.e > > > > /sys/bus/event_source/devices/PMU/events > > > > where PMU is a core pmu i.e one measuring cpu events. As of today > > raw events are not supported. > > > > v5: > > - address review comments > > - fix sign extension while reading pmu on x86 > > - fix regex mentioned in doc > > - various minor changes/improvements here and there > > v4: > > - fix freeing mem detected by debug_autotest > > v3: > > - fix shared build > > v2: > > - fix problems reported by test build infra > > > > Tomasz Duszynski (4): > > eal: add generic support for reading PMU events > > eal/arm: support reading ARM PMU events in runtime > > eal/x86: support reading Intel PMU events in runtime > > eal: add PMU support to tracing library > > > > app/test/meson.build | 1 + > > app/test/test_pmu.c | 47 +++ > > app/test/test_trace_perf.c | 4 + > > doc/guides/prog_guide/profile_app.rst | 13 + > > doc/guides/prog_guide/trace_lib.rst | 32 ++ > > lib/eal/arm/include/meson.build | 1 + > > lib/eal/arm/include/rte_pmu_pmc.h | 39 ++ > > lib/eal/arm/meson.build | 4 + > > lib/eal/arm/rte_pmu.c | 104 +++++ > > lib/eal/common/eal_common_trace_points.c | 3 + > > lib/eal/common/meson.build | 3 + > > lib/eal/common/pmu_private.h | 41 ++ > > lib/eal/common/rte_pmu.c | 504 > +++++++++++++++++++++++ > > lib/eal/include/meson.build | 1 + > > lib/eal/include/rte_eal_trace.h | 10 + > > lib/eal/include/rte_pmu.h | 202 +++++++++ > > lib/eal/linux/eal.c | 4 + > > lib/eal/version.map | 7 + > > lib/eal/x86/include/meson.build | 1 + > > lib/eal/x86/include/rte_pmu_pmc.h | 33 ++ > > 20 files changed, 1054 insertions(+) > > create mode 100644 app/test/test_pmu.c > > create mode 100644 lib/eal/arm/include/rte_pmu_pmc.h > > create mode 100644 lib/eal/arm/rte_pmu.c > > create mode 100644 lib/eal/common/pmu_private.h > > create mode 100644 lib/eal/common/rte_pmu.c > > create mode 100644 lib/eal/include/rte_pmu.h > > create mode 100644 lib/eal/x86/include/rte_pmu_pmc.h > > > > -- > > 2.34.1 [Moved Tyler's post down here.] >=20 > hi, >=20 > don't interpret this as an objection to the functionality but this > looks > like a clear example of something that doesn't belong in the EAL. has > there been a discussion as to whether or not this should be in a > separate library? IIRC, there has been no such discussion. Although I agree that this doesn't belong in EAL, I would point to the = trace library as a reference for allowing it into the EAL. For the records, I also oppose to the trace library being part of the = EAL. On the other hand, it would be interesting to determine if it is = *impossible* adding this functionality as any other normal DPDK library, = i.e. outside of the EAL, or if there is an unavoidable tie-in to the = EAL. @Tomasz, if this is impossible, please describe the unavoidable tie-in = to the EAL. No need for a long report, just a few words. You (and this = functionality) shouldn't suffer from our long term ambition to move = stuff out of the EAL. >=20 > a basic test is whether or not an implementation exists or can be > reasonably provided for all platforms and that isn't strictly evident > here. red flag is to see yet more code being added conditionally > compiled for a single platform. Another basic test: Can DPDK applications run without it? If they can, = an Environment Abstraction Layer does not need to have it, and thus it = does not need to be part of the EAL. >=20 > Morten, Bruce comments? >=20 > thanks