Re: [PATCH v10 03/10] lib/pmu: quiesce rte_pmu_read deprecation warning in chkincs

[Thomas Monjalon <thomas@monjalon.net>]

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 <tduszynski@marvell.com> 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 <tduszynski@marvell.com>
> > > > > > ---
> > > > > >  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