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 1E3C3423D3; Sat, 14 Jan 2023 10:54:01 +0100 (CET) Received: from mails.dpdk.org (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id B714340156; Sat, 14 Jan 2023 10:54:00 +0100 (CET) Received: from smartserver.smartsharesystems.com (smartserver.smartsharesystems.com [77.243.40.215]) by mails.dpdk.org (Postfix) with ESMTP id 6508F40042 for ; Sat, 14 Jan 2023 10:53:59 +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: [EXT] Re: [PATCH v5 0/4] add support for self monitoring Date: Sat, 14 Jan 2023 10:53:57 +0100 X-MimeOLE: Produced By Microsoft Exchange V6.5 Message-ID: <98CBD80474FA8B44BF855DF32C47DC35D87670@smartserver.smartshare.dk> In-Reply-To: <20230113192211.GC11082@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> X-MS-Has-Attach: X-MS-TNEF-Correlator: Thread-Topic: [EXT] Re: [PATCH v5 0/4] add support for self monitoring Thread-Index: AdknhFkyZS9sv2GwR+O/DWFeMuv5wAAd1Edg References: <20221213104350.3218167-1-tduszynski@marvell.com> <20230110234642.1188550-1-tduszynski@marvell.com> <20230111003222.GA24460@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> <20230111210547.GA2134@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> <20230113192211.GC11082@linuxonhyperv3.guj3yctzbm1etfxqx2vob5hsef.xx.internal.cloudapp.net> From: =?iso-8859-1?Q?Morten_Br=F8rup?= To: "Tyler Retzlaff" , "Tomasz Duszynski" Cc: , , , "Jerin Jacob Kollanukkaran" , , , 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: Friday, 13 January 2023 20.22 >=20 > On Fri, Jan 13, 2023 at 07:44:57AM +0000, Tomasz Duszynski wrote: > > > > >From: Tyler Retzlaff > > >Sent: Wednesday, January 11, 2023 10:06 PM > > > > > >On Wed, Jan 11, 2023 at 09:39:35AM +0000, Tomasz Duszynski wrote: > > >> Hi Tyler, > > >> > > >> >From: Tyler Retzlaff > > >> >Sent: Wednesday, January 11, 2023 1:32 AM > > >> > > > >> >hi, > > >> > > > >> >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? > > >> > > >> No, I don't recall anybody having any concerns about the code > > >> placement. Rationale behind making this part of eal was based on > the > > >> fact that tracing itself is a part of eal and since this was = meant > to be extension to tracing, > > >code placement decision came out naturally. > > >> > > >> During development phase idea evolved a bit and what initially = was > > >> supposed to be solely yet another tracepoint become generic API = to > > >> read pmu and tracepoint based on that. Which means both can be > used independently. > > >> > > >> That said, since this code has both platform agnostic and = platform > specific parts this can either > > >be split into: > > >> 1. library + eal platform code > > >> 2. all under eal > > >> > > >> Either approach seems legit. Thoughts? > > >> > > >> > > > >> >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. > > >> > > >> Even libs are not entirely pristine and have platform specific > ifdefs > > >> lurking so not sure where this red flag is coming from. > > > > > >i think red flag was probably the wrong term to use sorry for that. > > >rather i should say it is an indicator that the api probably = doesn't > belong in the eal. > > > > > >fundamentally the purpose of the abstraction library is to relieve > the application from having to > > >do conditional compilation and/or execution for the subject apis > coming from eal. including and > > >exporting apis that work for only one platform is in direct > contradiction. > > > > > >please explore adding this as a separate library, it is understood > that there are tradeoffs > > >involved. > > > > > >thanks! > > > > Any ideas how to name the library? >=20 > naming is always so hard and i'm definitely not authoritative. >=20 > it seems like lib/pmu would be the least churn against your existing > patch series, here are some other suggestions that might work. +1 to lib/pmu Less work, as Tyler already mentioned. Furthermore: Both Intel and ARM use the term Performance Monitoring Unit (abbreviated = PMU). Microsoft does too [1]. [1]: = https://learn.microsoft.com/en-us/windows-hardware/test/wpt/recording-pmu= -events RISC-V uses the term Hardware Performance Monitor (abbreviated HPM). I haven't checked other CPU vendors.