* [PATCH v1] perf inject: Fix leader sampling inserting additional samples
@ 2024-07-29 22:06 Ian Rogers
2024-07-31 18:21 ` Namhyung Kim
0 siblings, 1 reply; 4+ messages in thread
From: Ian Rogers @ 2024-07-29 22:06 UTC (permalink / raw)
To: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Namhyung Kim, Mark Rutland, Alexander Shishkin, Jiri Olsa,
Ian Rogers, Adrian Hunter, Kan Liang, linux-kernel,
linux-perf-users, Andi Kleen
The processing of leader samples would turn an individual sample with
a group of read values into multiple samples. Perf inject would pass
through the additional samples increasing the output data file size:
```
$ perf record -g -e "{instructions,cycles}:S" -o perf.orig.data true
$ perf script -D -i perf.orig.data | sed -e 's/perf.orig.data/perf.data/g' > orig.txt
$ perf inject -i perf.orig.data -o perf.new.data
$ perf script -D -i perf.new.data | sed -e 's/perf.new.data/perf.data/g' > new.txt
$ diff -u orig.txt new.txt
--- orig.txt 2024-07-29 14:29:40.606576769 -0700
+++ new.txt 2024-07-29 14:30:04.142737434 -0700
...
-0xc550@perf.data [0x30]: event: 3
+0xc550@perf.data [0xd0]: event: 9
+.
+. ... raw event: size 208 bytes
+. 0000: 09 00 00 00 01 00 d0 00 fc 72 01 86 ff ff ff ff .........r......
+. 0010: 74 7d 2c 00 74 7d 2c 00 fb c3 79 f9 ba d5 05 00 t},.t},...y.....
+. 0020: e6 cb 1a 00 00 00 00 00 01 00 00 00 00 00 00 00 ................
+. 0030: 02 00 00 00 00 00 00 00 76 01 00 00 00 00 00 00 ........v.......
+. 0040: e6 cb 1a 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
+. 0050: 62 18 00 00 00 00 00 00 f6 cb 1a 00 00 00 00 00 b...............
+. 0060: 00 00 00 00 00 00 00 00 0c 00 00 00 00 00 00 00 ................
+. 0070: 80 ff ff ff ff ff ff ff fc 72 01 86 ff ff ff ff .........r......
+. 0080: f3 0e 6e 85 ff ff ff ff 0c cb 7f 85 ff ff ff ff ..n.............
+. 0090: bc f2 87 85 ff ff ff ff 44 af 7f 85 ff ff ff ff ........D.......
+. 00a0: bd be 7f 85 ff ff ff ff 26 d0 7f 85 ff ff ff ff ........&.......
+. 00b0: 6d a4 ff 85 ff ff ff ff ea 00 20 86 ff ff ff ff m......... .....
+. 00c0: 00 fe ff ff ff ff ff ff 57 14 4f 43 fc 7e 00 00 ........W.OC.~..
+
+1642373909693435 0xc550 [0xd0]: PERF_RECORD_SAMPLE(IP, 0x1): 2915700/2915700: 0xffffffff860172fc period: 1 addr: 0
+... FP chain: nr:12
+..... 0: ffffffffffffff80
+..... 1: ffffffff860172fc
+..... 2: ffffffff856e0ef3
+..... 3: ffffffff857fcb0c
+..... 4: ffffffff8587f2bc
+..... 5: ffffffff857faf44
+..... 6: ffffffff857fbebd
+..... 7: ffffffff857fd026
+..... 8: ffffffff85ffa46d
+..... 9: ffffffff862000ea
+..... 10: fffffffffffffe00
+..... 11: 00007efc434f1457
+... sample_read:
+.... group nr 2
+..... id 00000000001acbe6, value 0000000000000176, lost 0
+..... id 00000000001acbf6, value 0000000000001862, lost 0
+
+0xc620@perf.data [0x30]: event: 3
...
```
This behavior is incorrect as in the case above perf inject should
have done nothing. Fix this behavior by disabling separating samples
for a tool that requests it. Only request this for `perf inject` so as
to not affect other perf tools. With the patch and the test above
there are no differences between the orig.txt and new.txt.
Fixes: e4caec0d1af3 ("perf evsel: Add PERF_SAMPLE_READ sample related processing")
Signed-off-by: Ian Rogers <irogers@google.com>
---
tools/perf/builtin-inject.c | 1 +
tools/perf/util/session.c | 3 +++
tools/perf/util/tool.h | 1 +
3 files changed, 5 insertions(+)
diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
index a212678d47be..c80fb0f60e61 100644
--- a/tools/perf/builtin-inject.c
+++ b/tools/perf/builtin-inject.c
@@ -2204,6 +2204,7 @@ int cmd_inject(int argc, const char **argv)
.finished_init = perf_event__repipe_op2_synth,
.compressed = perf_event__repipe_op4_synth,
.auxtrace = perf_event__repipe_auxtrace,
+ .dont_split_sample_group = true,
},
.input_name = "-",
.samples = LIST_HEAD_INIT(inject.samples),
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 5596bed1b8c8..080242c69196 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -1511,6 +1511,9 @@ static int deliver_sample_group(struct evlist *evlist,
int ret = -EINVAL;
struct sample_read_value *v = sample->read.group.values;
+ if (tool->dont_split_sample_group)
+ return deliver_sample_value(evlist, tool, event, sample, v, machine);
+
sample_read_group__for_each(v, sample->read.group.nr, read_format) {
ret = deliver_sample_value(evlist, tool, event, sample, v,
machine);
diff --git a/tools/perf/util/tool.h b/tools/perf/util/tool.h
index c957fb849ac6..62bbc9cec151 100644
--- a/tools/perf/util/tool.h
+++ b/tools/perf/util/tool.h
@@ -85,6 +85,7 @@ struct perf_tool {
bool namespace_events;
bool cgroup_events;
bool no_warn;
+ bool dont_split_sample_group;
enum show_feature_header show_feat_hdr;
};
--
2.46.0.rc1.232.g9752f9e123-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v1] perf inject: Fix leader sampling inserting additional samples
2024-07-29 22:06 [PATCH v1] perf inject: Fix leader sampling inserting additional samples Ian Rogers
@ 2024-07-31 18:21 ` Namhyung Kim
2024-08-12 20:37 ` Ian Rogers
0 siblings, 1 reply; 4+ messages in thread
From: Namhyung Kim @ 2024-07-31 18:21 UTC (permalink / raw)
To: Ian Rogers
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
Kan Liang, linux-kernel, linux-perf-users, Andi Kleen
Hi Ian,
On Mon, Jul 29, 2024 at 03:06:20PM -0700, Ian Rogers wrote:
> The processing of leader samples would turn an individual sample with
> a group of read values into multiple samples. Perf inject would pass
> through the additional samples increasing the output data file size:
>
> ```
> $ perf record -g -e "{instructions,cycles}:S" -o perf.orig.data true
> $ perf script -D -i perf.orig.data | sed -e 's/perf.orig.data/perf.data/g' > orig.txt
> $ perf inject -i perf.orig.data -o perf.new.data
> $ perf script -D -i perf.new.data | sed -e 's/perf.new.data/perf.data/g' > new.txt
> $ diff -u orig.txt new.txt
> --- orig.txt 2024-07-29 14:29:40.606576769 -0700
> +++ new.txt 2024-07-29 14:30:04.142737434 -0700
> ...
> -0xc550@perf.data [0x30]: event: 3
> +0xc550@perf.data [0xd0]: event: 9
> +.
> +. ... raw event: size 208 bytes
> +. 0000: 09 00 00 00 01 00 d0 00 fc 72 01 86 ff ff ff ff .........r......
> +. 0010: 74 7d 2c 00 74 7d 2c 00 fb c3 79 f9 ba d5 05 00 t},.t},...y.....
> +. 0020: e6 cb 1a 00 00 00 00 00 01 00 00 00 00 00 00 00 ................
> +. 0030: 02 00 00 00 00 00 00 00 76 01 00 00 00 00 00 00 ........v.......
> +. 0040: e6 cb 1a 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> +. 0050: 62 18 00 00 00 00 00 00 f6 cb 1a 00 00 00 00 00 b...............
> +. 0060: 00 00 00 00 00 00 00 00 0c 00 00 00 00 00 00 00 ................
> +. 0070: 80 ff ff ff ff ff ff ff fc 72 01 86 ff ff ff ff .........r......
> +. 0080: f3 0e 6e 85 ff ff ff ff 0c cb 7f 85 ff ff ff ff ..n.............
> +. 0090: bc f2 87 85 ff ff ff ff 44 af 7f 85 ff ff ff ff ........D.......
> +. 00a0: bd be 7f 85 ff ff ff ff 26 d0 7f 85 ff ff ff ff ........&.......
> +. 00b0: 6d a4 ff 85 ff ff ff ff ea 00 20 86 ff ff ff ff m......... .....
> +. 00c0: 00 fe ff ff ff ff ff ff 57 14 4f 43 fc 7e 00 00 ........W.OC.~..
> +
> +1642373909693435 0xc550 [0xd0]: PERF_RECORD_SAMPLE(IP, 0x1): 2915700/2915700: 0xffffffff860172fc period: 1 addr: 0
> +... FP chain: nr:12
> +..... 0: ffffffffffffff80
> +..... 1: ffffffff860172fc
> +..... 2: ffffffff856e0ef3
> +..... 3: ffffffff857fcb0c
> +..... 4: ffffffff8587f2bc
> +..... 5: ffffffff857faf44
> +..... 6: ffffffff857fbebd
> +..... 7: ffffffff857fd026
> +..... 8: ffffffff85ffa46d
> +..... 9: ffffffff862000ea
> +..... 10: fffffffffffffe00
> +..... 11: 00007efc434f1457
> +... sample_read:
> +.... group nr 2
> +..... id 00000000001acbe6, value 0000000000000176, lost 0
> +..... id 00000000001acbf6, value 0000000000001862, lost 0
> +
> +0xc620@perf.data [0x30]: event: 3
> ...
> ```
>
> This behavior is incorrect as in the case above perf inject should
> have done nothing. Fix this behavior by disabling separating samples
> for a tool that requests it. Only request this for `perf inject` so as
> to not affect other perf tools. With the patch and the test above
> there are no differences between the orig.txt and new.txt.
>
> Fixes: e4caec0d1af3 ("perf evsel: Add PERF_SAMPLE_READ sample related processing")
> Signed-off-by: Ian Rogers <irogers@google.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Thanks,
Namhyung
> ---
> tools/perf/builtin-inject.c | 1 +
> tools/perf/util/session.c | 3 +++
> tools/perf/util/tool.h | 1 +
> 3 files changed, 5 insertions(+)
>
> diff --git a/tools/perf/builtin-inject.c b/tools/perf/builtin-inject.c
> index a212678d47be..c80fb0f60e61 100644
> --- a/tools/perf/builtin-inject.c
> +++ b/tools/perf/builtin-inject.c
> @@ -2204,6 +2204,7 @@ int cmd_inject(int argc, const char **argv)
> .finished_init = perf_event__repipe_op2_synth,
> .compressed = perf_event__repipe_op4_synth,
> .auxtrace = perf_event__repipe_auxtrace,
> + .dont_split_sample_group = true,
> },
> .input_name = "-",
> .samples = LIST_HEAD_INIT(inject.samples),
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index 5596bed1b8c8..080242c69196 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -1511,6 +1511,9 @@ static int deliver_sample_group(struct evlist *evlist,
> int ret = -EINVAL;
> struct sample_read_value *v = sample->read.group.values;
>
> + if (tool->dont_split_sample_group)
> + return deliver_sample_value(evlist, tool, event, sample, v, machine);
> +
> sample_read_group__for_each(v, sample->read.group.nr, read_format) {
> ret = deliver_sample_value(evlist, tool, event, sample, v,
> machine);
> diff --git a/tools/perf/util/tool.h b/tools/perf/util/tool.h
> index c957fb849ac6..62bbc9cec151 100644
> --- a/tools/perf/util/tool.h
> +++ b/tools/perf/util/tool.h
> @@ -85,6 +85,7 @@ struct perf_tool {
> bool namespace_events;
> bool cgroup_events;
> bool no_warn;
> + bool dont_split_sample_group;
> enum show_feature_header show_feat_hdr;
> };
>
> --
> 2.46.0.rc1.232.g9752f9e123-goog
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v1] perf inject: Fix leader sampling inserting additional samples
2024-07-31 18:21 ` Namhyung Kim
@ 2024-08-12 20:37 ` Ian Rogers
2024-08-12 20:51 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 4+ messages in thread
From: Ian Rogers @ 2024-08-12 20:37 UTC (permalink / raw)
To: Namhyung Kim
Cc: Peter Zijlstra, Ingo Molnar, Arnaldo Carvalho de Melo,
Mark Rutland, Alexander Shishkin, Jiri Olsa, Adrian Hunter,
Kan Liang, linux-kernel, linux-perf-users, Andi Kleen
On Wed, Jul 31, 2024 at 11:21 AM Namhyung Kim <namhyung@kernel.org> wrote:
>
> Hi Ian,
>
> On Mon, Jul 29, 2024 at 03:06:20PM -0700, Ian Rogers wrote:
> > The processing of leader samples would turn an individual sample with
> > a group of read values into multiple samples. Perf inject would pass
> > through the additional samples increasing the output data file size:
> >
> > ```
> > $ perf record -g -e "{instructions,cycles}:S" -o perf.orig.data true
> > $ perf script -D -i perf.orig.data | sed -e 's/perf.orig.data/perf.data/g' > orig.txt
> > $ perf inject -i perf.orig.data -o perf.new.data
> > $ perf script -D -i perf.new.data | sed -e 's/perf.new.data/perf.data/g' > new.txt
> > $ diff -u orig.txt new.txt
> > --- orig.txt 2024-07-29 14:29:40.606576769 -0700
> > +++ new.txt 2024-07-29 14:30:04.142737434 -0700
> > ...
> > -0xc550@perf.data [0x30]: event: 3
> > +0xc550@perf.data [0xd0]: event: 9
> > +.
> > +. ... raw event: size 208 bytes
> > +. 0000: 09 00 00 00 01 00 d0 00 fc 72 01 86 ff ff ff ff .........r......
> > +. 0010: 74 7d 2c 00 74 7d 2c 00 fb c3 79 f9 ba d5 05 00 t},.t},...y.....
> > +. 0020: e6 cb 1a 00 00 00 00 00 01 00 00 00 00 00 00 00 ................
> > +. 0030: 02 00 00 00 00 00 00 00 76 01 00 00 00 00 00 00 ........v.......
> > +. 0040: e6 cb 1a 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> > +. 0050: 62 18 00 00 00 00 00 00 f6 cb 1a 00 00 00 00 00 b...............
> > +. 0060: 00 00 00 00 00 00 00 00 0c 00 00 00 00 00 00 00 ................
> > +. 0070: 80 ff ff ff ff ff ff ff fc 72 01 86 ff ff ff ff .........r......
> > +. 0080: f3 0e 6e 85 ff ff ff ff 0c cb 7f 85 ff ff ff ff ..n.............
> > +. 0090: bc f2 87 85 ff ff ff ff 44 af 7f 85 ff ff ff ff ........D.......
> > +. 00a0: bd be 7f 85 ff ff ff ff 26 d0 7f 85 ff ff ff ff ........&.......
> > +. 00b0: 6d a4 ff 85 ff ff ff ff ea 00 20 86 ff ff ff ff m......... .....
> > +. 00c0: 00 fe ff ff ff ff ff ff 57 14 4f 43 fc 7e 00 00 ........W.OC.~..
> > +
> > +1642373909693435 0xc550 [0xd0]: PERF_RECORD_SAMPLE(IP, 0x1): 2915700/2915700: 0xffffffff860172fc period: 1 addr: 0
> > +... FP chain: nr:12
> > +..... 0: ffffffffffffff80
> > +..... 1: ffffffff860172fc
> > +..... 2: ffffffff856e0ef3
> > +..... 3: ffffffff857fcb0c
> > +..... 4: ffffffff8587f2bc
> > +..... 5: ffffffff857faf44
> > +..... 6: ffffffff857fbebd
> > +..... 7: ffffffff857fd026
> > +..... 8: ffffffff85ffa46d
> > +..... 9: ffffffff862000ea
> > +..... 10: fffffffffffffe00
> > +..... 11: 00007efc434f1457
> > +... sample_read:
> > +.... group nr 2
> > +..... id 00000000001acbe6, value 0000000000000176, lost 0
> > +..... id 00000000001acbf6, value 0000000000001862, lost 0
> > +
> > +0xc620@perf.data [0x30]: event: 3
> > ...
> > ```
> >
> > This behavior is incorrect as in the case above perf inject should
> > have done nothing. Fix this behavior by disabling separating samples
> > for a tool that requests it. Only request this for `perf inject` so as
> > to not affect other perf tools. With the patch and the test above
> > there are no differences between the orig.txt and new.txt.
> >
> > Fixes: e4caec0d1af3 ("perf evsel: Add PERF_SAMPLE_READ sample related processing")
> > Signed-off-by: Ian Rogers <irogers@google.com>
>
> Acked-by: Namhyung Kim <namhyung@kernel.org>
Ping.
Thanks,
Ian
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v1] perf inject: Fix leader sampling inserting additional samples
2024-08-12 20:37 ` Ian Rogers
@ 2024-08-12 20:51 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 4+ messages in thread
From: Arnaldo Carvalho de Melo @ 2024-08-12 20:51 UTC (permalink / raw)
To: Ian Rogers
Cc: Namhyung Kim, Peter Zijlstra, Ingo Molnar, Mark Rutland,
Alexander Shishkin, Jiri Olsa, Adrian Hunter, Kan Liang,
linux-kernel, linux-perf-users, Andi Kleen
On Mon, Aug 12, 2024 at 01:37:36PM -0700, Ian Rogers wrote:
> On Wed, Jul 31, 2024 at 11:21 AM Namhyung Kim <namhyung@kernel.org> wrote:
> > > This behavior is incorrect as in the case above perf inject should
> > > have done nothing. Fix this behavior by disabling separating samples
> > > for a tool that requests it. Only request this for `perf inject` so as
> > > to not affect other perf tools. With the patch and the test above
> > > there are no differences between the orig.txt and new.txt.
> > > Fixes: e4caec0d1af3 ("perf evsel: Add PERF_SAMPLE_READ sample related processing")
> > > Signed-off-by: Ian Rogers <irogers@google.com>
> > Acked-by: Namhyung Kim <namhyung@kernel.org>
> Ping.
Thanks, applied to perf-tools-next,
- Arnaldo
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-08-12 20:51 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-29 22:06 [PATCH v1] perf inject: Fix leader sampling inserting additional samples Ian Rogers
2024-07-31 18:21 ` Namhyung Kim
2024-08-12 20:37 ` Ian Rogers
2024-08-12 20:51 ` Arnaldo Carvalho de Melo
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.