linux-kernel - Re: [PATCH V3 10/12] perf top: add overwrite fall back

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:   Thu, 11 Jan 2018 15:26:26 +0100
From:   Jiri Olsa <jolsa@...hat.com>
To:     kan.liang@...el.com
Cc:     acme@...nel.org, peterz@...radead.org, mingo@...hat.com,
        linux-kernel@...r.kernel.org, wangnan0@...wei.com,
        jolsa@...nel.org, namhyung@...nel.org, ak@...ux.intel.com,
        yao.jin@...ux.intel.com
Subject: Re: [PATCH V3 10/12] perf top: add overwrite fall back

On Thu, Dec 21, 2017 at 10:08:52AM -0800, kan.liang@...el.com wrote:
> From: Kan Liang <kan.liang@...el.com>
> 
> Switch to non-overwrite mode if kernel doesnot support overwrite
> ringbuffer.
> 
> It's only effect when overwrite mode is supported.
> No change to current behavior.
> 
> Signed-off-by: Kan Liang <kan.liang@...el.com>
> ---
>  tools/perf/builtin-top.c | 36 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 36 insertions(+)
> 
> diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> index 4b85e7b..8d19ef7 100644
> --- a/tools/perf/builtin-top.c
> +++ b/tools/perf/builtin-top.c
> @@ -931,6 +931,27 @@ static int perf_top_overwrite_check(struct perf_top *top)
>  	return 0;
>  }
>  
> +static int perf_top_overwrite_fallback(struct perf_top *top,
> +				       struct perf_evsel *evsel)
> +{
> +	struct record_opts *opts = &top->record_opts;
> +	struct perf_evlist *evlist = top->evlist;
> +	struct perf_evsel *counter;
> +
> +	if (!opts->overwrite)
> +		return 0;
> +
> +	/* only fall back when first event fails */
> +	if (evsel != perf_evlist__first(evlist))
> +		return 0;
> +
> +	evlist__for_each_entry(evlist, counter)
> +		counter->attr.write_backward = false;
> +	opts->overwrite = false;
> +	ui__warning("fall back to non-overwrite mode\n");
> +	return 1;
> +}

we already do that for evsel in perf_evsel__open.. could we make
this fallback on one place only?

checking the code, why don't we follow the perf_missing_features.write_backward
like we do for other features in the perf_evsel__open and set
write_backward accordingly, like in attached patch.

jirka


---
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index dbd80268de54..89253f4ff9ee 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1683,9 +1683,6 @@ int perf_evsel__open(struct perf_evsel *evsel, struct cpu_map *cpus,
 	int pid = -1, err;
 	enum { NO_CHANGE, SET_TO_MAX, INCREASED_MAX } set_rlimit = NO_CHANGE;
 
-	if (perf_missing_features.write_backward && evsel->attr.write_backward)
-		return -EINVAL;
-
 	if (cpus == NULL) {
 		static struct cpu_map *empty_cpu_map;
 
@@ -1725,6 +1722,8 @@ int perf_evsel__open(struct perf_evsel *evsel, struct cpu_map *cpus,
 	}
 
 fallback_missing_features:
+	if (perf_missing_features.write_backward)
+		evsel->attr.write_backward = false;
 	if (perf_missing_features.clockid_wrong)
 		evsel->attr.clockid = CLOCK_MONOTONIC; /* should always work */
 	if (perf_missing_features.clockid) {