[PATCH v1 1/2] perf metrics: Fix metric matching

[PATCH v1 1/2] perf metrics: Fix metric matching

[Ian Rogers]

The metric match function fails for cases like looking for "metric" in
the string "all;foo_metric;metric" as the "metric" in "foo_metric"
matches but isn't preceeded by a ';'. Fix this by matching the first
list item and recursively matching on failure the next item after a
semicolon.

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

diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index b24a1c177a80..2d6865c392ef 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -352,25 +352,23 @@ static int setup_metric_events(const char *pmu, struct hashmap *ids,
 	return 0;
 }
 
-static bool match_metric(const char *n, const char *list)
+static bool match_metric(const char *metric_or_groups, const char *sought)
 {
 	int len;
 	char *m;
 
-	if (!list)
+	if (!sought)
 		return false;
-	if (!strcmp(list, "all"))
+	if (!strcmp(sought, "all"))
 		return true;
-	if (!n)
-		return !strcasecmp(list, "No_group");
-	len = strlen(list);
-	m = strcasestr(n, list);
-	if (!m)
-		return false;
-	if ((m == n || m[-1] == ';' || m[-1] == ' ') &&
-	    (m[len] == 0 || m[len] == ';'))
+	if (!metric_or_groups)
+		return !strcasecmp(sought, "No_group");
+	len = strlen(sought);
+	if (!strncasecmp(metric_or_groups, sought, len) &&
+	    (metric_or_groups[len] == 0 || metric_or_groups[len] == ';'))
 		return true;
-	return false;
+	m = strchr(metric_or_groups, ';');
+	return m && match_metric(m + 1, sought);
 }
 
 static bool match_pm_metric(const struct pmu_metric *pm, const char *pmu, const char *metric)
-- 
2.44.0.rc0.258.g7320e95886-goog

[PATCH v1 2/2] perf metrics: Fix segv for metrics with no events

[Ian Rogers]

A metric may have no events, for example, the transaction metrics on
x86 are dependent on there being TSX events. Fix a segv where an evsel
of NULL is dereferenced for a metric leader value.

Fixes: a59fb796a36b ("perf metrics: Compute unmerged uncore metrics individually")
Signed-off-by: Ian Rogers <irogers@google.com>
---
 tools/perf/util/metricgroup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
index 2d6865c392ef..79ef6095ab28 100644
--- a/tools/perf/util/metricgroup.c
+++ b/tools/perf/util/metricgroup.c
@@ -44,7 +44,7 @@ struct metric_event *metricgroup__lookup(struct rblist *metric_events,
 	if (!metric_events)
 		return NULL;
 
-	if (evsel->metric_leader)
+	if (evsel && evsel->metric_leader)
 		me.evsel = evsel->metric_leader;
 	nd = rblist__find(metric_events, &me);
 	if (nd)
-- 
2.44.0.rc0.258.g7320e95886-goog

Re: [PATCH v1 1/2] perf metrics: Fix metric matching

[Liang, Kan]

On 2024-02-23 8:14 p.m., Ian Rogers wrote:
> The metric match function fails for cases like looking for "metric" in
> the string "all;foo_metric;metric" as the "metric" in "foo_metric"
> matches but isn't preceeded by a ';'. Fix this by matching the first
> list item and recursively matching on failure the next item after a
> semicolon.
> 
> Signed-off-by: Ian Rogers <irogers@google.com>

For the series,

Reviewed-by: Kan Liang <kan.liang@linux.intel.com>

Thanks,
Kan

> ---
>  tools/perf/util/metricgroup.c | 22 ++++++++++------------
>  1 file changed, 10 insertions(+), 12 deletions(-)
> 
> diff --git a/tools/perf/util/metricgroup.c b/tools/perf/util/metricgroup.c
> index b24a1c177a80..2d6865c392ef 100644
> --- a/tools/perf/util/metricgroup.c
> +++ b/tools/perf/util/metricgroup.c
> @@ -352,25 +352,23 @@ static int setup_metric_events(const char *pmu, struct hashmap *ids,
>  	return 0;
>  }
>  
> -static bool match_metric(const char *n, const char *list)
> +static bool match_metric(const char *metric_or_groups, const char *sought)
>  {
>  	int len;
>  	char *m;
>  
> -	if (!list)
> +	if (!sought)
>  		return false;
> -	if (!strcmp(list, "all"))
> +	if (!strcmp(sought, "all"))
>  		return true;
> -	if (!n)
> -		return !strcasecmp(list, "No_group");
> -	len = strlen(list);
> -	m = strcasestr(n, list);
> -	if (!m)
> -		return false;
> -	if ((m == n || m[-1] == ';' || m[-1] == ' ') &&
> -	    (m[len] == 0 || m[len] == ';'))
> +	if (!metric_or_groups)
> +		return !strcasecmp(sought, "No_group");
> +	len = strlen(sought);
> +	if (!strncasecmp(metric_or_groups, sought, len) &&
> +	    (metric_or_groups[len] == 0 || metric_or_groups[len] == ';'))
>  		return true;
> -	return false;
> +	m = strchr(metric_or_groups, ';');
> +	return m && match_metric(m + 1, sought);
>  }
>  
>  static bool match_pm_metric(const struct pmu_metric *pm, const char *pmu, const char *metric)

Re: [PATCH v1 1/2] perf metrics: Fix metric matching

[Namhyung Kim]

On Fri, 23 Feb 2024 17:14:19 -0800, Ian Rogers wrote:
> The metric match function fails for cases like looking for "metric" in
> the string "all;foo_metric;metric" as the "metric" in "foo_metric"
> matches but isn't preceeded by a ';'. Fix this by matching the first
> list item and recursively matching on failure the next item after a
> semicolon.
> 
> 
> [...]

Applied to perf-tools-next, thanks!

Best regards,
-- 
Namhyung Kim <namhyung@kernel.org>