linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ian Rogers <irogers@google.com>
To: Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@kernel.org>, Namhyung Kim <namhyung@kernel.org>,
	Maxime Coquelin <mcoquelin.stm32@gmail.com>,
	Alexandre Torgue <alexandre.torgue@foss.st.com>,
	Zhengjun Xing <zhengjun.xing@linux.intel.com>,
	Sandipan Das <sandipan.das@amd.com>,
	James Clark <james.clark@arm.com>,
	Kajol Jain <kjain@linux.ibm.com>,
	John Garry <john.g.garry@oracle.com>,
	Kan Liang <kan.liang@linux.intel.com>,
	Adrian Hunter <adrian.hunter@intel.com>,
	Andrii Nakryiko <andrii@kernel.org>,
	Eduard Zingerman <eddyz87@gmail.com>,
	Suzuki Poulouse <suzuki.poulose@arm.com>,
	Leo Yan <leo.yan@linaro.org>,
	Florian Fischer <florian.fischer@muhq.space>,
	Ravi Bangoria <ravi.bangoria@amd.com>,
	Jing Zhang <renyu.zj@linux.alibaba.com>,
	Sean Christopherson <seanjc@google.com>,
	Athira Rajeev <atrajeev@linux.vnet.ibm.com>,
	linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org,
	linux-stm32@st-md-mailman.stormreply.com,
	linux-arm-kernel@lists.infradead.org,
	Perry Taylor <perry.taylor@intel.com>,
	Caleb Biggers <caleb.biggers@intel.com>
Cc: Stephane Eranian <eranian@google.com>, Ian Rogers <irogers@google.com>
Subject: [PATCH v1 02/51] perf metrics: Improve variable names
Date: Sun, 19 Feb 2023 01:27:59 -0800	[thread overview]
Message-ID: <20230219092848.639226-3-irogers@google.com> (raw)
In-Reply-To: <20230219092848.639226-1-irogers@google.com>

has_constraint implies the NMI_WATCHDOG_CONSTRAINT and if the
constraint is detected it causes events not to be grouped. Most of the
code cares about whether events are grouped or not, so rename
has_constraint to group_events.

Also remove group from metricgroup___watchdog_constraint_hint as the
warning is specific to a metric. Make the warning message agree with
this too.

Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/metricgroup.c | 45 +++++++++++++++++------------------
 1 file changed, 22 insertions(+), 23 deletions(-)

diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index f3559be95541..b2aa6e049804 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -136,10 +136,9 @@ struct metric {
 	/** Optional null terminated array of referenced metrics. */
 	struct metric_ref *metric_refs;
 	/**
-	 * Is there a constraint on the group of events? In which case the
-	 * events won't be grouped.
+	 * Should events of the metric be grouped?
 	 */
-	bool has_constraint;
+	bool group_events;
 	/**
 	 * Parsed events for the metric. Optional as events may be taken from a
 	 * different metric whose group contains all the IDs necessary for this
@@ -148,12 +147,12 @@ struct metric {
 	struct evlist *evlist;
 };
 
-static void metricgroup___watchdog_constraint_hint(const char *name, bool foot)
+static void metric__watchdog_constraint_hint(const char *name, bool foot)
 {
 	static bool violate_nmi_constraint;
 
 	if (!foot) {
-		pr_warning("Splitting metric group %s into standalone metrics.\n", name);
+		pr_warning("Not grouping metric %s's events.\n", name);
 		violate_nmi_constraint = true;
 		return;
 	}
@@ -167,18 +166,18 @@ static void metricgroup___watchdog_constraint_hint(const char *name, bool foot)
 		   "    echo 1 > /proc/sys/kernel/nmi_watchdog\n");
 }
 
-static bool metricgroup__has_constraint(const struct pmu_metric *pm)
+static bool metric__group_events(const struct pmu_metric *pm)
 {
 	if (!pm->metric_constraint)
-		return false;
+		return true;
 
 	if (!strcmp(pm->metric_constraint, "NO_NMI_WATCHDOG") &&
 	    sysctl__nmi_watchdog_enabled()) {
-		metricgroup___watchdog_constraint_hint(pm->metric_name, false);
-		return true;
+		metric__watchdog_constraint_hint(pm->metric_name, /*foot=*/false);
+		return false;
 	}
 
