linux-perf-users.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ian Rogers <irogers@google.com>
To: Andi Kleen <ak@linux.intel.com>, Jiri Olsa <jolsa@redhat.com>,
	Jin Yao <yao.jin@linux.intel.com>,
	Namhyung Kim <namhyung@kernel.org>,
	John Garry <john.garry@huawei.com>,
	Kajol Jain <kjain@linux.ibm.com>,
	"Paul A . Clarke" <pc@us.ibm.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Riccardo Mancini <rickyman7@gmail.com>,
	Kan Liang <kan.liang@linux.intel.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Kees Cook <keescook@chromium.org>,
	Sami Tolvanen <samitolvanen@google.com>,
	Nick Desaulniers <ndesaulniers@google.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Jacob Keller <jacob.e.keller@intel.com>,
	Zhen Lei <thunder.leizhen@huawei.com>,
	ToastC <mrtoastcheng@gmail.com>,
	Joakim Zhang <qiangqing.zhang@nxp.com>,
	Felix Fietkau <nbd@nbd.name>,
	Jiapeng Chong <jiapeng.chong@linux.alibaba.com>,
	Song Liu <songliubraving@fb.com>, Fabian Hemmer <copy@copy.sh>,
	Alexander Antonov <alexander.antonov@linux.intel.com>,
	Nicholas Fraser <nfraser@codeweavers.com>,
	Adrian Hunter <adrian.hunter@intel.com>,
	Denys Zagorui <dzagorui@cisco.com>,
	Wan Jiabing <wanjiabing@vivo.com>,
	Thomas Richter <tmricht@linux.ibm.com>,
	Sumanth Korikkar <sumanthk@linux.ibm.com>,
	Heiko Carstens <hca@linux.ibm.com>,
	Changbin Du <changbin.du@intel.com>,
	linux-kernel@vger.kernel.org, linux-perf-users@vger.kernel.org
Cc: Stephane Eranian <eranian@google.com>, Ian Rogers <irogers@google.com>
Subject: [PATCH 07/21] perf metric: Add metric new and free
Date: Thu,  7 Oct 2021 09:56:33 -0700	[thread overview]
Message-ID: <20211007165647.3514803-8-irogers@google.com> (raw)
In-Reply-To: <20211007165647.3514803-1-irogers@google.com>

Metrics are complex enough that a new/free reduces the risk of memory
leaks. Move static functions used in new.

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

diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index 3e5f02938452..e4ce19389258 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -127,6 +127,78 @@ struct metric {
 	bool has_constraint;
 };
 
+static void metricgroup___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);
+		violate_nmi_constraint = true;
+		return;
+	}
+
+	if (!violate_nmi_constraint)
+		return;
+
+	pr_warning("Try disabling the NMI watchdog to comply NO_NMI_WATCHDOG metric constraint:\n"
+		   "    echo 0 > /proc/sys/kernel/nmi_watchdog\n"
+		   "    perf stat ...\n"
+		   "    echo 1 > /proc/sys/kernel/nmi_watchdog\n");
+}
+
+static bool metricgroup__has_constraint(const struct pmu_event *pe)
+{
+	if (!pe->metric_constraint)
+		return false;
+
+	if (!strcmp(pe->metric_constraint, "NO_NMI_WATCHDOG") &&
+	    sysctl__nmi_watchdog_enabled()) {
+		metricgroup___watchdog_constraint_hint(pe->metric_name, false);
+		return true;
+	}
+
+	return false;
+}
+
+static struct metric *metric__new(const struct pmu_event *pe,
+				  bool metric_no_group,
+				  int runtime)
+{
+	struct metric *m;
+
+	m = zalloc(sizeof(*m));
+	if (!m)
+		return NULL;
+
+	m->pctx = expr__ctx_new();
+	if (!m->pctx) {
+		free(m);
+		return NULL;
+	}
+
+	m->metric_name = pe->metric_name;
+	m->metric_expr = pe->metric_expr;
+	m->metric_unit = pe->unit;
+	m->pctx->runtime = runtime;
+	m->has_constraint = metric_no_group || metricgroup__has_constraint(pe);
+	INIT_LIST_HEAD(&m->metric_refs);
+	m->metric_refs_cnt = 0;
+
+	return m;
+}
+
+static void metric__free(struct metric *m)
+{
+	struct metric_ref_node *ref, *tmp;
+
+	list_for_each_entry_safe(ref, tmp, &m->metric_refs, list) {
+		list_del(&ref->list);
+		free(ref);
+	}
+	expr__ctx_free(m->pctx);
+	free(m);
+}
+
 #define RECURSION_ID_MAX 1000
 
 struct expr_ids {
@@ -736,39 +808,6 @@ static void metricgroup__add_metric_non_group(struct strbuf *events,
 	}
 }
 
