[EXT] Re: [PATCH v5 0/4] add support for self monitoring

Morten Brørup mb at smartsharesystems.com
Sat Jan 14 10:53:57 CET 2023


> From: Tyler Retzlaff [mailto:roretzla at linux.microsoft.com]
> Sent: Friday, 13 January 2023 20.22
> 
> On Fri, Jan 13, 2023 at 07:44:57AM +0000, Tomasz Duszynski wrote:
> >
> > >From: Tyler Retzlaff <roretzla at linux.microsoft.com>
> > >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 <roretzla at linux.microsoft.com>
> > >> >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?
> 
> naming is always so hard and i'm definitely not authoritative.
> 
> 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.



More information about the dev mailing list