From nobody Wed Jun 14 16:46:46 2023 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4QhBCW2xD1z4dJ0m; Wed, 14 Jun 2023 16:46:47 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4QhBCW1qy3z4Q0L; Wed, 14 Jun 2023 16:46:47 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1686761207; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=f+ct7X5wEZNHrXpyBFGQ8ukjccfqM+30lcpM6cQi9ZA=; b=sVtTtO/UM8g38l+28bpasq1P4KM4zTOwohAkCqQdeXdlg9r8XP53TmaGkVv/VVGSAnloFE hen8fU2stWycb9t3kvnwvY7flieQm9n2v++xLSR79yt1m1gmjVFdSCn3bkY/kqjabdN5FG XckrEg3cM/JVQouGti+g5ktu9P5jqvmqeHoQMEpHLX/8WhZD4K1ju+ENJ/n6gn7v52oLYJ jXSxt3Pe9Dh8ttl7WSxS5oAIkfsb5axWoTUf/ooy+Q5pHcjstgUQoQH0PcEGXd8ql9b005 aprGONE8Ccr1yQGeCajc3JDV1dh+hUBHmYsMXUguZ4mjPhDVdaqNF6q7bcab7g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1686761207; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=f+ct7X5wEZNHrXpyBFGQ8ukjccfqM+30lcpM6cQi9ZA=; b=BpYnhU3IYzS402LwW3WJDKC8jd1k2pkrbtzSnjliaxDzfiFh1iU9GtwzDGfhDrakC2ZwV6 GV4NPCoDoQtZOH1O5+yfar/Gv/zeQuIRbhsUNedCVtsrjJ3vqUqVRBylwUFo+8+DWF9tat gHtmsseBro+WeWfSo17ny33YsVbrv3/HEQZaErzG9MoF3uIR828ixhGMfVDc3Q9BL7YWzm UcOOr1UUsZ4p57cn8iS3E7MVM1U0MPYOrh+0WtZdHM78wd+QsfJNl25sYVIuc5PD44Mack IviYZ3Sr1Qfc4gGl1/X/U+/0GfbCQNXgQPMA3OYO5Xls9niStySmr/m1+YVBpA== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1686761207; a=rsa-sha256; cv=none; b=urmT9hhZXawIhO+WlzrOKT6AEAcRq0c0sY+SAZksJuVhjtaQVUtfYmFCJLDnfY6lpzfo3c jhRbyiR0Mpp1zDVjY7jq1Zi41qoc3l1WdkD5EMyOiC9LWo8QOD6GXPkKsKuaPIgY7Th7GA 5MuGQ5A4aV2Rj89RfZ+pmvgSvmdRhYMSsX5SCM5a0Yqg13ElkXX+F/3cAqmLU6+3FgiDeV qw4aX4YP0OUw62GZcP/WFGyY878olBxs3jpacRBlmJ7v7dNZHt+ZvYuHvWe6dIk77wQQPI QBL3Vk8lrzvwQPCXh4OpAao+0M+1DW068UWJn4rSixtllBH+x0HG1L5rxbnMzA== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4QhBCW0hM6z19wQ; Wed, 14 Jun 2023 16:46:47 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 35EGkl10028522; Wed, 14 Jun 2023 16:46:47 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 35EGkkX6028521; Wed, 14 Jun 2023 16:46:46 GMT (envelope-from git) Date: Wed, 14 Jun 2023 16:46:46 GMT Message-Id: <202306141646.35EGkkX6028521@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Mitchell Horne Subject: git: 104e8215cc75 - main - hwpmc: flatten conditional in pmc_process_exit() List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: mhorne X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 104e8215cc7529a767b480a395ce97cf7a7037f8 Auto-Submitted: auto-generated X-ThisMailContainsUnwantedMimeParts: N The branch main has been updated by mhorne: URL: https://cgit.FreeBSD.org/src/commit/?id=104e8215cc7529a767b480a395ce97cf7a7037f8 commit 104e8215cc7529a767b480a395ce97cf7a7037f8 Author: Mitchell Horne AuthorDate: 2023-06-14 16:33:46 +0000 Commit: Mitchell Horne 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);