git: 1ffcc2983834 - releng/14.0 - pmcstat: fix duplicate event allocation on CPU 0

From: Mitchell Horne <mhorne_at_FreeBSD.org>
Date: Tue, 17 Oct 2023 14:11:38 UTC
The branch releng/14.0 has been updated by mhorne:

URL: https://cgit.FreeBSD.org/src/commit/?id=1ffcc2983834accfd21089daa116ec802e90e088

commit 1ffcc2983834accfd21089daa116ec802e90e088
Author:     Mitchell Horne <mhorne@FreeBSD.org>
AuthorDate: 2023-09-27 16:37:46 +0000
Commit:     Mitchell Horne <mhorne@FreeBSD.org>
CommitDate: 2023-10-17 14:11:13 +0000

    pmcstat: fix duplicate event allocation on CPU 0
    
    Commit b6e28991bf3a modified the allocation path for system scope PMCs
    so that the event was allocated early for CPU 0. The reason is so that
    the PMC's capabilities could be checked, to determine if pmcstat should
    allocate the event on every CPU, or just on one CPU in each NUMA domain.
    In the current scheme, there is no way to determine this information
    without performing the PMC allocation.
    
    This broke the established use-case of log analysis, and so
    0aa150775179a was committed to fix the assertion. The result was what
    appeared to be functional, but in normal counter measurement pmcstat was
    silently allocating two counters for CPU 0.
    
    This cuts the total number of counters that can be allocated from a CPU
    in half. Additionally, depending on the particular hardware/event, we
    might not be able to allocate the same event twice on a single CPU.
    
    The simplest solution is to release the early-allocated PMC once we have
    obtained its capabilities, and reallocate it later on. This restores the
    event list logic to behave as it has for many years, and partially
    reverts commit b6e28991bf3a.
    
    Approved by:    re (karels)
    Reported by:    alc, kevans
    Reviewed by:    jkoshy, ray
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D41978
    
    (cherry picked from commit c362fe939f6fe52056fb7506be9e5cbd0a5ef60b)
    (cherry picked from commit 5a2849bc3c60426039ff2aeef1d2b54940152927)
---
 usr.sbin/pmcstat/pmcstat.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/usr.sbin/pmcstat/pmcstat.c b/usr.sbin/pmcstat/pmcstat.c
index fd4be99f83c8..c36cee436e55 100644
--- a/usr.sbin/pmcstat/pmcstat.c
+++ b/usr.sbin/pmcstat/pmcstat.c
@@ -713,8 +713,16 @@ main(int argc, char **argv)
 				errx(EX_SOFTWARE, "ERROR: Out of memory.");
 			(void) strncpy(ev->ev_name, optarg, c);
 			*(ev->ev_name + c) = '\0';
+
 			libpmc_initialize(&npmc);
+
 			if (args.pa_flags & FLAG_HAS_SYSTEM_PMCS) {
+				/*
+				 * We need to check the capabilities of the
+				 * desired event to determine if it should be
+				 * allocated on every CPU, or only a subset of
+				 * them. This requires allocating a PMC now.
+				 */
 				if (pmc_allocate(ev->ev_spec, ev->ev_mode,
 				    ev->ev_flags, ev->ev_cpu, &ev->ev_pmcid,
 				    ev->ev_count) < 0)
@@ -726,8 +734,14 @@ main(int argc, char **argv)
 					err(EX_OSERR, "ERROR: Cannot get pmc "
 					    "capabilities");
 				}
-			}
 
+				/*
+				 * Release the PMC now that we have caps; we
+				 * will reallocate shortly.
+				 */
+				pmc_release(ev->ev_pmcid);
+				ev->ev_pmcid = PMC_ID_INVALID;
+			}
 
 			STAILQ_INSERT_TAIL(&args.pa_events, ev, ev_next);
 
@@ -751,10 +765,7 @@ main(int argc, char **argv)
 			}
 			if (option == 's' || option == 'S') {
 				CPU_CLR(ev->ev_cpu, &cpumask);
-				pmc_id_t saved_pmcid = ev->ev_pmcid;
-				ev->ev_pmcid = PMC_ID_INVALID;
 				pmcstat_clone_event_descriptor(ev, &cpumask, &args);
-				ev->ev_pmcid = saved_pmcid;
 				CPU_SET(ev->ev_cpu, &cpumask);
 			}