Em Fri, Jul 01, 2016 at 02:23:44AM +0000, Wang Nan escreveu:
> From: Arnaldo Carvalho de Melo <[email protected]>
> 
> 'perf test backward' FAILED on old kernel:
> 
>  [root@jouet ~]# perf test -v backward
>  45: Test backward reading from ring buffer                   :
>  --- start ---
>  <SNIP>
>  mmap size 1052672B
>  Unexpected counter: sample_count=0, comm_count=0
>  ---- end ----
>  Test backward reading from ring buffer: FAILED!
>  [root@jouet ~]#
> 
> Reason: when kernel doesn't support 'attr.backward', perf_evsel__open() will
> trim this bit and try again, unless evsel->overwrite is true.
> 
> Set evsel->overwrite in this test.

This fixes it, but then, if we get attr.backward set and the kernel
doesn't support it, what whould be the point of trying without it? I.e.
its not like we can do what is being asked for by fallbacking to some
other method.

I.e. perhaps evsel->overwrite is completely unnecessary and places
checking for it should instead check for evsel->attr.backward, no?

Something like the patch below, with it I also get:

[root@jouet perf]# perf test backward
45: Test backward reading from ring buffer                   : Skip
[root@jouet perf]# perf test -v backward 2>&1 | grep -v regist
45: Test backward reading from ring buffer                   :
--- start ---
test child forked, pid 29997
perf_evlist__open: Invalid argument
test child finished with -2
---- end ----
Test backward reading from ring buffer: Skip
[root@jouet perf]# 

- Arnaldo

diff --git a/tools/perf/util/evlist.c b/tools/perf/util/evlist.c
index 113507716044..060c3b3d14db 100644
--- a/tools/perf/util/evlist.c
+++ b/tools/perf/util/evlist.c
@@ -1003,7 +1003,7 @@ static bool
 perf_evlist__should_poll(struct perf_evlist *evlist __maybe_unused,
                         struct perf_evsel *evsel)
 {
-       if (evsel->overwrite)
+       if (evsel->attr.write_backward)
                return false;
        return true;
 }
@@ -1018,7 +1018,7 @@ static int perf_evlist__mmap_per_evsel(struct perf_evlist 
*evlist, int idx,
        evlist__for_each_entry(evlist, evsel) {
                int fd;
 
-               if (evsel->overwrite != (evlist->overwrite && evlist->backward))
+               if (evsel->attr.write_backward != (evlist->overwrite && 
evlist->backward))
                        continue;
 
                if (evsel->system_wide && thread)
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
index 0fea724e735c..3abe5195fe80 100644
--- a/tools/perf/util/evsel.c
+++ b/tools/perf/util/evsel.c
@@ -1359,6 +1359,9 @@ static 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 (evsel->system_wide)
                nthreads = 1;
        else
@@ -1389,11 +1392,6 @@ fallback_missing_features:
        if (perf_missing_features.lbr_flags)
                evsel->attr.branch_sample_type &= ~(PERF_SAMPLE_BRANCH_NO_FLAGS 
|
                                     PERF_SAMPLE_BRANCH_NO_CYCLES);
-       if (perf_missing_features.write_backward) {
-               if (evsel->overwrite)
-                       return -EINVAL;
-               evsel->attr.write_backward = false;
-       }
 retry_sample_id:
        if (perf_missing_features.sample_id_all)
                evsel->attr.sample_id_all = 0;
@@ -1495,7 +1493,7 @@ try_fallback:
         */
        if (!perf_missing_features.write_backward && 
evsel->attr.write_backward) {
                perf_missing_features.write_backward = true;
-               goto fallback_missing_features;
+               goto out_close;
        } else if (!perf_missing_features.clockid_wrong && 
evsel->attr.use_clockid) {
                perf_missing_features.clockid_wrong = true;
                goto fallback_missing_features;
@@ -2404,7 +2402,7 @@ int perf_evsel__open_strerror(struct perf_evsel *evsel, 
struct target *target,
        "We found oprofile daemon running, please stop it and try again.");
                break;
        case EINVAL:
-               if (evsel->overwrite && perf_missing_features.write_backward)
+               if (evsel->attr.write_backward && 
perf_missing_features.write_backward)
                        return scnprintf(msg, size, "Reading from overwrite 
event is not supported by this kernel.");
                if (perf_missing_features.clockid)
                        return scnprintf(msg, size, "clockid feature not 
supported.");
diff --git a/tools/perf/util/evsel.h b/tools/perf/util/evsel.h
index 86fed7a2932b..a31ee2d5d2d3 100644
--- a/tools/perf/util/evsel.h
+++ b/tools/perf/util/evsel.h
@@ -114,7 +114,6 @@ struct perf_evsel {
        bool                    tracking;
        bool                    per_pkg;
        bool                    precise_max;
-       bool                    overwrite;
        /* parse modifier helper */
        int                     exclude_GH;
        int                     nr_members;

Reply via email to