* [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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).