linux-kernel - Re: [PATCH 01/10] perf tools: Add fallback for exclude

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f171c2e5-3468-4cdb-b369-87e5aeb6660b@linux.intel.com>
Date: Fri, 6 Sep 2024 09:47:53 -0400
From: "Liang, Kan" <kan.liang@...ux.intel.com>
To: Namhyung Kim <namhyung@...nel.org>,
 Arnaldo Carvalho de Melo <acme@...nel.org>, Ian Rogers <irogers@...gle.com>
Cc: Jiri Olsa <jolsa@...nel.org>, Adrian Hunter <adrian.hunter@...el.com>,
 Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...nel.org>,
 LKML <linux-kernel@...r.kernel.org>, linux-perf-users@...r.kernel.org,
 Ravi Bangoria <ravi.bangoria@....com>, Mark Rutland <mark.rutland@....com>,
 James Clark <james.clark@....com>, Kajol Jain <kjain@...ux.ibm.com>,
 Thomas Richter <tmricht@...ux.ibm.com>, Atish Patra <atishp@...shpatra.org>,
 Palmer Dabbelt <palmer@...osinc.com>, Mingwei Zhang <mizhang@...gle.com>,
 James Clark <james.clark@...aro.org>
Subject: Re: [PATCH 01/10] perf tools: Add fallback for exclude_guest



On 2024-09-05 4:24 p.m., Namhyung Kim wrote:
> Commit 7b100989b4f6bce70 ("perf evlist: Remove __evlist__add_default")
> changed to parse "cycles:P" event instead of creating a new cycles
> event for perf record.  But it also changed the way how modifiers are
> handled so it doesn't set the exclude_guest bit by default.
> 
> It seems Apple M1 PMU requires exclude_guest set and returns EOPNOTSUPP
> if not.  Let's add a fallback so that it can work with default events.
> 
> Fixes: 7b100989b4f6bce70 ("perf evlist: Remove __evlist__add_default")
> Cc: Mark Rutland <mark.rutland@....com>
> Cc: James Clark <james.clark@...aro.org>
> Signed-off-by: Namhyung Kim <namhyung@...nel.org>
> ---
>  tools/perf/builtin-stat.c |  3 +--
>  tools/perf/util/evsel.c   | 21 +++++++++++++++++++++
>  2 files changed, 22 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
> index cf985cdb9a6ee588..d8315dae930184ba 100644
> --- a/tools/perf/builtin-stat.c
> +++ b/tools/perf/builtin-stat.c
> @@ -639,8 +639,7 @@ static enum counter_recovery stat_handle_error(struct evsel *counter)
>  	 * (behavior changed with commit b0a873e).
>  	 */
>  	if (errno == EINVAL || errno == ENOSYS ||
> -	    errno == ENOENT || errno == EOPNOTSUPP ||
> -	    errno == ENXIO) {
> +	    errno == ENOENT || errno == ENXIO) {
>  		if (verbose > 0)
>  			ui__warning("%s event is not supported by the kernel.\n",
>  				    evsel__name(counter));

It seems the behavior for other reasons which trigger the 'EOPNOTSUPP'
is changed as well.
At least, it looks like we don't skip the member event with EOPNOTSUPP
anymore.

I'm not sure if it's a big deal. But I think we'd better mention it in
the change log or the comments.

Thanks,
Kan

> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index 49cc71511c0c8ce8..d59ad76b28758906 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -3244,6 +3244,27 @@ bool evsel__fallback(struct evsel *evsel, struct target *target, int err,
>  		evsel->core.attr.exclude_kernel = 1;
>  		evsel->core.attr.exclude_hv     = 1;
>  
> +		return true;
> +	} else if (err == EOPNOTSUPP && !evsel->core.attr.exclude_guest &&
> +		   !evsel->exclude_GH) {
> +		const char *name = evsel__name(evsel);
> +		char *new_name;
> +		const char *sep = ":";
> +
> +		/* Is there already the separator in the name. */
> +		if (strchr(name, '/') ||
> +		    (strchr(name, ':') && !evsel->is_libpfm_event))
> +			sep = "";
> +
> +		if (asprintf(&new_name, "%s%sH", name, sep) < 0)
> +			return false;
> +
> +		free(evsel->name);
> +		evsel->name = new_name;
> +		/* Apple M1 requires exclude_guest */
> +		scnprintf(msg, msgsize, "trying to fall back to excluding guest samples");
> +		evsel->core.attr.exclude_guest = 1;
> +
>  		return true;
>  	}
>