| Message ID | 20220506201627.85598-2-namhyung@kernel.org (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | BPF |
| Headers | show |
| Series | perf record: Implement off-cpu profiling with BPF (v2) | expand |
| Context | Check | Description |
|---|---|---|
| bpf/vmtest-bpf-next-VM_Test-2 | success | Logs for Kernel LATEST on z15 + selftests |
| bpf/vmtest-bpf-next-PR | success | PR summary |
| bpf/vmtest-bpf-next-VM_Test-1 | success | Logs for Kernel LATEST on ubuntu-latest + selftests |
| netdev/tree_selection | success | Not a local patch |
Em Fri, May 06, 2022 at 01:16:24PM -0700, Namhyung Kim escreveu: > Currently evsel__new_idx() sets more sample_type bits when it finds a > BPF-output event. But it should honor what's recorded in the perf > data file rather than blindly sets the bits. Otherwise it could lead > to a parse error when it recorded with a modified sample_type. Can you please try to provide a Fixes: tag for this? This way reviewing gets simpler by looking at who introduced this, if there was some reason or if it was a plain oversight. It also helps to get this fix propabated to the stable trees. - Arnaldo > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > --- > tools/perf/util/evsel.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c > index d38722560e80..63a8b832b822 100644 > --- a/tools/perf/util/evsel.c > +++ b/tools/perf/util/evsel.c > @@ -269,8 +269,8 @@ struct evsel *evsel__new_idx(struct perf_event_attr *attr, int idx) > return NULL; > evsel__init(evsel, attr, idx); > > - if (evsel__is_bpf_output(evsel)) { > - evsel->core.attr.sample_type |= (PERF_SAMPLE_RAW | PERF_SAMPLE_TIME | > + if (evsel__is_bpf_output(evsel) && !attr->sample_type) { > + evsel->core.attr.sample_type = (PERF_SAMPLE_RAW | PERF_SAMPLE_TIME | > PERF_SAMPLE_CPU | PERF_SAMPLE_PERIOD), > evsel->core.attr.sample_period = 1; > } > -- > 2.36.0.512.ge40c2bad7a-goog
Hi Arnaldo, On Tue, May 10, 2022 at 9:50 AM Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > > Em Fri, May 06, 2022 at 01:16:24PM -0700, Namhyung Kim escreveu: > > Currently evsel__new_idx() sets more sample_type bits when it finds a > > BPF-output event. But it should honor what's recorded in the perf > > data file rather than blindly sets the bits. Otherwise it could lead > > to a parse error when it recorded with a modified sample_type. > > Can you please try to provide a Fixes: tag for this? This way reviewing > gets simpler by looking at who introduced this, if there was some reason > or if it was a plain oversight. > > It also helps to get this fix propabated to the stable trees. Well.. actually this is not a fix. I've realized it adds some sample types when it creates a new evsel regardless of the perf_event_attr. This was not a problem so far (as nobody touched it), but when I changed some sample types during record for this change, perf report sees a different value and rejects the data. Thanks, namhyung
diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c index d38722560e80..63a8b832b822 100644 --- a/tools/perf/util/evsel.c +++ b/tools/perf/util/evsel.c @@ -269,8 +269,8 @@ struct evsel *evsel__new_idx(struct perf_event_attr *attr, int idx) return NULL; evsel__init(evsel, attr, idx); - if (evsel__is_bpf_output(evsel)) { - evsel->core.attr.sample_type |= (PERF_SAMPLE_RAW | PERF_SAMPLE_TIME | + if (evsel__is_bpf_output(evsel) && !attr->sample_type) { + evsel->core.attr.sample_type = (PERF_SAMPLE_RAW | PERF_SAMPLE_TIME | PERF_SAMPLE_CPU | PERF_SAMPLE_PERIOD), evsel->core.attr.sample_period = 1; }
Currently evsel__new_idx() sets more sample_type bits when it finds a BPF-output event. But it should honor what's recorded in the perf data file rather than blindly sets the bits. Otherwise it could lead to a parse error when it recorded with a modified sample_type. Signed-off-by: Namhyung Kim <namhyung@kernel.org> --- tools/perf/util/evsel.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)