-	return false;
+	return true;
 }
 
 static void metric__free(struct metric *m)
@@ -227,7 +226,7 @@ static struct metric *metric__new(const struct pmu_metric *pm,
 	}
 	m->pctx->sctx.runtime = runtime;
 	m->pctx->sctx.system_wide = system_wide;
-	m->has_constraint = metric_no_group || metricgroup__has_constraint(pm);
+	m->group_events = !metric_no_group && metric__group_events(pm);
 	m->metric_refs = NULL;
 	m->evlist = NULL;
 
@@ -637,7 +636,7 @@ static int decode_all_metric_ids(struct evlist *perf_evlist, const char *modifie
 static int metricgroup__build_event_string(struct strbuf *events,
 					   const struct expr_parse_ctx *ctx,
 					   const char *modifier,
-					   bool has_constraint)
+					   bool group_events)
 {
 	struct hashmap_entry *cur;
 	size_t bkt;
@@ -662,7 +661,7 @@ static int metricgroup__build_event_string(struct strbuf *events,
 		}
 		/* Separate events with commas and open the group if necessary. */
 		if (no_group) {
-			if (!has_constraint) {
+			if (group_events) {
 				ret = strbuf_addch(events, '{');
 				RETURN_IF_NON_ZERO(ret);
 			}
@@ -716,7 +715,7 @@ static int metricgroup__build_event_string(struct strbuf *events,
 			RETURN_IF_NON_ZERO(ret);
 		}
 	}
-	if (!no_group && !has_constraint) {
+	if (!no_group && group_events) {
 		ret = strbuf_addf(events, "}:W");
 		RETURN_IF_NON_ZERO(ret);
 	}
@@ -1252,7 +1251,7 @@ static int metricgroup__add_metric_list(const char *list, bool metric_no_group,
 		 * Warn about nmi_watchdog if any parsed metrics had the
 		 * NO_NMI_WATCHDOG constraint.
 		 */
-		metricgroup___watchdog_constraint_hint(NULL, true);
+		metric__watchdog_constraint_hint(NULL, /*foot=*/true);
 		/* No metrics. */
 		if (count == 0)
 			return -EINVAL;
@@ -1295,7 +1294,7 @@ static void find_tool_events(const struct list_head *metric_list,
 }
 
 /**
- * build_combined_expr_ctx - Make an expr_parse_ctx with all has_constraint
+ * build_combined_expr_ctx - Make an expr_parse_ctx with all !group_events
  *                           metric IDs, as the IDs are held in a set,
  *                           duplicates will be removed.
  * @metric_list: List to take metrics from.
@@ -1315,7 +1314,7 @@ static int build_combined_expr_ctx(const struct list_head *metric_list,
 		return -ENOMEM;
 
 	list_for_each_entry(m, metric_list, nd) {
-		if (m->has_constraint && !m->modifier) {
+		if (!m->group_events && !m->modifier) {
 			hashmap__for_each_entry(m->pctx->ids, cur, bkt) {
 				dup = strdup(cur->pkey);
 				if (!dup) {
@@ -1342,14 +1341,14 @@ static int build_combined_expr_ctx(const struct list_head *metric_list,
  * @fake_pmu: used when testing metrics not supported by the current CPU.
  * @ids: the event identifiers parsed from a metric.
  * @modifier: any modifiers added to the events.
- * @has_constraint: false if events should be placed in a weak group.
+ * @group_events: should events be placed in a weak group.
  * @tool_events: entries set true if the tool event of index could be present in
  *               the overall list of metrics.
  * @out_evlist: the created list of events.
  */
 static int parse_ids(bool metric_no_merge, struct perf_pmu *fake_pmu,
 		     struct expr_parse_ctx *ids, const char *modifier,
-		     bool has_constraint, const bool tool_events[PERF_TOOL_MAX],
+		     bool group_events, const bool tool_events[PERF_TOOL_MAX],
 		     struct evlist **out_evlist)
 {
 	struct parse_events_error parse_error;
@@ -1393,7 +1392,7 @@ static int parse_ids(bool metric_no_merge, struct perf_pmu *fake_pmu,
 		}
 	}
 	ret = metricgroup__build_event_string(&events, ids, modifier,
-					      has_constraint);
+					      group_events);
 	if (ret)
 		return ret;
 
@@ -1458,7 +1457,7 @@ static int parse_groups(struct evlist *perf_evlist, const char *str,
 		if (!ret && combined && hashmap__size(combined->ids)) {
 			ret = parse_ids(metric_no_merge, fake_pmu, combined,
 					/*modifier=*/NULL,
-					/*has_constraint=*/true,
+					/*group_events=*/false,
 					tool_events,
 					&combined_evlist);
 		}
@@ -1476,7 +1475,7 @@ static int parse_groups(struct evlist *perf_evlist, const char *str,
 		struct metric *n;
 		struct metric_expr *expr;
 
-		if (combined_evlist && m->has_constraint) {
+		if (combined_evlist && !m->group_events) {
 			metric_evlist = combined_evlist;
 		} else if (!metric_no_merge) {
 			/*
@@ -1507,7 +1506,7 @@ static int parse_groups(struct evlist *perf_evlist, const char *str,
 		}
 		if (!metric_evlist) {
 			ret = parse_ids(metric_no_merge, fake_pmu, m->pctx, m->modifier,
-					m->has_constraint, tool_events, &m->evlist);
+					m->group_events, tool_events, &m->evlist);
 			if (ret)
 				goto out;
 
-- 
2.39.2.637.g21b0678d19-goog


  parent reply	other threads:[~2023-02-19  9:29 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-02-19  9:27 [PATCH v1 00/51] shadow metric clean up and improvements Ian Rogers
2023-02-19  9:27 ` [PATCH v1 01/51] perf tools: Ensure evsel name is initialized Ian Rogers
2023-02-28 12:06   ` kajoljain
2023-02-19  9:27 ` Ian Rogers [this message]
2023-02-19  9:28 ` [PATCH v1 03/51] perf pmu-events: Remove aggr_mode from pmu_event Ian Rogers
2023-02-19  9:28 ` [PATCH v1 04/51] perf pmu-events: Change aggr_mode to be an enum Ian Rogers
2023-02-19  9:28 ` [PATCH v1 05/51] perf pmu-events: Change deprecated to be a bool Ian Rogers
2023-02-19  9:28 ` [PATCH v1 06/51] perf pmu-events: Change perpkg " Ian Rogers
2023-02-19  9:28 ` [PATCH v1 07/51] perf expr: Make the online topology accessible globally Ian Rogers
2023-02-19  9:28 ` [PATCH v1 08/51] perf pmu-events: Make the metric_constraint an enum Ian Rogers
2023-02-19  9:28 ` [PATCH v1 09/51] perf pmu-events: Don't '\0' terminate enum values Ian Rogers
2023-02-19  9:28 ` [PATCH v1 11/51] perf vendor events intel: Refresh alderlake-n metrics Ian Rogers
2023-02-19  9:28 ` [PATCH v1 16/51] perf vendor events intel: Add graniterapids events Ian Rogers
2023-02-19  9:28 ` [PATCH v1 24/51] perf vendor events intel: Refresh knightslanding events Ian Rogers
2023-02-19  9:28 ` [PATCH v1 25/51] perf vendor events intel: Refresh sandybridge events Ian Rogers
2023-02-19  9:28 ` [PATCH v1 27/51] perf vendor events intel: Refresh silvermont events Ian Rogers
2023-02-19  9:28 ` [PATCH v1 31/51] perf vendor events intel: Refresh westmereep-dp events Ian Rogers
2023-02-19  9:28 ` [PATCH v1 32/51] perf jevents: Add rand support to metrics Ian Rogers
2023-02-19  9:28 ` [PATCH v1 33/51] perf jevent: Parse metric thresholds Ian Rogers
2023-02-19  9:28 ` [PATCH v1 34/51] perf pmu-events: Test parsing metric thresholds with the fake PMU Ian Rogers
2023-02-19  9:28 ` [PATCH v1 35/51] perf list: Support for printing metric thresholds Ian Rogers
2023-02-19  9:28 ` [PATCH v1 36/51] perf metric: Compute and print threshold values Ian Rogers
2023-02-19  9:28 ` [PATCH v1 37/51] perf expr: More explicit NAN handling Ian Rogers
2023-02-19  9:28 ` [PATCH v1 38/51] perf metric: Add --metric-no-threshold option Ian Rogers
2023-02-19  9:28 ` [PATCH v1 39/51] perf stat: Add TopdownL1 metric as a default if present Ian Rogers
2023-02-27 19:12   ` Liang, Kan
2023-02-27 19:33     ` Ian Rogers
2023-02-27 20:12       ` Liang, Kan
2023-02-28  6:27         ` Ian Rogers
2023-02-28 14:15           ` Liang, Kan
2023-02-19  9:28 ` [PATCH v1 40/51] perf stat: Implement --topdown using json metrics Ian Rogers
2023-02-19  9:28 ` [PATCH v1 41/51] perf stat: Remove topdown event special handling Ian Rogers
2023-02-19  9:28 ` [PATCH v1 42/51] perf doc: Refresh topdown documentation Ian Rogers
2023-02-19  9:28 ` [PATCH v1 43/51] perf stat: Remove hard coded transaction events Ian Rogers
2023-02-19  9:28 ` [PATCH v1 44/51] perf stat: Use metrics for --smi-cost Ian Rogers
2023-02-19  9:28 ` [PATCH v1 45/51] perf stat: Remove perf_stat_evsel_id Ian Rogers
2023-02-19  9:28 ` [PATCH v1 46/51] perf stat: Move enums from header Ian Rogers
2023-02-19  9:28 ` [PATCH v1 47/51] perf stat: Hide runtime_stat Ian Rogers
2023-02-19  9:28 ` [PATCH v1 48/51] perf stat: Add cpu_aggr_map for loop Ian Rogers
2023-02-19  9:28 ` [PATCH v1 49/51] perf metric: Directly use counts rather than saved_value Ian Rogers
2023-02-19  9:28 ` [PATCH v1 50/51] perf stat: Use " Ian Rogers
2023-02-24 22:48   ` Namhyung Kim
2023-02-25  5:47     ` Ian Rogers
2023-02-19  9:28 ` [PATCH v1 51/51] perf stat: Remove saved_value/runtime_stat Ian Rogers
2023-02-19 11:17 ` [PATCH v1 00/51] shadow metric clean up and improvements Arnaldo Carvalho de Melo
2023-02-19 15:43   ` Ian Rogers
2023-02-21 17:44     ` Ian Rogers
2023-02-22 13:47       ` Arnaldo Carvalho de Melo
2023-02-27 22:04 ` Liang, Kan
2023-02-28  6:21   ` Ian Rogers

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230219092848.639226-3-irogers@google.com \
    --to=irogers@google.com \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=alexandre.torgue@foss.st.com \
    --cc=andrii@kernel.org \
    --cc=atrajeev@linux.vnet.ibm.com \
    --cc=caleb.biggers@intel.com \
    --cc=eddyz87@gmail.com \
    --cc=eranian@google.com \
    --cc=florian.fischer@muhq.space \
    --cc=james.clark@arm.com \
    --cc=john.g.garry@oracle.com \
    --cc=jolsa@kernel.org \
    --cc=kan.liang@linux.intel.com \
    --cc=kjain@linux.ibm.com \
    --cc=leo.yan@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=linux-stm32@st-md-mailman.stormreply.com \
    --cc=mark.rutland@arm.com \
    --cc=mcoquelin.stm32@gmail.com \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=perry.taylor@intel.com \
    --cc=peterz@infradead.org \
    --cc=ravi.bangoria@amd.com \
    --cc=renyu.zj@linux.alibaba.com \
    --cc=sandipan.das@amd.com \
    --cc=seanjc@google.com \
    --cc=suzuki.poulose@arm.com \
    --cc=zhengjun.xing@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).