linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jiri Olsa <jolsa@redhat.com>
To: Namhyung Kim <namhyung@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>,
	Ingo Molnar <mingo@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Andi Kleen <ak@linux.intel.com>, Ian Rogers <irogers@google.com>,
	Stephane Eranian <eranian@google.com>,
	Adrian Hunter <adrian.hunter@intel.com>,
	Jin Yao <yao.jin@linux.intel.com>
Subject: Re: [PATCH v2 2/2] perf record: Add --synth option
Date: Mon, 26 Jul 2021 15:15:44 +0200	[thread overview]
Message-ID: <YP61gJD91GwvfzwT@krava> (raw)
In-Reply-To: <20210724040129.2268452-2-namhyung@kernel.org>

On Fri, Jul 23, 2021 at 09:01:29PM -0700, Namhyung Kim wrote:
> Add an option to control synthesize behavior.
> 
>     --synth <no|all|task|mmap|cgroup>
>                       Fine-tune event synthesis: default=all
> 
> This can be useful when we know it doesn't need some synthesis like
> in a specific usecase and/or when using pipe:
> 
>   $ perf record -a --all-cgroups --synth cgroup -o- sleep 1 | \
>   > perf report -i- -s cgroup
> 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/Documentation/perf-record.txt |  9 +++++
>  tools/perf/builtin-record.c              | 48 +++++++++++++++++++-----
>  tools/perf/util/record.h                 |  1 +
>  tools/perf/util/synthetic-events.c       | 28 ++++++++++++++
>  tools/perf/util/synthetic-events.h       | 12 ++++++
>  5 files changed, 89 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/perf/Documentation/perf-record.txt b/tools/perf/Documentation/perf-record.txt
> index d71bac847936..03a41bec0583 100644
> --- a/tools/perf/Documentation/perf-record.txt
> +++ b/tools/perf/Documentation/perf-record.txt
> @@ -596,6 +596,15 @@ options.
>  'perf record --dry-run -e' can act as a BPF script compiler if llvm.dump-obj
>  in config file is set to true.
>  
> +--synth=TYPE::
> +Collect and synthesize given type of events (comma separated).
> +Available types are:
> +  'task'    - synthesize FORK and COMM events for each task
> +  'mmap'    - synthesize MMAP events for each process (implies 'task')
> +  'cgroup'  - synthesize CGROUP events for each cgroup
> +  'all'     - synthesize all events (default)
> +  'no'      - do not synthesize any of the above events

this only affects events we take from proc right?

should we perhaps mention it, because you'll get mmap
events from kernel even if you specify --synth=task

jirka


