> From: Tomasz Duszynski [mailto:tduszyn...@marvell.com] > Sent: Thursday, 26 January 2023 10.40 > > >From: Morten Brørup <m...@smartsharesystems.com> > >Sent: Friday, January 20, 2023 10:47 AM > > > >> From: Tomasz Duszynski [mailto:tduszyn...@marvell.com] > >> Sent: Friday, 20 January 2023 00.39 > >> > >> Add support for programming PMU counters and reading their values in > >> runtime bypassing kernel completely. > >> > >> This is especially useful in cases where CPU cores are isolated > >> (nohz_full) i.e run dedicated tasks. In such cases one cannot use > >> standard perf utility without sacrificing latency and performance. > >> > >> Signed-off-by: Tomasz Duszynski <tduszyn...@marvell.com> > >> --- > > > >If you insist on passing lcore_id around as a function parameter, the > function description must > >mention that the lcore_id parameter must be set to rte_lcore_id() for > the functions where this is a > >requirement, including all functions that use those functions.
Perhaps I'm stating this wrong, so let me try to rephrase: As I understand it, some of the setup functions must be called from the EAL thread that executes that function - due to some syscall (SYS_perf_event_open) needing to be called from the thread itself. Those functions should not take an lcore_id parameter. Otherwise, I would expect to be able to call those functions from e.g. the main thread and pass the lcore_id of any EAL thread as a parameter, which you at the bottom of this email [1] explained is not possible. [1]: http://inbox.dpdk.org/dev/dm4pr18mb4368461ec42603f77a7dc1bcd2...@dm4pr18mb4368.namprd18.prod.outlook.com/ > > > > Not sure why are you insisting so much on removing that rte_lcore_id(). > Yes that macro evaluates > to integer but if you don't think about internals this resembles a > function call. I agree with this argument. And for that reason, passing lcore_id around could be relevant. I only wanted to bring your attention to the low cost of fetching it inside the functions, as an alternative to passing it as an argument. > > Then natural pattern is to call it once and reuse results if possible. Yes, and I would usually agree to using this pattern. > Passing lcore_id around > implies that calls are per l-core, why would that confuse anyone > reading that code? This is where I disagree: Passing lcore_id as a parameter to a function does NOT imply that the function is running on that lcore! E.g rte_mempool_default_cache(struct rte_mempool *mp, unsigned lcore_id) [2] takes lcore_id as a parameter, and does not assume that lcore_id==rte_lcore_id(). [2]: https://elixir.bootlin.com/dpdk/latest/source/lib/mempool/rte_mempool.h#L1315 > > Besides, all functions taking it are internal stuff hence you cannot > call it elsewhere. OK. I agree that this reduces the risk of incorrect use. Generally, I think that internal functions should be documented too. Not to the full extent, like public functions, but some documentation is nice. And if there are special requirements to a function parameter, it should be documented with that function. Requiring that the lcore_id parameter must be == rte_lcore_id() is certainly a special requirement. It might just be me worrying too much, so... If nobody else complains about this, I can live with it as is. Assuming that none of the public functions have this special requirement (either directly or indirectly, by calling functions with the special requirement). > > >Alternatively, follow my previous suggestion: Omit the lcore_id > function parameter, and use > >rte_lcore_id() instead. > > >