-static void metricgroup___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);
-		violate_nmi_constraint = true;
-		return;
-	}
-
-	if (!violate_nmi_constraint)
-		return;
-
-	pr_warning("Try disabling the NMI watchdog to comply NO_NMI_WATCHDOG metric constraint:\n"
-		   "    echo 0 > /proc/sys/kernel/nmi_watchdog\n"
-		   "    perf stat ...\n"
-		   "    echo 1 > /proc/sys/kernel/nmi_watchdog\n");
-}
-
-static bool metricgroup__has_constraint(const struct pmu_event *pe)
-{
-	if (!pe->metric_constraint)
-		return false;
-
-	if (!strcmp(pe->metric_constraint, "NO_NMI_WATCHDOG") &&
-	    sysctl__nmi_watchdog_enabled()) {
-		metricgroup___watchdog_constraint_hint(pe->metric_name, false);
-		return true;
-	}
-
-	return false;
-}
-
 int __weak arch_get_runtimeparam(const struct pmu_event *pe __maybe_unused)
 {
 	return 1;
@@ -813,23 +852,10 @@ static int __add_metric(struct list_head *metric_list,
 		 * We got in here for the parent group,
 		 * allocate it and put it on the list.
 		 */
-		m = zalloc(sizeof(*m));
+		m = metric__new(pe, metric_no_group, runtime);
 		if (!m)
 			return -ENOMEM;
 
-		m->pctx = expr__ctx_new();
-		if (!m->pctx) {
-			free(m);
-			return -ENOMEM;
-		}
-		m->metric_name = pe->metric_name;
-		m->metric_expr = pe->metric_expr;
-		m->metric_unit = pe->unit;
-		m->pctx->runtime = runtime;
-		m->has_constraint = metric_no_group || metricgroup__has_constraint(pe);
-		INIT_LIST_HEAD(&m->metric_refs);
-		m->metric_refs_cnt = 0;
-
 		parent = expr_ids__alloc(ids);
 		if (!parent) {
 			free(m);
@@ -877,8 +903,7 @@ static int __add_metric(struct list_head *metric_list,
 	 */
 	if (expr__find_ids(pe->metric_expr, NULL, m->pctx) < 0) {
 		if (m->metric_refs_cnt == 0) {
-			expr__ctx_free(m->pctx);
-			free(m);
+			metric__free(m);
 			*mp = NULL;
 		}
 		return -EINVAL;
@@ -1251,25 +1276,13 @@ static int metricgroup__add_metric_list(const char *list, bool metric_no_group,
 	return ret;
 }
 
-static void metric__free_refs(struct metric *metric)
-{
-	struct metric_ref_node *ref, *tmp;
-
-	list_for_each_entry_safe(ref, tmp, &metric->metric_refs, list) {
-		list_del(&ref->list);
-		free(ref);
-	}
-}
-
 static void metricgroup__free_metrics(struct list_head *metric_list)
 {
 	struct metric *m, *tmp;
 
 	list_for_each_entry_safe (m, tmp, metric_list, nd) {
-		metric__free_refs(m);
-		expr__ctx_free(m->pctx);
 		list_del_init(&m->nd);
-		free(m);
+		metric__free(m);
 	}
 }
 
-- 
2.33.0.882.g93a45727a2-goog


  parent reply	other threads:[~2021-10-07 16:58 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-07 16:56 [PATCH 00/21] perf metric: Fixes and allow modifiers Ian Rogers
2021-10-07 16:56 ` [PATCH 01/21] tools lib: Add list_sort Ian Rogers
2021-10-07 23:54   ` Andi Kleen
2021-10-08 20:21   ` Arnaldo Carvalho de Melo
2021-10-07 16:56 ` [PATCH 02/21] perf pmu: Add const to pmu_events_map Ian Rogers
2021-10-08 11:01   ` John Garry
2021-10-08 15:22     ` Andrew Kilroy
2021-10-07 16:56 ` [PATCH 03/21] perf pmu: Make pmu_sys_event_tables const Ian Rogers
2021-10-08 11:01   ` John Garry
2021-10-07 16:56 ` [PATCH 04/21] perf pmu: Make pmu_event tables const Ian Rogers
2021-10-08 11:07   ` John Garry
2021-10-07 16:56 ` [PATCH 05/21] perf metric: Move runtime value to the expr context Ian Rogers
2021-10-07 16:56 ` [PATCH 06/21] perf metric: Add documentation and rename a variable Ian Rogers
2021-10-07 16:56 ` Ian Rogers [this message]
2021-10-07 16:56 ` [PATCH 08/21] perf metric: Only add a referenced metric once Ian Rogers
2021-10-07 16:56 ` [PATCH 09/21] perf metric: Modify resolution and recursion check Ian Rogers
2021-10-07 16:56 ` [PATCH 10/21] perf metric: Comment data structures Ian Rogers
2021-10-07 16:56 ` [PATCH 11/21] perf metric: Document the internal 'struct metric' Ian Rogers
2021-10-07 16:56 ` [PATCH 12/21] perf metric: Simplify metric_refs calculation Ian Rogers
2021-10-07 16:56 ` [PATCH 13/21] perf parse-events: Add const to evsel name Ian Rogers
2021-10-07 16:56 ` [PATCH 14/21] perf parse-events: Add new "metric-id" term Ian Rogers
2021-10-07 16:56 ` [PATCH 15/21] perf parse-events: Allow config on kernel PMU events Ian Rogers
2021-10-07 16:56 ` [PATCH 16/21] perf metric: Encode and use metric-id as qualifier Ian Rogers
2021-10-07 16:56 ` [PATCH 17/21] perf expr: Add subset utility Ian Rogers
2021-10-07 16:56 ` [PATCH 18/21] perf metrics: Modify setup and deduplication Ian Rogers
2021-10-07 16:56 ` [PATCH 19/21] perf metric: Switch fprintf to pr_err Ian Rogers
2021-10-07 16:56 ` [PATCH 20/21] perf parse-events: Identify broken modifiers Ian Rogers
2021-10-07 16:56 ` [PATCH 21/21] perf metric: Allow modifiers on metrics Ian Rogers
2021-10-07 23:59 ` [PATCH 00/21] perf metric: Fixes and allow modifiers Andi Kleen

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=20211007165647.3514803-8-irogers@google.com \
    --to=irogers@google.com \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=ak@linux.intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=alexander.antonov@linux.intel.com \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=changbin.du@intel.com \
    --cc=copy@copy.sh \
    --cc=dzagorui@cisco.com \
    --cc=eranian@google.com \
    --cc=hca@linux.ibm.com \
    --cc=jacob.e.keller@intel.com \
    --cc=jiapeng.chong@linux.alibaba.com \
    --cc=john.garry@huawei.com \
    --cc=jolsa@redhat.com \
    --cc=kan.liang@linux.intel.com \
    --cc=keescook@chromium.org \
    --cc=kjain@linux.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@redhat.com \
    --cc=mrtoastcheng@gmail.com \
    --cc=namhyung@kernel.org \
    --cc=nbd@nbd.name \
    --cc=ndesaulniers@google.com \
    --cc=nfraser@codeweavers.com \
    --cc=pc@us.ibm.com \
    --cc=peterz@infradead.org \
    --cc=qiangqing.zhang@nxp.com \
    --cc=rickyman7@gmail.com \
    --cc=samitolvanen@google.com \
    --cc=songliubraving@fb.com \
    --cc=sumanthk@linux.ibm.com \
    --cc=thunder.leizhen@huawei.com \
    --cc=tmricht@linux.ibm.com \
    --cc=wanjiabing@vivo.com \
    --cc=yao.jin@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).