linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Masami Hiramatsu <mhiramat@kernel.org>
To: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: linux-kernel@vger.kernel.org,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@redhat.com>, Namhyung Kim <namhyung@kernel.org>,
	linux-perf-users@vger.kernel.org
Subject: Re: [PATCH 3/3] perf/probe: Add --bootconfig to output definition in bootconfig format
Date: Sat, 19 Jun 2021 00:07:31 +0900	[thread overview]
Message-ID: <20210619000731.56f0d7ee63c4acd857c2f287@kernel.org> (raw)
In-Reply-To: <YMycGkhkPuo4xTc4@kernel.org>

On Fri, 18 Jun 2021 10:14:02 -0300
Arnaldo Carvalho de Melo <acme@kernel.org> wrote:

> Em Sat, Jun 05, 2021 at 01:28:43AM +0900, Masami Hiramatsu escreveu:
> > Now the boot-time tracing supports the kprobes events and that
> > must be written in bootconfig file as following format.
> > 
> > ftrace.event.kprobes.<EVENT_NAME>.probes = <PROBE-DEF>
> > 
> > The perf probe already supports --definition (-D) action to
> > show the probe definitions, but the format is for the tracefs.
> 
> You forgot to add the tools/perf/Documentation/perf-probe.txt entry for
> this new command line option, please sent it as a followup patch.

Oops, let me write it up! Thanks for pointing!

