On Wed, Jan 11, 2023 at 09:39:35AM +0000, Tomasz Duszynski wrote: > Hi Tyler, > > >-----Original Message----- > >From: Tyler Retzlaff <roret...@linux.microsoft.com> > >Sent: Wednesday, January 11, 2023 1:32 AM > >To: Tomasz Duszynski <tduszyn...@marvell.com>; bruce.richard...@intel.com; > >m...@smartsharesystems.com > >Cc: dev@dpdk.org; tho...@monjalon.net; Jerin Jacob Kollanukkaran > ><jer...@marvell.com>; > >m...@smartsharesystems.com; ruifeng.w...@arm.com; > >mattias.ronnb...@ericsson.com; zhou...@loongson.cn > >Subject: [EXT] Re: [PATCH v5 0/4] add support for self monitoring > > > >External Email > > > >---------------------------------------------------------------------- > >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!