> +
>  --tail-synthesize::
>  Instead of collecting non-sample events (for example, fork, comm, mmap) at
>  the beginning of record, collect them during finalizing an output file.
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index 535da4dfd8d3..caaada3594ce 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -1254,6 +1254,7 @@ static int record__synthesize_workload(struct record *rec, bool tail)
>  {
>  	int err;
>  	struct perf_thread_map *thread_map;
> +	bool needs_mmap = rec->opts.synth & PERF_SYNTH_MMAP;
>  
>  	if (rec->opts.tail_synthesize != tail)
>  		return 0;
> @@ -1265,7 +1266,7 @@ static int record__synthesize_workload(struct record *rec, bool tail)
>  	err = perf_event__synthesize_thread_map(&rec->tool, thread_map,
>  						 process_synthesized_event,
>  						 &rec->session->machines.host,
> -						 true,
> +						 needs_mmap,
>  						 rec->opts.sample_address);
>  	perf_thread_map__put(thread_map);
>  	return err;
> @@ -1500,20 +1501,26 @@ static int record__synthesize(struct record *rec, bool tail)
>  	if (err < 0)
>  		pr_warning("Couldn't synthesize bpf events.\n");
>  
> -	err = perf_event__synthesize_cgroups(tool, process_synthesized_event,
> -					     machine);
> -	if (err < 0)
> -		pr_warning("Couldn't synthesize cgroup events.\n");
> +	if (rec->opts.synth & PERF_SYNTH_CGROUP) {
> +		err = perf_event__synthesize_cgroups(tool, process_synthesized_event,
> +						     machine);
> +		if (err < 0)
> +			pr_warning("Couldn't synthesize cgroup events.\n");
> +	}
>  
>  	if (rec->opts.nr_threads_synthesize > 1) {
>  		perf_set_multithreaded();
>  		f = process_locked_synthesized_event;
>  	}
>  
> -	err = __machine__synthesize_threads(machine, tool, &opts->target,
> -					    rec->evlist->core.threads,
> -					    f, true, opts->sample_address,
> -					    rec->opts.nr_threads_synthesize);
> +	if (rec->opts.synth & PERF_SYNTH_TASK) {
> +		bool needs_mmap = rec->opts.synth & PERF_SYNTH_MMAP;
> +
> +		err = __machine__synthesize_threads(machine, tool, &opts->target,
> +						    rec->evlist->core.threads,
> +						    f, needs_mmap, opts->sample_address,
> +						    rec->opts.nr_threads_synthesize);
> +	}
>  
>  	if (rec->opts.nr_threads_synthesize > 1)
>  		perf_set_singlethreaded();
> @@ -2422,6 +2429,26 @@ static int process_timestamp_boundary(struct perf_tool *tool,
>  	return 0;
>  }
>  
> +static int parse_record_synth_option(const struct option *opt,
> +				     const char *str,
> +				     int unset __maybe_unused)
> +{
> +	struct record_opts *opts = opt->value;
> +	char *p = strdup(str);
> +
> +	if (p == NULL)
> +		return -1;
> +
> +	opts->synth = parse_synth_opt(p);
> +	free(p);
> +
> +	if (opts->synth < 0) {
> +		pr_err("Invalid synth option: %s\n", str);
> +		return -1;
> +	}
> +	return 0;
> +}
> +
>  /*
>   * XXX Ideally would be local to cmd_record() and passed to a record__new
>   * because we need to have access to it in record__exit, that is called
> @@ -2447,6 +2474,7 @@ static struct record record = {
>  		.nr_threads_synthesize = 1,
>  		.ctl_fd              = -1,
>  		.ctl_fd_ack          = -1,
> +		.synth               = PERF_SYNTH_ALL,
>  	},
>  	.tool = {
>  		.sample		= process_sample_event,
> @@ -2662,6 +2690,8 @@ static struct option __record_options[] = {
>  		     "\t\t\t  Optionally send control command completion ('ack\\n') to ack-fd descriptor.\n"
>  		     "\t\t\t  Alternatively, ctl-fifo / ack-fifo will be opened and used as ctl-fd / ack-fd.",
>  		      parse_control_option),
> +	OPT_CALLBACK(0, "synth", &record.opts, "no|all|task|mmap|cgroup",
> +		     "Fine-tune event synthesis: default=all", parse_record_synth_option),
>  	OPT_END()
>  };
>  
> diff --git a/tools/perf/util/record.h b/tools/perf/util/record.h
> index 68f471d9a88b..ef6c2715fdd9 100644
> --- a/tools/perf/util/record.h
> +++ b/tools/perf/util/record.h
> @@ -77,6 +77,7 @@ struct record_opts {
>  	int	      ctl_fd;
>  	int	      ctl_fd_ack;
>  	bool	      ctl_fd_close;
> +	int	      synth;
>  };
>  
>  extern const char * const *record_usage;
> diff --git a/tools/perf/util/synthetic-events.c b/tools/perf/util/synthetic-events.c
> index 566e0859fcfb..9485d0532b9c 100644
> --- a/tools/perf/util/synthetic-events.c
> +++ b/tools/perf/util/synthetic-events.c
> @@ -2186,3 +2186,31 @@ int perf_event__synthesize_features(struct perf_tool *tool, struct perf_session
>  	free(ff.buf);
>  	return ret;
>  }
> +
> +int parse_synth_opt(char *synth)
> +{
> +	char *p, *q;
> +	int ret = 0;
> +
> +	if (synth == NULL)
> +		return -1;
> +
> +	for (q = synth; (p = strsep(&q, ",")); p = q) {
> +		if (!strcasecmp(p, "no") || !strcasecmp(p, "none"))
> +			return 0;
> +
> +		if (!strcasecmp(p, "all"))
> +			return PERF_SYNTH_ALL;
> +
> +		if (!strcasecmp(p, "task"))
> +			ret |= PERF_SYNTH_TASK;
> +		else if (!strcasecmp(p, "mmap"))
> +			ret |= PERF_SYNTH_TASK | PERF_SYNTH_MMAP;
> +		else if (!strcasecmp(p, "cgroup"))
> +			ret |= PERF_SYNTH_CGROUP;
> +		else
> +			return -1;
> +	}
> +
> +	return ret;
> +}
> diff --git a/tools/perf/util/synthetic-events.h b/tools/perf/util/synthetic-events.h
> index 61bbdb3b64df..913803506345 100644
> --- a/tools/perf/util/synthetic-events.h
> +++ b/tools/perf/util/synthetic-events.h
> @@ -26,6 +26,18 @@ struct target;
>  
>  union perf_event;
>  
> +enum perf_record_synth {
> +	PERF_SYNTH_TASK		= 1 << 0,
> +	PERF_SYNTH_MMAP		= 1 << 1,
> +	PERF_SYNTH_CGROUP	= 1 << 2,
> +
> +	/* last element */
> +	PERF_SYNTH_MAX		= 1 << 3,
> +};
> +#define PERF_SYNTH_ALL  (PERF_SYNTH_MAX - 1)
> +
> +int parse_synth_opt(char *str);
> +
>  typedef int (*perf_event__handler_t)(struct perf_tool *tool, union perf_event *event,
>  				     struct perf_sample *sample, struct machine *machine);
>  
> -- 
> 2.32.0.432.gabb21c7263-goog
> 


  reply	other threads:[~2021-07-26 13:15 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-24  4:01 [PATCH v2 1/2] perf tools: Allow to control synthesize during record Namhyung Kim
2021-07-24  4:01 ` [PATCH v2 2/2] perf record: Add --synth option Namhyung Kim
2021-07-26 13:15   ` Jiri Olsa [this message]
2021-08-11  4:33     ` Namhyung Kim

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YP61gJD91GwvfzwT@krava \
    --to=jolsa@redhat.com \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=ak@linux.intel.com \
    --cc=eranian@google.com \
    --cc=irogers@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    --cc=yao.jin@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).