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

["Morten Brørup" <mb@smartsharesystems.com>]

> From: Jerin Jacob [mailto:jerinjacobk@gmail.com]
> Sent: Friday, 26 August 2022 10.16
> 
> On Fri, Aug 26, 2022 at 1:37 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> >
> > 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.)
> 
> I was talking about buildtime only. "enable_trace_fp" meson option
> selected as
> false as default.

Agree. "enable_lcore_poll_busyness" is in the fast path, so it should follow the design pattern of "enable_trace_fp".

> 
> If the concern is enabling on generic distros then distro generic
> config can opt in this
> 
> >
> > /Bruce

@Kevin, are you considering a roadmap for using RTE_LCORE_TELEMETRY_TIMESTAMP() for other purposes? Otherwise, it should also be renamed to indicate that it is part of the "poll busyness" telemetry.