Hello Tomasz, Recently David has improved the checks on headers. We would like to see a new revision (with the approach described below by David) passing in the CI with the new checks.
Thanks 17/10/2025 10:45, David Marchand: > On Fri, 17 Oct 2025 at 08:58, Tomasz Duszynski <[email protected]> wrote: > > > > > > > > Add a dummy rte_pmu_read() definition for chkincs when > > > > > > ALLOW_EXPERIMENTAL_API is not defined to suppress warnings from > > > > > > use of experimental APIs in tracepoints. > > > > > > > > > > > > Fixes: 960c43184c4d ("pmu: introduce library for reading PMU > > > > > > events") > > > > > > > > > > > > Signed-off-by: Tomasz Duszynski <[email protected]> > > > > > > --- > > > > > > lib/pmu/rte_pmu.h | 4 ++++ > > > > > > 1 file changed, 4 insertions(+) > > > > > > > > > > > > diff --git a/lib/pmu/rte_pmu.h b/lib/pmu/rte_pmu.h > > > > > > index 57b634ecd8..84a5d522d1 100644 > > > > > > --- a/lib/pmu/rte_pmu.h > > > > > > +++ b/lib/pmu/rte_pmu.h > > > > > > @@ -232,6 +232,10 @@ rte_pmu_read(unsigned int index) > > > > > > > > > > > > return __rte_pmu_read_userpage(group->mmap_pages[index]); > > > > > > } > > > > > > +/* quiesce warnings produced by chkincs */ > > > > > > +#ifndef ALLOW_EXPERIMENTAL_API > > > > > > +#define rte_pmu_read(pc) ({ RTE_SET_USED(pc); 0; }) > > > > > > +#endif > > > > > > > > > > Seems like an awkward solution I would rather that fix chkincs > > > > > or restructure this. > > > > > > > > > > The way you are doing it will cause the checks for use > > > > > of experimental API to not work. > > > > > > > > No matter how I shuffle things around, the experimental API keeps > > > > getting called from > > > > non-experimental code, so those warnings will show up.. Fixing chkincs > > > > would mean telling it to > > > > skip checks for this library, which is basically what I'm already doing > > > > here. > > > > > > > > Dropping experimental tag feels too soon. The library still needs some > > > > time to bake. > > > > > > > > So, unless anyone has got other ideas, which is the better trade off > > > > here: yanking experimental > > > > tags or tweaking chkincs to ignore the library checks? > > > > > What we have been doing in other libraries is to hide calls to > experimental symbols in inline helpers, which is the issue here, > right? > Then add a RTE_VERIFY(false) so that an abort is triggered at runtime > in case an application called this helper. > > See a3e126fd58d1 ("bitset: fix build for GCC without experimental > API") for example. > > > I see the same #define hiding for __rte_pmu_enable_group and > __rte_pmu_read_userpage. > Something like below should be applied on main before looking at this > new series. > > $ git diff > diff --git a/lib/pmu/rte_pmu.h b/lib/pmu/rte_pmu.h > index 57b634ecd8..fcf125a908 100644 > --- a/lib/pmu/rte_pmu.h > +++ b/lib/pmu/rte_pmu.h > @@ -31,6 +31,7 @@ > #include <rte_branch_prediction.h> > #include <rte_common.h> > #include <rte_compat.h> > +#include <rte_debug.h> > #include <rte_lcore.h> > > #define RTE_PMU_SUPPORTED > @@ -181,12 +182,6 @@ __rte_experimental > int > rte_pmu_add_event(const char *name); > > -/* quiesce warnings produced by chkincs caused by calling internal > functions directly */ > -#ifndef ALLOW_EXPERIMENTAL_API > -#define __rte_pmu_enable_group(group) ({ RTE_SET_USED(group); 0; }) > -#define __rte_pmu_read_userpage(pc) ({ RTE_SET_USED(pc); 0; }) > -#endif > - > /** > * @warning > * @b EXPERIMENTAL: this API may change without prior notice. > @@ -211,6 +206,7 @@ __rte_experimental > static __rte_always_inline uint64_t > rte_pmu_read(unsigned int index) > { > +#ifdef ALLOW_EXPERIMENTAL_API > unsigned int lcore_id = rte_lcore_id(); > struct rte_pmu_event_group *group; > > @@ -231,6 +227,10 @@ rte_pmu_read(unsigned int index) > } > > return __rte_pmu_read_userpage(group->mmap_pages[index]); > +#else > + RTE_SET_USED(index); > + RTE_VERIFY(false); > +#endif > } > > #ifdef __cplusplus
