Re: [PATCH v12 1/8] perf tests: Skip backward ring buffer on old kernel

[Arnaldo Carvalho de Melo]

Em Fri, Jul 01, 2016 at 02:23:44AM +0000, Wang Nan escreveu:
> From: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
> 
> '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;