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

[Jiri Olsa]

On Thu, Dec 21, 2017 at 10:08:52AM -0800, kan.li...@intel.com wrote:
> From: Kan Liang <kan.li...@intel.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.li...@intel.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) {