> 
> Applied.
> 
> - Arnaldo
>  
> > [p|r][:EVENT_NAME] <PROBE-DEF>
> > 
> > This adds --bootconfig option for -D action so that it output
> > the probe definitions in bootconfig fromat. E.g.
> > 
> >  $ perf probe --bootconfig -D "path_lookupat:7 err:s32 s:string"
> >  ftrace.event.kprobes.path_lookupat_L7.probe = 'path_lookupat.isra.0+309 err_s32=%ax:s32 s_string=+0(%r13):string'
> > 
> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> > ---
> >  tools/perf/builtin-probe.c    |   12 ++++++-
> >  tools/perf/util/probe-event.c |   72 +++++++++++++++++++++++++++++++++++++++++
> >  tools/perf/util/probe-event.h |    2 +
> >  3 files changed, 85 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
> > index 6b1507566770..2bfd41df621c 100644
> > --- a/tools/perf/builtin-probe.c
> > +++ b/tools/perf/builtin-probe.c
> > @@ -347,7 +347,10 @@ static int perf_add_probe_events(struct perf_probe_event *pevs, int npevs)
> >  		goto out_cleanup;
> >  
> >  	if (params.command == 'D') {	/* it shows definition */
> > -		ret = show_probe_trace_events(pevs, npevs);
> > +		if (probe_conf.bootconfig)
> > +			ret = show_bootconfig_events(pevs, npevs);
> > +		else
> > +			ret = show_probe_trace_events(pevs, npevs);
> >  		goto out_cleanup;
> >  	}
> >  
> > @@ -581,6 +584,8 @@ __cmd_probe(int argc, const char **argv)
> >  		   "Look for files with symbols relative to this directory"),
> >  	OPT_CALLBACK(0, "target-ns", NULL, "pid",
> >  		     "target pid for namespace contexts", opt_set_target_ns),
> > +	OPT_BOOLEAN(0, "bootconfig", &probe_conf.bootconfig,
> > +		    "Output probe definition with bootconfig format"),
> >  	OPT_END()
> >  	};
> >  	int ret;
> > @@ -692,6 +697,11 @@ __cmd_probe(int argc, const char **argv)
> >  		}
> >  		break;
> >  	case 'D':
> > +		if (probe_conf.bootconfig && params.uprobes) {
> > +			pr_err("  Error: --bootconfig doesn't support uprobes.\n");
> > +			return -EINVAL;
> > +		}
> > +		__fallthrough;
> >  	case 'a':
> >  
> >  		/* Ensure the last given target is used */
> > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> > index 505c0702dbe2..f1348fa3dd1b 100644
> > --- a/tools/perf/util/probe-event.c
> > +++ b/tools/perf/util/probe-event.c
> > @@ -3564,6 +3564,78 @@ int show_probe_trace_events(struct perf_probe_event *pevs, int npevs)
> >  	return ret;
> >  }
> >  
> > +static int show_bootconfig_event(struct probe_trace_event *tev)
> > +{
> > +	struct probe_trace_point *tp = &tev->point;
> > +	struct strbuf buf;
> > +	char *ret = NULL;
> > +	int err;
> > +
> > +	if (strbuf_init(&buf, 32) < 0)
> > +		return -ENOMEM;
> > +
> > +	err = synthesize_kprobe_trace_def(tp, &buf);
> > +	if (err >= 0)
> > +		err = synthesize_probe_trace_args(tev, &buf);
> > +	if (err >= 0)
> > +		ret = strbuf_detach(&buf, NULL);
> > +	strbuf_release(&buf);
> > +
> > +	if (ret) {
> > +		printf("'%s'", ret);
> > +		free(ret);
> > +	}
> > +
> > +	return err;
> > +}
> > +
> > +int show_bootconfig_events(struct perf_probe_event *pevs, int npevs)
> > +{
> > +	struct strlist *namelist = strlist__new(NULL, NULL);
> > +	struct probe_trace_event *tev;
> > +	struct perf_probe_event *pev;
> > +	char *cur_name = NULL;
> > +	int i, j, ret = 0;
> > +
> > +	if (!namelist)
> > +		return -ENOMEM;
> > +
> > +	for (j = 0; j < npevs && !ret; j++) {
> > +		pev = &pevs[j];
> > +		if (pev->group && strcmp(pev->group, "probe"))
> > +			pr_warning("WARN: Group name %s is ignored\n", pev->group);
> > +		if (pev->uprobes) {
> > +			pr_warning("ERROR: Bootconfig doesn't support uprobes\n");
> > +			ret = -EINVAL;
> > +			break;
> > +		}
> > +		for (i = 0; i < pev->ntevs && !ret; i++) {
> > +			tev = &pev->tevs[i];
> > +			/* Skip if the symbol is out of .text or blacklisted */
> > +			if (!tev->point.symbol && !pev->uprobes)
> > +				continue;
> > +
> > +			/* Set new name for tev (and update namelist) */
> > +			ret = probe_trace_event__set_name(tev, pev,
> > +							  namelist, true);
> > +			if (ret)
> > +				break;
> > +
> > +			if (!cur_name || strcmp(cur_name, tev->event)) {
> > +				printf("%sftrace.event.kprobes.%s.probe = ",
> > +					cur_name ? "\n" : "", tev->event);
> > +				cur_name = tev->event;
> > +			} else
> > +				printf(", ");
> > +			ret = show_bootconfig_event(tev);
> > +		}
> > +	}
> > +	printf("\n");
> > +	strlist__delete(namelist);
> > +
> > +	return ret;
> > +}
> > +
> >  int apply_perf_probe_events(struct perf_probe_event *pevs, int npevs)
> >  {
> >  	int i, ret = 0;
> > diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
> > index 4f0eb3a20c36..65769d7949a3 100644
> > --- a/tools/perf/util/probe-event.h
> > +++ b/tools/perf/util/probe-event.h
> > @@ -15,6 +15,7 @@ struct probe_conf {
> >  	bool	force_add;
> >  	bool	no_inlines;
> >  	bool	cache;
> > +	bool	bootconfig;
> >  	int	max_probes;
> >  	unsigned long	magic_num;
> >  };
> > @@ -163,6 +164,7 @@ int add_perf_probe_events(struct perf_probe_event *pevs, int npevs);
> >  int convert_perf_probe_events(struct perf_probe_event *pevs, int npevs);
> >  int apply_perf_probe_events(struct perf_probe_event *pevs, int npevs);
> >  int show_probe_trace_events(struct perf_probe_event *pevs, int npevs);
> > +int show_bootconfig_events(struct perf_probe_event *pevs, int npevs);
> >  void cleanup_perf_probe_events(struct perf_probe_event *pevs, int npevs);
> >  
> >  struct strfilter;
> > 
> 
> -- 
> 
> - Arnaldo


-- 
Masami Hiramatsu <mhiramat@kernel.org>

  reply	other threads:[~2021-06-18 15:07 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-04 16:28 [PATCH 0/3] perf probe: Boot time tracing support Masami Hiramatsu
2021-06-04 16:28 ` [PATCH 1/3] perf/probe: Support probes on init functions for offline kernel Masami Hiramatsu
2021-06-04 16:28 ` [PATCH 2/3] perf/probe: Cleanup synthesize_probe_trace_command Masami Hiramatsu
2021-06-04 16:28 ` [PATCH 3/3] perf/probe: Add --bootconfig to output definition in bootconfig format Masami Hiramatsu
2021-06-18 13:14   ` Arnaldo Carvalho de Melo
2021-06-18 15:07     ` Masami Hiramatsu [this message]
2021-06-18 16:10     ` [PATCH 0/3] perf probe: Boot time tracing support followup Masami Hiramatsu
2021-06-18 16:10       ` [PATCH 1/3] [v2] perf/probe: Add --bootconfig to output definition in bootconfig format Masami Hiramatsu
2021-06-18 16:11       ` [PATCH 2/3] perf/probe: Show return probe correctly with --bootconfig Masami Hiramatsu
2021-06-18 16:11       ` [PATCH 3/3] perf/probe: docs: Add --bootconfig option to perf-probe manual Masami Hiramatsu
2021-06-18 15:55   ` [PATCH 3/3] perf/probe: Add --bootconfig to output definition in bootconfig format Masami Hiramatsu

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=20210619000731.56f0d7ee63c4acd857c2f287@kernel.org \
    --to=mhiramat@kernel.org \
    --cc=acme@kernel.org \
    --cc=alexander.shishkin@linux.intel.com \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    /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).