mbox series

[v11,0/9] lib/pmu: cleanups and trace integration

Message ID 20251024054830.933910-1-tduszynski@marvell.com (mailing list archive)
Headers
Series lib/pmu: cleanups and trace integration |

Message

Tomasz Duszynski Oct. 24, 2025, 5:48 a.m. UTC
This series does some cleanup and refactoring around the rc1 code like: trimming unused headers,
switching to callbacks for per-arch handling, and adding trace support. It also re-enables existing
base test to help catch reported issues on some architectures.

v11:
- rebase series
- hide calls to experimental syms in inline helpers
v10:
- fix build without ALLOW_EXPERIMENTAL_API
- move rte_pmu_tread_read() registration to avoid MSVC linker issues
v9:
- properly rebase patch integrating pmu and trace
v8:
- export __rte_pmu_trace_read from library itself to avoid build issues
  with msvc linker
v7:
- change test return value
v6:
- add more logs to functional test
- skip test in case of setup failure, user must make sure
  system is properly configured to get valid results
v5:
- add missing patch that quiesces chincs check
v4:
- change fast test so that it won't fail on misconfigured system
- fix compilation on windows
v3:
- do not export __rte_pmu_trace_read because that breaks compilation
  on windows - script generating map files does not handle conditional
  compilation
- skip testing if paranoia is at wrong level
v2:
- explicitly check against NULL
- make pmu lib optional by checking if dpdk config has RTE_LIB_PMU

Tomasz Duszynski (10):
  trace: change scope of conditional block
  lib/pmu: export only necessary arch headers
  lib/pmu: reimplement per-arch ops as callbacks
  lib/pmu: do not try enabling perf counter access on arm64
  lib/pmu: use build system defined RTE_LIB_PMU macro
  test/pmu: enable test
  trace: add PMU
  lib/pmu: fix out-of-bound access

 MAINTAINERS                                |  1 +
 app/test/test_pmu.c                        | 60 ++++++++++++--
 app/test/test_trace_perf.c                 | 10 +++
 doc/guides/prog_guide/profile_app.rst      |  5 ++
 doc/guides/prog_guide/trace_lib.rst        | 31 ++++++++
 lib/eal/common/eal_common_trace.c          |  6 +-
 lib/eal/common/eal_common_trace_pmu.c      | 45 +++++++++++
 lib/eal/common/eal_trace_pmu.h             | 12 +++
 lib/eal/common/meson.build                 |  1 +
 lib/eal/include/rte_eal_trace.h            | 23 ++++++
 lib/eal/include/rte_trace_point.h          | 10 ++-
 lib/eal/include/rte_trace_point_register.h |  2 +
 lib/eal/meson.build                        |  3 +
 lib/meson.build                            |  2 +-
 lib/pmu/meson.build                        | 10 +--
 lib/pmu/pmu.c                              | 93 ++++++++++++++++------
 lib/pmu/pmu_arm64.c                        | 59 +++++---------
 lib/pmu/pmu_private.h                      | 51 ++++++++++--
 lib/pmu/rte_pmu.h                          | 37 +++++++--
 19 files changed, 372 insertions(+), 89 deletions(-)
 create mode 100644 lib/eal/common/eal_common_trace_pmu.c
 create mode 100644 lib/eal/common/eal_trace_pmu.h

--
2.34.1
  

Comments

Tomasz Duszynski Oct. 30, 2025, 3:50 a.m. UTC | #1
Anything else required from your angle?
  
David Marchand Nov. 5, 2025, 1:38 p.m. UTC | #2
Hello,

On Fri, 24 Oct 2025 at 07:49, Tomasz Duszynski <tduszynski@marvell.com> wrote:
>
> This series does some cleanup and refactoring around the rc1 code like: trimming unused headers,
> switching to callbacks for per-arch handling, and adding trace support. It also re-enables existing
> base test to help catch reported issues on some architectures.
>
> v11:
> - rebase series
> - hide calls to experimental syms in inline helpers
> v10:
> - fix build without ALLOW_EXPERIMENTAL_API
> - move rte_pmu_tread_read() registration to avoid MSVC linker issues
> v9:
> - properly rebase patch integrating pmu and trace
> v8:
> - export __rte_pmu_trace_read from library itself to avoid build issues
>   with msvc linker
> v7:
> - change test return value
> v6:
> - add more logs to functional test
> - skip test in case of setup failure, user must make sure
>   system is properly configured to get valid results
> v5:
> - add missing patch that quiesces chincs check
> v4:
> - change fast test so that it won't fail on misconfigured system
> - fix compilation on windows
> v3:
> - do not export __rte_pmu_trace_read because that breaks compilation
>   on windows - script generating map files does not handle conditional
>   compilation
> - skip testing if paranoia is at wrong level
> v2:
> - explicitly check against NULL
> - make pmu lib optional by checking if dpdk config has RTE_LIB_PMU
>

Strange output in the cover letter.

Shortlog shows 10 patches:
> Tomasz Duszynski (10):

Followed by 8 lines:
>   trace: change scope of conditional block
>   lib/pmu: export only necessary arch headers
>   lib/pmu: reimplement per-arch ops as callbacks
>   lib/pmu: do not try enabling perf counter access on arm64
>   lib/pmu: use build system defined RTE_LIB_PMU macro
>   test/pmu: enable test
>   trace: add PMU
>   lib/pmu: fix out-of-bound access

But the series has 9 patches.

In any case,
- patch 2 "trace: change scope of conditional block" is unneeded, I
see nothing wrong with current code. I tried stopping at various
points of the series, no build issue,
- patch 7 has a comment from Morten,
- patch 8 has comments from me,
- patch 9 is vague, what is this about? Fixing coverity or some static
analysis tool bug report?

I applied the rest of the series, as other patches look valid fixes / cleanups.
  
Tomasz Duszynski Nov. 7, 2025, 2:33 p.m. UTC | #3
On Wed, Nov 05, 2025 at 02:38:44PM +0100, David Marchand wrote:
> Strange output in the cover letter.

I think I let vimdiff copy to much from previous cover letter.

>
> Shortlog shows 10 patches:
> > Tomasz Duszynski (10):
>
> Followed by 8 lines:
> >   trace: change scope of conditional block
> >   lib/pmu: export only necessary arch headers
> >   lib/pmu: reimplement per-arch ops as callbacks
> >   lib/pmu: do not try enabling perf counter access on arm64
> >   lib/pmu: use build system defined RTE_LIB_PMU macro
> >   test/pmu: enable test
> >   trace: add PMU
> >   lib/pmu: fix out-of-bound access
>
> But the series has 9 patches.
>
> In any case,
> - patch 2 "trace: change scope of conditional block" is unneeded, I
> see nothing wrong with current code. I tried stopping at various
> points of the series, no build issue,

I'll take another look because I cannot recall at this point all
specifics of this change. Most likely thing was some CI build issue.

> - patch 7 has a comment from Morten,
> - patch 8 has comments from me,

Ack.

> - patch 9 is vague, what is this about? Fixing coverity or some static
> analysis tool bug report?

Most likely that was from some coverity report. I don't have access to
corporate mbox so I'm not able to track this down at this point.

Regardless of that, this fix is about making string NUL terminated.
Otherwise strtol() may go haywire.

>
> I applied the rest of the series, as other patches look valid fixes / cleanups.
>

Thanks!

>
> --
> David Marchand
>