Date: Mon, 20 Jul 2026 10:25:16 +0000 From: Alexander Leidinger <netchild@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: 86fa065f1862 - main - hwpmc: drain a process-mode PMC's runcount when a live target detaches Message-ID: <6a5df78c.33323.733bf27@gitrepo.freebsd.org>
index | next in thread | raw e-mail
The branch main has been updated by netchild: URL: https://cgit.FreeBSD.org/src/commit/?id=86fa065f1862f3b638efa1868523878d9db14ada commit 86fa065f1862f3b638efa1868523878d9db14ada Author: Alexander Leidinger <netchild@FreeBSD.org> AuthorDate: 2026-07-19 11:39:27 +0000 Commit: Alexander Leidinger <netchild@FreeBSD.org> CommitDate: 2026-07-20 10:24:46 +0000 hwpmc: drain a process-mode PMC's runcount when a live target detaches A process-mode PMC's runcount tracks how many CPUs currently have it loaded in hardware. It is decremented only by the context-switch-out and process-exit reclaim paths, both of which the scheduler invokes only for processes flagged P_HWPMC. Detaching a target that still has the PMC live in hardware dropped the target and cleared P_HWPMC without taking the PMC off the hardware or dropping the runcount reference, so the reference leaked. A subsequent release then spun in pmc_wait_for_pmc_idle() forever waiting for the runcount to reach zero: on an INVARIANTS kernel this panics ("waiting too long for pmc to be free"), otherwise it is an unkillable loop holding the hwpmc lock. Any process able to allocate a PMC can trigger this by attaching a counting PMC to itself and detaching it before releasing. Take the PMC off the hardware and drop the runcount reference as part of detaching, before P_HWPMC is cleared: reclaim it from the detaching thread's own CPU directly, and, when the detach removes the PMC's last target, wait for any references held by the target's other threads to drain while P_HWPMC is still set (they can no longer reload it). Reviewed by: adrian MFC after: 2 weeks Assisted-by: Claude Code (Fable 5) Differential Revision: https://reviews.freebsd.org/D58342 --- sys/dev/hwpmc/hwpmc_mod.c | 88 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/sys/dev/hwpmc/hwpmc_mod.c b/sys/dev/hwpmc/hwpmc_mod.c index 086a05657cfe..efc2aefec2b9 100644 --- a/sys/dev/hwpmc/hwpmc_mod.c +++ b/sys/dev/hwpmc/hwpmc_mod.c @@ -225,6 +225,8 @@ static int pmc_detach_one_process(struct proc *p, struct pmc *pm, static void pmc_destroy_owner_descriptor(struct pmc_owner *po); static void pmc_destroy_pmc_descriptor(struct pmc *pm); static void pmc_destroy_process_descriptor(struct pmc_process *pp); +static void pmc_reclaim_pmc_from_cpu(struct pmc *pm, + struct pmc_process *pp, int cpu); static struct pmc_owner *pmc_find_owner_descriptor(struct proc *p); static int pmc_find_pmc(pmc_id_t pmcid, struct pmc **pm); static struct pmc *pmc_find_pmc_descriptor_in_process(struct pmc_owner *po, @@ -1241,6 +1243,21 @@ pmc_detach_one_process(struct proc *p, struct pmc *pm, int flags) if (pp->pp_pmcs[ri].pp_pmc != pm) return (EINVAL); + /* + * If this is a process-virtual PMC that is still loaded on the + * hardware of the CPU we are running on (the common case when a + * process detaches a PMC from itself), take it off and drop its + * runcount reference now. The reference is otherwise only + * dropped by the switch-out reclaim, which the scheduler stops + * calling once P_HWPMC is cleared below - leaking it and later + * wedging pmc_wait_for_pmc_idle() at release time. + */ + if (PMC_IS_VIRTUAL_MODE(PMC_TO_MODE(pm))) { + critical_enter(); + pmc_reclaim_pmc_from_cpu(pm, pp, curthread->td_oncpu); + critical_exit(); + } + pmc_unlink_target_process(pm, pp); /* Issue a detach entry if a log file is configured */ @@ -1259,6 +1276,22 @@ pmc_detach_one_process(struct proc *p, struct pmc *pm, int flags) if (pp->pp_refcnt != 0) /* still a target of some PMC */ return (0); + /* + * This detach removed the process' last PMC and we are about to + * clear P_HWPMC. If the detached PMC was its last target and is + * still loaded on other CPUs (e.g. sibling threads of a + * multi-threaded target, or a target running on another CPU), + * drain those references first: the target is already unlinked so + * it cannot reload the PMC, and P_HWPMC is still set so those + * CPUs' switch-out reclaim still runs. Bounded by the target + * threads being scheduled out. + */ + if (PMC_IS_VIRTUAL_MODE(PMC_TO_MODE(pm)) && + LIST_EMPTY(&pm->pm_targets)) { + while (counter_u64_fetch(pm->pm_runcount) > 0) + pmc_force_context_switch(); + } + pmc_remove_process_descriptor(pp); if (flags & PMC_FLAG_REMOVE) @@ -1612,6 +1645,61 @@ pmc_delta(const struct pmc_classdep *pcd, pmc_value_t newvalue, return (delta); } +/* + * Take a process-virtual PMC off the hardware of 'cpu' if it is + * currently loaded there for process 'pp', accumulating its final + * count and dropping its runcount reference. This is the same reclaim + * that context switch out and process exit perform, factored out so it + * can also run when a target is detached while the PMC may still be + * live: the runcount reference is decremented by the switch-out reclaim + * only, which the scheduler gates on P_HWPMC, so a detach that clears + * P_HWPMC without draining would leak the reference and later wedge + * pmc_wait_for_pmc_idle(). Must be called in a critical section. + */ +static void +pmc_reclaim_pmc_from_cpu(struct pmc *pm, struct pmc_process *pp, int cpu) +{ + struct pmc_classdep *pcd; + struct pmc *phw_pm; + pmc_value_t newvalue, tmp; + u_int adjri, ri; + + ri = PMC_TO_ROWINDEX(pm); + pcd = pmc_ri_to_classdep(md, ri, &adjri); + + /* Only reclaim if this PMC is actually loaded on this CPU. */ + phw_pm = NULL; + (void)(*pcd->pcd_get_config)(cpu, adjri, &phw_pm); + if (phw_pm != pm) + return; + + KASSERT(counter_u64_fetch(pm->pm_runcount) > 0, + ("[pmc,%d] pm=%p runcount %ju", __LINE__, pm, + (uintmax_t)counter_u64_fetch(pm->pm_runcount))); + + if (pm->pm_pcpu_state[cpu].pps_cpustate) { + pm->pm_pcpu_state[cpu].pps_cpustate = 0; + if (pm->pm_pcpu_state[cpu].pps_stalled == 0) { + (void)pcd->pcd_stop_pmc(cpu, adjri, pm); + + if (PMC_TO_MODE(pm) == PMC_MODE_TC) { + (void)pcd->pcd_read_pmc(cpu, adjri, pm, + &newvalue); + tmp = pmc_delta(pcd, newvalue, + PMC_PCPU_SAVED(cpu, ri)); + + mtx_pool_lock_spin(pmc_mtxpool, pm); + pm->pm_gv.pm_savedvalue += tmp; + pp->pp_pmcs[ri].pp_pmcval += tmp; + mtx_pool_unlock_spin(pmc_mtxpool, pm); + } + } + } + + counter_u64_add(pm->pm_runcount, -1); + (void)pcd->pcd_config_pmc(cpu, adjri, NULL); +} + /* * Thread context switch OUT. */home | help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?6a5df78c.33323.733bf27>