linux-kernel - Re: [PATCH perf/core v11 07/20] perf probe: Add --cache option to cache the probe definitions

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20160616142906.GD4882@kernel.org>
Date:	Thu, 16 Jun 2016 11:29:06 -0300
From:	Arnaldo Carvalho de Melo <acme@...nel.org>
To:	Masami Hiramatsu <mhiramat@...nel.org>
Cc:	linux-kernel@...r.kernel.org, Namhyung Kim <namhyung@...nel.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...nel.org>,
	Hemant Kumar <hemant@...ux.vnet.ibm.com>,
	Ananth N Mavinakayanahalli <ananth@...ux.vnet.ibm.com>,
	Brendan Gregg <brendan.d.gregg@...il.com>
Subject: Re: [PATCH perf/core v11 07/20] perf probe: Add --cache option to
 cache the probe definitions

Em Thu, Jun 16, 2016 at 06:38:30AM +0900, Masami Hiramatsu escreveu:
> On Wed, 15 Jun 2016 14:38:59 -0300
> Arnaldo Carvalho de Melo <acme@...nel.org> wrote:
> 
> > Em Wed, Jun 15, 2016 at 12:28:40PM +0900, Masami Hiramatsu escreveu:
> > > From: Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
> > > 
> > > Add --cache option to cache the probe definitions. This
> > > just saves the result of the dwarf analysis to probe cache.
> > 
> > So, I applied up to this patch and will test build and send to Ingo
> > today, but from this point on I think you should provide usage examples,
> > i.e. use the options introduced, showing what effects it has on
> > the cache, etc.
> 
> Hmm, OK, I'll add that on perf-probe.txt, but anyway, unless [8/23]
> perf probe just write events in cache files, but not reuse (read)
> the cache entries. IOW, it would be just a waste of disk space....

This is just to ease reviewing, sometimes the way patch authors describe
their changes is not clear enough, so seeing what the tool does with the
patch applied helps in reviewing, that is all.

- Arnaldo
 
> Thank you,
> 
> > 
> > - Arnaldo
> >  
> > > Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
> > > Signed-off-by: Masami Hiramatsu <mhiramat@...nel.org>
> > > ---
> > >  Changes in v11:
> > >   - Check failure of probe_cache operations and warn it.
> > >  Changes in v10:
> > >   - Split into 5 subpatches.
> > >  Changes in v6:
> > >   - Remove unneeded O_APPEND from open(). (Thanks Namhyung!)
> > >   - Fix to check the return value of probe_cache_entry__new and strdup.(ditto)
> > >  Changes in v5:
> > >   - Move probe_cache* definitions. (code cleanup)
> > > 
> > >  Changes in v4:
> > >   - Remove cache saving failure message.
> > > ---
> > >  tools/perf/Documentation/perf-probe.txt |    4 ++++
> > >  tools/perf/builtin-probe.c              |    1 +
> > >  tools/perf/util/probe-event.c           |    9 +++++++++
> > >  tools/perf/util/probe-event.h           |    1 +
> > >  4 files changed, 15 insertions(+)
> > > 
> > > diff --git a/tools/perf/Documentation/perf-probe.txt b/tools/perf/Documentation/perf-probe.txt
> > > index 3a8a9ba..947db6f 100644
> > > --- a/tools/perf/Documentation/perf-probe.txt
> > > +++ b/tools/perf/Documentation/perf-probe.txt
> > > @@ -109,6 +109,10 @@ OPTIONS
> > >  	Dry run. With this option, --add and --del doesn't execute actual
> > >  	adding and removal operations.
> > >  
> > > +--cache::
> > > +	Cache the probes (with --add option). Any events which successfully added
> > > +	are also stored in the cache file.
> > > +
> > >  --max-probes=NUM::
> > >  	Set the maximum number of probe points for an event. Default is 128.
> > >  
> > > diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
> > > index 9af859b..6d7ab431 100644
> > > --- a/tools/perf/builtin-probe.c
> > > +++ b/tools/perf/builtin-probe.c
> > > @@ -512,6 +512,7 @@ __cmd_probe(int argc, const char **argv, const char *prefix __maybe_unused)
> > >  		    "Enable symbol demangling"),
> > >  	OPT_BOOLEAN(0, "demangle-kernel", &symbol_conf.demangle_kernel,
> > >  		    "Enable kernel symbol demangling"),
> > > +	OPT_BOOLEAN(0, "cache", &probe_conf.cache, "Manipulate probe cache"),
> > >  	OPT_END()
> > >  	};
> > >  	int ret;
> > > diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> > > index cbc8a8b..084756c 100644
> > > --- a/tools/perf/util/probe-event.c
> > > +++ b/tools/perf/util/probe-event.c
> > > @@ -2514,6 +2514,7 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
> > >  {
> > >  	int i, fd, ret;
> > >  	struct probe_trace_event *tev = NULL;
> > > +	struct probe_cache *cache = NULL;
> > >  	struct strlist *namelist;
> > >  
> > >  	fd = probe_file__open(PF_FL_RW | (pev->uprobes ? PF_FL_UPROBE : 0));
> > > @@ -2555,6 +2556,14 @@ static int __add_probe_trace_events(struct perf_probe_event *pev,
> > >  	}
> > >  	if (ret == -EINVAL && pev->uprobes)
> > >  		warn_uprobe_event_compat(tev);
> > > +	if (ret == 0 && probe_conf.cache) {
> > > +		cache = probe_cache__new(pev->target);
> > > +		if (!cache ||
> > > +		    probe_cache__add_entry(cache, pev, tevs, ntevs) < 0 ||
> > > +		    probe_cache__commit(cache) < 0)
> > > +			pr_warning("Failed to add event to probe cache\n");
> > > +		probe_cache__delete(cache);
> > > +	}
> > >  
> > >  	strlist__delete(namelist);
> > >  close_out:
> > > diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
> > > index 0b024ba..432b690 100644
> > > --- a/tools/perf/util/probe-event.h
> > > +++ b/tools/perf/util/probe-event.h
> > > @@ -12,6 +12,7 @@ struct probe_conf {
> > >  	bool	show_location_range;
> > >  	bool	force_add;
> > >  	bool	no_inlines;
> > > +	bool	cache;
> > >  	int	max_probes;
> > >  };
> > >  extern struct probe_conf probe_conf;
> 
> 
> -- 
> Masami Hiramatsu <mhiramat@...nel.org>