Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1434967 > unrolled thread
| Started by | Wang Nan <wangnan0@huawei.com> |
|---|---|
| First post | 2016-07-01 04:30 +0200 |
| Last post | 2016-07-01 16:50 +0200 |
| Articles | 4 — 4 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH v12 1/8] perf tests: Skip backward ring buffer on old kernel Wang Nan <wangnan0@huawei.com> - 2016-07-01 04:30 +0200
Re: [PATCH v12 1/8] perf tests: Skip backward ring buffer on old kernel Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-07-01 15:50 +0200
Re: [PATCH v12 1/8] perf tests: Skip backward ring buffer on old kernel pi3orama <pi3orama@163.com> - 2016-07-01 16:30 +0200
Re: [PATCH v12 1/8] perf tests: Skip backward ring buffer on old kernel Arnaldo Carvalho de Melo <acme@redhat.com> - 2016-07-01 16:50 +0200
| From | Wang Nan <wangnan0@huawei.com> |
|---|---|
| Date | 2016-07-01 04:30 +0200 |
| Subject | [PATCH v12 1/8] perf tests: Skip backward ring buffer on old kernel |
| Message-ID | <rPWnn-Rf-3@gated-at.bofh.it> |
From: Arnaldo Carvalho de Melo <acme@redhat.com>
'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.
Signed-off-by: Wang Nan <wangnan0@huawei.com>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
tools/perf/tests/backward-ring-buffer.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/tools/perf/tests/backward-ring-buffer.c b/tools/perf/tests/backward-ring-buffer.c
index e70313f..0731b0b 100644
--- a/tools/perf/tests/backward-ring-buffer.c
+++ b/tools/perf/tests/backward-ring-buffer.c
@@ -118,8 +118,10 @@ int test__backward_ring_buffer(int subtest __maybe_unused)
perf_evlist__config(evlist, &opts, NULL);
/* Set backward bit, ring buffer should be writing from end */
- evlist__for_each_entry(evlist, evsel)
+ evlist__for_each_entry(evlist, evsel) {
+ evsel->overwrite = true;
evsel->attr.write_backward = 1;
+ }
err = perf_evlist__open(evlist);
if (err < 0) {
--
1.8.3.4
[toc] | [next] | [standalone]
| From | Arnaldo Carvalho de Melo <acme@kernel.org> |
|---|---|
| Date | 2016-07-01 15:50 +0200 |
| Subject | Re: [PATCH v12 1/8] perf tests: Skip backward ring buffer on old kernel |
| Message-ID | <rQ6Zs-7ge-19@gated-at.bofh.it> |
| In reply to | #1434967 |
Em Fri, Jul 01, 2016 at 02:23:44AM +0000, Wang Nan escreveu:
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>
> '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;
[toc] | [prev] | [next] | [standalone]
| From | pi3orama <pi3orama@163.com> |
|---|---|
| Date | 2016-07-01 16:30 +0200 |
| Message-ID | <rQ7Ca-7Jf-49@gated-at.bofh.it> |
| In reply to | #1435346 |
发自我的 iPhone
> 在 2016年7月1日，下午9:40，Arnaldo Carvalho de Melo <acme@kernel.org> 写道：
>
> Em Fri, Jul 01, 2016 at 02:23:44AM +0000, Wang Nan escreveu:
>> From: Arnaldo Carvalho de Melo <acme@redhat.com>
>>
>> '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?
>
If we remove evsel->overwrite, what should
we set in parser? Make it directly setting
attr?
Thank you.
> 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;
[toc] | [prev] | [next] | [standalone]
| From | Arnaldo Carvalho de Melo <acme@redhat.com> |
|---|---|
| Date | 2016-07-01 16:50 +0200 |
| Subject | Re: [PATCH v12 1/8] perf tests: Skip backward ring buffer on old kernel |
| Message-ID | <rQ7Vv-7Qe-29@gated-at.bofh.it> |
| In reply to | #1435373 |
Em Fri, Jul 01, 2016 at 10:27:35PM +0800, pi3orama escreveu: > > 在 2016年7月1日，下午9:40，Arnaldo Carvalho de Melo <acme@kernel.org> 写道： > > I.e. perhaps evsel->overwrite is completely unnecessary and places > > checking for it should instead check for evsel->attr.backward, no? > If we remove evsel->overwrite, what should we set in parser? Make it > directly setting attr? yeah, is that possible? Trying... And then overwrite is how we chose to expose this to the user, in the event syntax, but internally that means using the backwards_ring buffer, right? So this simplifies the code, removing information that was being duplicated in evsel->overwrite and evsel->attr.write_backward. - Arnaldo
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web