lib/libpmc/pmc.3
204	Unclear to me. Do you mean something like "Counters can be tied/specific to each NUMA domain"?
224	Likewise, do you mean "Counters can have a single value shared among all NUMA domains" here?

Tangentially: since MIPS CPUs are no longer supported per https://www.freebsd.org/platforms/, should they and the associated class still be listed in sys/sys/pmc.h?

In D35342#801259, @pauamma_gundo.com wrote:

Tangentially: since MIPS CPUs are no longer supported per https://www.freebsd.org/platforms/, should they and the associated class still be listed in sys/sys/pmc.h?

I think,it would indeed need cleanup, but I do not want to do it here IMO it should be done in separate commit.

pauamma_chezdork.com mentioned this in D35344: Refresh CPU types and classes from sys/sys/pmc.h..May 28 2022, 6:58 PM

In D35342#801269, @tsoome wrote:

In D35342#801259, @pauamma_gundo.com wrote:

Tangentially: since MIPS CPUs are no longer supported per https://www.freebsd.org/platforms/, should they and the associated class still be listed in sys/sys/pmc.h?

I think,it would indeed need cleanup, but I do not want to do it here IMO it should be done in separate commit.

Fair enough. (I can't do it myself, since I don't have the resources to test kernel or userland changes meaningfully.)

allanjude subscribed.Jun 23 2022, 7:20 PM

allanjude added inline comments.

lib/libpmc/pmc.3
204	It means that there will be a separate value/counter for each NUMA domain
224	One value for the entire system, rather than one per domain

pauamma_chezdork.com requested changes to this revision.Jun 25 2022, 6:41 PM

pauamma_chezdork.com added inline comments.

lib/libpmc/pmc.3
204
224

This revision now requires changes to proceed.Jun 25 2022, 6:41 PM

tsoome marked 6 inline comments as done.Jun 25 2022, 8:01 PM

tsoome added inline comments.

lib/libpmc/pmc.3
204	thanks!
224	thanks!

Updates to pmc.3 as suggested by pauamma_gundo.com.

Harbormaster completed remote builds in B46117: Diff 107387.Jun 25 2022, 8:03 PM

Seems to be the same as the original review with my two suggested tweaks? LGTM

In D35342#807371, @mhorne wrote:

Seems to be the same as the original review with my two suggested tweaks? LGTM

Quite so, yes. Since Aleksandr can not complete this work now, we do not want it to get lost.

This revision was not accepted when it landed; it landed in state Needs Review.Jun 26 2022, 9:12 AM

Closed by commit rGb6e28991bf3a: System wide and NUMA domain wide counters support. PMC classes for ARM DMC-620…. · Explain Why

This revision was automatically updated to reflect the committed changes.

tsoome added a commit: rGb6e28991bf3a: System wide and NUMA domain wide counters support. PMC classes for ARM DMC-620….

gallatin subscribed.Jul 4 2022, 3:33 PM

gallatin added inline comments.

usr.sbin/pmcstat/pmcstat.c

1190

This utterly breaks system wide profiling for me on all arches that I've tried (amd64, arm64). The cause is that we don't add the pmc for CPU0, as the ID is not invalid. This causes samples from CPU0 to fail to be recognized, and to cause an assertion when analyzing logs:

Core was generated by `pmcstat -R out.pmclog_cpi01 -z 32 -G out.pmcstat_cpi01'.
Program terminated with signal SIGABRT, Aborted.
Sent by thr_kill() from pid 13680 and user 0.
#0 thr_kill () at thr_kill.S:4
4 thr_kill.S: No such file or directory.
(gdb) bt
#0 thr_kill () at thr_kill.S:4
#1 0x00000f8a5e2021a4 in __raise (s=s@entry=0x6) at /data/ocafirmware/FreeBSD/lib/libc/gen/raise.c:52
#2 0x00000f8a5e2b3039 in abort () at /data/ocafirmware/FreeBSD/lib/libc/stdlib/abort.c:67
#3 0x00000f8a5e1e5011 in __assert (func=<optimized out>, file=<optimized out>, line=<optimized out>,
failedexpr=<optimized out>) at /data/ocafirmware/FreeBSD/lib/libc/gen/assert.c:51
#4 0x00000f8236919e09 in pmcstat_analyze_log (args=0xf823691e858 <args>, plugins=0xf823691e120 <plugins>,
pmcstat_stats=0xf823691e980 <pmcstat_stats>, pmcstat_kernproc=0xf8a602a50f0, pmcstat_mergepmc=0x0,
pmcstat_npmcs=0xf823691e9b4 <pmcstat_npmcs>, ps_samples_period=0xf823691e9b8 <ps_samples_period>)
at /data/ocafirmware/FreeBSD/lib/libpmcstat/libpmcstat_logging.c:293
#5 0x00000f823691367a in pmcstat_process_log () at /data/ocafirmware/FreeBSD/usr.sbin/pmcstat/pmcstat_log.c:534
#6 0x00000f8236912fc8 in main (argc=<optimized out>, argv=<optimized out>)
at /data/ocafirmware/FreeBSD/usr.sbin/pmcstat/pmcstat.c:1411
(gdb) frame 4
#4 0x00000f8236919e09 in pmcstat_analyze_log (args=0xf823691e858 <args>, plugins=0xf823691e120 <plugins>,
pmcstat_stats=0xf823691e980 <pmcstat_stats>, pmcstat_kernproc=0xf8a602a50f0, pmcstat_mergepmc=0x0,
pmcstat_npmcs=0xf823691e9b4 <pmcstat_npmcs>, ps_samples_period=0xf823691e9b8 <ps_samples_period>)
at /data/ocafirmware/FreeBSD/lib/libpmcstat/libpmcstat_logging.c:293
293 /data/ocafirmware/FreeBSD/lib/libpmcstat/libpmcstat_logging.c: No such file or directory.

gallatin mentioned this in D35709: fix pmcstat .Jul 4 2022, 3:48 PM

gallatin mentioned this in rG0aa150775179: pmcstat: fix log analysis.Jul 4 2022, 4:43 PM

pauamma_chezdork.com mentioned this in rG5faf9f4dff8d: Refresh CPU types and classes from sys/sys/pmc.h..Jan 18 2023, 8:52 PM

kevans added subscribers: np, kevans.Sep 18 2023, 5:30 PM

kevans added inline comments.

usr.sbin/pmcstat/pmcstat.c
738	@np hit an assertion similar to @gallatin's below in `pmcstat -TS` mode. It turns out that this is too early to allocate system events, the pmclog hasn't been configured yet so libpmcstat will not be made aware of any system-mode counters that are allocated. (Thus, logs recorded with -O are also not aware of these)