>On Wed, 9 Oct 2024 14:50:02 +0200 >Morten Brørup <m...@smartsharesystems.com> wrote: > >> > From: Tomasz Duszynski [mailto:tduszyn...@marvell.com] >> > Sent: Wednesday, 9 October 2024 13.23 >> > >> > +PMU tracepoint >> > +-------------- >> > + >> > +Performance monitoring unit (PMU) event values can be read from >> > hardware >> > +registers using predefined ``rte_pmu_read`` tracepoint. >> > + >> > +Tracing is enabled via ``--trace`` EAL option by passing both >> > expression >> > +matching PMU tracepoint name i.e ``lib.eal.pmu.read`` and >> > +expression ``e=ev1[,ev2,...]`` matching particular events:: >> > + >> > + --trace='.*pmu.read\|e=cpu_cycles,l1d_cache' >> > + >> > +Event names are available under >> > ``/sys/bus/event_source/devices/PMU/events`` >> > +directory, where ``PMU`` is a placeholder for either a ``cpu`` or a >> > directory >> > +containing ``cpus``. >> > + >> > +In contrary to other tracepoints this does not need any extra >> > variables >> > +added to source files. Instead, caller passes index which follows >> > +the >> > order of >> > +events specified via ``--trace`` parameter. In the following >> > +example >> > index ``0`` >> > +corresponds to ``cpu_cyclces`` while index ``1`` corresponds to >> > ``l1d_cache``. >> > + >> > +.. code-block:: c >> > + >> > + ... >> > + rte_eal_trace_pmu_read(0); >> > + rte_eal_trace_pmu_read(1); >> > + ... >> > + >> > +PMU tracing support must be explicitly enabled using the >> > ``enable_trace_fp`` >> > +option for meson build. >> > + >> >> >> > +int >> > +rte_pmu_add_events_by_pattern(const char *pattern) { >> > + regmatch_t rmatch; >> > + char buf[BUFSIZ]; >> > + unsigned int num; >> > + regex_t reg; >> > + int ret; >> > + >> > + /* events are matched against occurrences of e=ev1[,ev2,..] >> > pattern */ >> > + ret = regcomp(®, "e=([_[:alnum:]-],?)+", REG_EXTENDED); >> > + if (ret) >> > + return -EINVAL; >> > + >> > + for (;;) { >> > + if (regexec(®, pattern, 1, &rmatch, 0)) >> > + break; >> > + >> > + num = rmatch.rm_eo - rmatch.rm_so; >> > + if (num > sizeof(buf)) >> > + num = sizeof(buf); >> > + >> > + /* skip e= pattern prefix */ >> > + memcpy(buf, pattern + rmatch.rm_so + 2, num - 2); >> > + buf[num - 2] = '\0'; >> > + ret = add_events(buf); >> > + if (ret) >> > + break; >> > + >> > + pattern += rmatch.rm_eo; >> > + } >> > + >> > + regfree(®); >> > + >> > + return ret; >> > +} >> >> This --trace parameter takes a regex, but the --log-level parameter takes a >> globbing pattern (and >a regex, for backwards compatibility, I assume). >> >> This --trace parameter should behave like the --log-level parameter, or if >> not able to parse >both, use globbing pattern, not regex. >> >> Check the --trace parameter parser here: >> https://urldefense.proofpoint.com/v2/url?u=https-3A__elixir.bootlin.co >> m_dpdk_v24.07_source_lib_eal_common_eal-5Fcommon-5Foptions.c-23L1409
Just in case, I'll stress that --trace parameter is inherited from tracing library and that itself takes a regex. Checked both documentation and sources for any discrepancies but there are none. That means there has never been any compatibility with --log-level behavior and I don't think that patch should straight things out. Maybe addressing that separately would be more sensible. > >The log level has two API's one takes regex and one takes file glob pattern. >The original one was the regex, but everybody (including the documentation) >got regex wrong when >the log level format had periods in it. The glob match was more intuitive, and >that is what is >recommended. > Thanks for providing more context. >For tracing if you want to use regex then function should be named with >_regexp( For extra bonus >points, having both a regex and glob version (with pattern) would be great. Adding extra suffix is fine. glob match lacks some syntax used for specifying events to read, which is \| (OR). Checked the link from Morten and seems regex matching is turned on whenever log-level contains a coma. Unfortunately I treat coma as event delimiter so presumably syntax would need to be changed. Maybe something like '.*pmu.read:cpu_cycles;l1d_cache,*some_other_unrelated tp*' would be more appropriate if making --trace similar to --log-level is the way to go.
