Re: [PATCH v3 1/3] eal: add lcore poll busyness telemetry

[Bruce Richardson <bruce.richardson@intel.com>]

On Fri, Aug 26, 2022 at 12:35:16PM +0530, Jerin Jacob wrote:
> On Thu, Aug 25, 2022 at 8:56 PM Kevin Laatz <kevin.laatz@intel.com> wrote:
> >
> > From: Anatoly Burakov <anatoly.burakov@intel.com>
> >
> > Currently, there is no way to measure lcore poll busyness in a passive way,
> > without any modifications to the application. This patch adds a new EAL API
> > that will be able to passively track core polling busyness.
> >
> > The poll busyness is calculated by relying on the fact that most DPDK API's
> > will poll for packets. Empty polls can be counted as "idle", while
> > non-empty polls can be counted as busy. To measure lcore poll busyness, we
> > simply call the telemetry timestamping function with the number of polls a
> > particular code section has processed, and count the number of cycles we've
> > spent processing empty bursts. The more empty bursts we encounter, the less
> > cycles we spend in "busy" state, and the less core poll busyness will be
> > reported.
> >
> > In order for all of the above to work without modifications to the
> > application, the library code needs to be instrumented with calls to the
> > lcore telemetry busyness timestamping function. The following parts of DPDK
> > are instrumented with lcore telemetry calls:
> >
> > - All major driver API's:
> >   - ethdev
> >   - cryptodev
> >   - compressdev
> >   - regexdev
> >   - bbdev
> >   - rawdev
> >   - eventdev
> >   - dmadev
> > - Some additional libraries:
> >   - ring
> >   - distributor
> >
> > To avoid performance impact from having lcore telemetry support, a global
> > variable is exported by EAL, and a call to timestamping function is wrapped
> > into a macro, so that whenever telemetry is disabled, it only takes one
> > additional branch and no function calls are performed. It is also possible
> > to disable it at compile time by commenting out RTE_LCORE_BUSYNESS from
> > build config.
> >
> > This patch also adds a telemetry endpoint to report lcore poll busyness, as
> > well as telemetry endpoints to enable/disable lcore telemetry. A
> > documentation entry has been added to the howto guides to explain the usage
> > of the new telemetry endpoints and API.
> >
> > Signed-off-by: Kevin Laatz <kevin.laatz@intel.com>
> > Signed-off-by: Conor Walsh <conor.walsh@intel.com>
> > Signed-off-by: David Hunt <david.hunt@intel.com>
> > Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
> >
> > ---
> > v3:
> >   * Fix missed renaming to poll busyness
> >   * Fix clang compilation
> >   * Fix arm compilation
> >
> > v2:
> >   * Use rte_get_tsc_hz() to adjust the telemetry period
> >   * Rename to reflect polling busyness vs general busyness
> >   * Fix segfault when calling telemetry timestamp from an unregistered
> >     non-EAL thread.
> >   * Minor cleanup
> > ---
> 
> > diff --git a/meson_options.txt b/meson_options.txt
> > index 7c220ad68d..725b851f69 100644
> > --- a/meson_options.txt
> > +++ b/meson_options.txt
> > @@ -20,6 +20,8 @@ option('enable_driver_sdk', type: 'boolean', value: false, description:
> >         'Install headers to build drivers.')
> >  option('enable_kmods', type: 'boolean', value: false, description:
> >         'build kernel modules')
> > +option('enable_lcore_poll_busyness', type: 'boolean', value: true, description:
> > +       'enable collection of lcore poll busyness telemetry')
> 
> IMO, All fastpath features should be opt-in. i.e default should be false.
> For the trace fastpath related changes, We have done the similar thing
> even though it cost additional one cycle for disabled trace points
> 

We do need to consider runtime and build defaults differently, though.
Since this has also runtime enabling, I think having build-time enabling
true as default is ok, so long as the runtime enabling is false (assuming
no noticable overhead when the feature is disabled.)

/Bruce