Re: [PATCH 3/5] perf record: Add new option to sample identifier

Ian Rogers
2022-06-14T14:53:21Z

On Fri, Jun 10, 2022 at 4:33 AM Adrian Hunter <adrian.hunter@intel.com> wrote:
>
> In preparation for recording sideband events in a virtual machine guest so
> that they can be injected into a host perf.data file.
>
> Add an option to always include sample type PERF_SAMPLE_IDENTIFIER.
>
> Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
> ---
> tools/perf/Documentation/perf-record.txt | 3 +++
> tools/perf/builtin-record.c | 2 ++
> tools/perf/util/record.c | 2 +-
> tools/perf/util/record.h | 1 +
> 4 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
> index cf8ad50f3de1..8fcea74fe3c2 100644
> --- a/tools/perf/Documentation/perf-record.txt
> +++ b/tools/perf/Documentation/perf-record.txt
> @@ -313,6 +313,9 @@ OPTIONS
> --sample-cpu::
> Record the sample cpu.
>
> +--sample-identifier::
> + Record the sample identifier.

I can see this is following the pattern of --sample-cpu above, but I'm
not sure a "sample identifier" is going to be clear to a novice user.
The comment adds little beyond the option name. Perhaps expand with an
example?

Thanks,
Ian

> +
> -n::
> --no-samples::
> Don't sample.
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index 00c2a6cdf1be..40dca1fba4e3 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -3191,6 +3191,8 @@ static struct option __record_options[] = {
> OPT_BOOLEAN(0, "code-page-size", &record.opts.sample_code_page_size,
> "Record the sampled code address (ip) page size"),
> OPT_BOOLEAN(0, "sample-cpu", &record.opts.sample_cpu, "Record the sample cpu"),
> + OPT_BOOLEAN(0, "sample-identifier", &record.opts.sample_identifier,
> + "Record the sample identifier"),
> OPT_BOOLEAN_SET('T', "timestamp", &record.opts.sample_time,
> &record.opts.sample_time_set,
> "Record the sample timestamps"),
> diff --git a/tools/perf/util/record.c b/tools/perf/util/record.c
> index 5b09ecbb05dc..b529636ab3ea 100644
> --- a/tools/perf/util/record.c
> +++ b/tools/perf/util/record.c
> @@ -121,7 +121,7 @@ void evlist__config(struct evlist *evlist, struct record_opts *opts, struct call
> evlist__for_each_entry(evlist, evsel)
> evsel__config_leader_sampling(evsel, evlist);
>
> - if (opts->full_auxtrace) {
> + if (opts->full_auxtrace || opts->sample_identifier) {
> /*
> * Need to be able to synthesize and parse selected events with
> * arbitrary sample types, which requires always being able to
> diff --git a/tools/perf/util/record.h b/tools/perf/util/record.h
> index be9a957501f4..4269e916f450 100644
> --- a/tools/perf/util/record.h
> +++ b/tools/perf/util/record.h
> @@ -28,6 +28,7 @@ struct record_opts {
> bool sample_time;
> bool sample_time_set;
> bool sample_cpu;
> + bool sample_identifier;
> bool period;
> bool period_set;
> bool running_time;
> --
> 2.25.1
>