The branch main has been updated by mhorne: URL: https://cgit.FreeBSD.org/src/commit/?id=104e8215cc7529a767b480a395ce97cf7a7037f8
commit 104e8215cc7529a767b480a395ce97cf7a7037f8 Author: Mitchell Horne <[email protected]> AuthorDate: 2023-06-14 16:33:46 +0000 Commit: Mitchell Horne <[email protected]> CommitDate: 2023-06-14 16:34:21 +0000 hwpmc: flatten conditional in pmc_process_exit() Use a goto to clarify the control flow when there is no process descriptor. This wins back a level of indentation. No functional change intended. Reviewed by: jkoshy MFC after: 2 weeks Sponsored by: The FreeBSD Foundation Differential Revision: https://reviews.freebsd.org/D40518 --- sys/dev/hwpmc/hwpmc_mod.c | 187 ++++++++++++++++++++++------------------------ 1 file changed, 88 insertions(+), 99 deletions(-) diff --git a/sys/dev/hwpmc/hwpmc_mod.c b/sys/dev/hwpmc/hwpmc_mod.c index b6e7baf1e7cd..781a2c116a40 100644 --- a/sys/dev/hwpmc/hwpmc_mod.c +++ b/sys/dev/hwpmc/hwpmc_mod.c @@ -4997,16 +4997,14 @@ pmc_process_exit(void *arg __unused, struct proc *p) p->p_comm); /* - * Since this code is invoked by the last thread in an exiting - * process, we would have context switched IN at some prior - * point. However, with PREEMPTION, kernel mode context - * switches may happen any time, so we want to disable a - * context switch OUT till we get any PMCs targeting this - * process off the hardware. + * Since this code is invoked by the last thread in an exiting process, + * we would have context switched IN at some prior point. However, with + * PREEMPTION, kernel mode context switches may happen any time, so we + * want to disable a context switch OUT till we get any PMCs targeting + * this process off the hardware. * - * We also need to atomically remove this process' - * entry from our target process hash table, using - * PMC_FLAG_REMOVE. + * We also need to atomically remove this process' entry from our + * target process hash table, using PMC_FLAG_REMOVE. */ PMCDBG3(PRC,EXT,1, "process-exit proc=%p (%d, %s)", p, p->p_pid, p->p_comm); @@ -5014,117 +5012,108 @@ pmc_process_exit(void *arg __unused, struct proc *p) critical_enter(); /* no preemption */ cpu = curthread->td_oncpu; - if ((pp = pmc_find_process_descriptor(p, PMC_FLAG_REMOVE)) != NULL) { - PMCDBG2(PRC,EXT,2, - "process-exit proc=%p pmc-process=%p", p, pp); + pp = pmc_find_process_descriptor(p, PMC_FLAG_REMOVE); + if (pp == NULL) { + critical_exit(); + goto out; + } + + PMCDBG2(PRC,EXT,2, "process-exit proc=%p pmc-process=%p", p, pp); + /* + * The exiting process could be the target of some PMCs which will be + * running on currently executing CPU. + * + * We need to turn these PMCs off like we would do at context switch + * OUT time. + */ + for (ri = 0; ri < md->pmd_npmc; ri++) { /* - * The exiting process could the target of - * some PMCs which will be running on - * currently executing CPU. - * - * We need to turn these PMCs off like we - * would do at context switch OUT time. + * Pick up the pmc pointer from hardware state similar to the + * CSW_OUT code. */ - for (ri = 0; ri < md->pmd_npmc; ri++) { - /* - * Pick up the pmc pointer from hardware - * state similar to the CSW_OUT code. - */ - pm = NULL; - - pcd = pmc_ri_to_classdep(md, ri, &adjri); + pm = NULL; + pcd = pmc_ri_to_classdep(md, ri, &adjri); - (void)(*pcd->pcd_get_config)(cpu, adjri, &pm); + (void)(*pcd->pcd_get_config)(cpu, adjri, &pm); - PMCDBG2(PRC,EXT,2, "ri=%d pm=%p", ri, pm); + PMCDBG2(PRC,EXT,2, "ri=%d pm=%p", ri, pm); - if (pm == NULL || !PMC_IS_VIRTUAL_MODE(PMC_TO_MODE(pm))) - continue; + if (pm == NULL || !PMC_IS_VIRTUAL_MODE(PMC_TO_MODE(pm))) + continue; - PMCDBG4(PRC,EXT,2, "ppmcs[%d]=%p pm=%p " - "state=%d", ri, pp->pp_pmcs[ri].pp_pmc, - pm, pm->pm_state); + PMCDBG4(PRC,EXT,2, "ppmcs[%d]=%p pm=%p state=%d", ri, + pp->pp_pmcs[ri].pp_pmc, pm, pm->pm_state); - KASSERT(PMC_TO_ROWINDEX(pm) == ri, - ("[pmc,%d] ri mismatch pmc(%d) ri(%d)", - __LINE__, PMC_TO_ROWINDEX(pm), ri)); - KASSERT(pm == pp->pp_pmcs[ri].pp_pmc, - ("[pmc,%d] pm %p != pp_pmcs[%d] %p", - __LINE__, pm, ri, pp->pp_pmcs[ri].pp_pmc)); - KASSERT(counter_u64_fetch(pm->pm_runcount) > 0, - ("[pmc,%d] bad runcount ri %d rc %ju", - __LINE__, ri, - (uintmax_t)counter_u64_fetch(pm->pm_runcount))); + KASSERT(PMC_TO_ROWINDEX(pm) == ri, + ("[pmc,%d] ri mismatch pmc(%d) ri(%d)", __LINE__, + PMC_TO_ROWINDEX(pm), ri)); + KASSERT(pm == pp->pp_pmcs[ri].pp_pmc, + ("[pmc,%d] pm %p != pp_pmcs[%d] %p", __LINE__, pm, ri, + pp->pp_pmcs[ri].pp_pmc)); + KASSERT(counter_u64_fetch(pm->pm_runcount) > 0, + ("[pmc,%d] bad runcount ri %d rc %ju", __LINE__, ri, + (uintmax_t)counter_u64_fetch(pm->pm_runcount))); - /* - * Change desired state, and then stop if not - * stalled. This two-step dance should avoid - * race conditions where an interrupt re-enables - * the PMC after this code has already checked - * the pm_stalled flag. - */ - if (pm->pm_pcpu_state[cpu].pps_cpustate) { - pm->pm_pcpu_state[cpu].pps_cpustate = 0; - if (!pm->pm_pcpu_state[cpu].pps_stalled) { - (void)pcd->pcd_stop_pmc(cpu, adjri, pm); - - if (PMC_TO_MODE(pm) == PMC_MODE_TC) { - pcd->pcd_read_pmc(cpu, adjri, - pm, &newvalue); - tmp = 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); - } + /* + * Change desired state, and then stop if not stalled. This + * two-step dance should avoid race conditions where an + * interrupt re-enables the PMC after this code has already + * checked the pm_stalled flag. + */ + if (pm->pm_pcpu_state[cpu].pps_cpustate) { + pm->pm_pcpu_state[cpu].pps_cpustate = 0; + if (!pm->pm_pcpu_state[cpu].pps_stalled) { + (void)pcd->pcd_stop_pmc(cpu, adjri, pm); + + if (PMC_TO_MODE(pm) == PMC_MODE_TC) { + pcd->pcd_read_pmc(cpu, adjri, pm, + &newvalue); + tmp = 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); } } + } - KASSERT(counter_u64_fetch(pm->pm_runcount) > 0, - ("[pmc,%d] runcount is %d", __LINE__, ri)); + KASSERT(counter_u64_fetch(pm->pm_runcount) > 0, + ("[pmc,%d] runcount is %d", __LINE__, ri)); - counter_u64_add(pm->pm_runcount, -1); - (void)pcd->pcd_config_pmc(cpu, adjri, NULL); - } + counter_u64_add(pm->pm_runcount, -1); + (void)pcd->pcd_config_pmc(cpu, adjri, NULL); + } - /* - * Inform the MD layer of this pseudo "context switch - * out" - */ - (void)md->pmd_switch_out(pmc_pcpu[cpu], pp); + /* + * Inform the MD layer of this pseudo "context switch out". + */ + (void)md->pmd_switch_out(pmc_pcpu[cpu], pp); - critical_exit(); /* ok to be pre-empted now */ + critical_exit(); /* ok to be pre-empted now */ - /* - * Unlink this process from the PMCs that are - * targeting it. This will send a signal to - * all PMC owner's whose PMCs are orphaned. - * - * Log PMC value at exit time if requested. - */ - for (ri = 0; ri < md->pmd_npmc; ri++) { - if ((pm = pp->pp_pmcs[ri].pp_pmc) != NULL) { - if ((pm->pm_flags & PMC_F_NEEDS_LOGFILE) != 0 && - PMC_IS_COUNTING_MODE(PMC_TO_MODE(pm))) { - pmclog_process_procexit(pm, pp); - } - pmc_unlink_target_process(pm, pp); + /* + * Unlink this process from the PMCs that are targeting it. This will + * send a signal to all PMC owner's whose PMCs are orphaned. + * + * Log PMC value at exit time if requested. + */ + for (ri = 0; ri < md->pmd_npmc; ri++) { + if ((pm = pp->pp_pmcs[ri].pp_pmc) != NULL) { + if ((pm->pm_flags & PMC_F_NEEDS_LOGFILE) != 0 && + PMC_IS_COUNTING_MODE(PMC_TO_MODE(pm))) { + pmclog_process_procexit(pm, pp); } + pmc_unlink_target_process(pm, pp); } - free(pp, M_PMC); - } else - critical_exit(); /* pp == NULL */ + } + free(pp, M_PMC); +out: /* - * If the process owned PMCs, free them up and free up - * memory. + * If the process owned PMCs, free them up and free up memory. */ if ((po = pmc_find_owner_descriptor(p)) != NULL) { pmc_remove_owner(po);
