Re: [PATCH perf/core v13 06/15] perf buildid-cache: Scan and import user SDT events to probe cache

[Masami Hiramatsu]

On Fri, 1 Jul 2016 15:22:01 -0300
Arnaldo Carvalho de Melo <acme@xxxxxxxxxx> wrote:

> Em Fri, Jul 01, 2016 at 05:04:10PM +0900, Masami Hiramatsu escreveu:
> > From: Masami Hiramatsu <masami.hiramatsu.pt@xxxxxxxxxxx>
> > 
> > perf buildid-cache --add <binary> scans given binary and add
> > the SDT events to probe cache. "sdt_" prefix is appended for
> > all SDT providers to avoid event-name clash with other pre-defined
> > events. It is possible to use the cached SDT events as other cached
> > events, via perf probe --add "sdt_<provider>:<event>=<event>".
> > 
> > e.g.
> >   ----
> >   # perf buildid-cache --add /lib/libc-2.17.so
> >   # perf probe --cache --list | head -n 5
> >   /usr/lib/libc-2.17.so (a6fb821bdf53660eb2c29f778757aef294d3d392):
> >   sdt_libc:setjmp=setjmp
> >   sdt_libc:longjmp=longjmp
> >   sdt_libc:longjmp_target=longjmp_target
> >   sdt_libc:memory_heap_new=memory_heap_new
> >   # perf probe -x /usr/lib/libc-2.17.so \
> >     -a sdt_libc:memory_heap_new=memory_heap_new
> 
> Why not remove the need for that build-cache --add? I.e. go straight to:
> 
>   perf probe -x /usr/lib/libc-2.17.so -a sdt_libc:memory_heap_new=memory_heap_new
> 
> And all the various steps above be done automagically?

Yes, I can add that.

> It may well be interesting to have it available as separate steps, as above,
> but requiring this cumbersome sequence to performed all the time when it seems
> possible to do automatically seems user unfriendly.

Agreed. In 11/15 added a feature to search SDT without -x, but
usually uprobes requires -x, using -x is also natural.

> 
> I am tentatively applying this and testing.

Thank you!

> 
> - Arnaldo
> 
> >   Added new event:
> >     sdt_libc:memory_heap_new (on memory_heap_new
> >    in /usr/lib/libc-2.17.so)
> > 
> >   You can now use it in all perf tools, such as:
> > 
> >           perf record -e sdt_libc:memory_heap_new -aR sleep 1
> > 
> >   # perf probe -l
> >     sdt_libc:memory_heap_new (on new_heap+183 in /usr/lib/libc-2.17.so)
> >   ----
> > 
> > Note that SDT event entries in probe-cache file is somewhat different
> > from normal cached events. Normal one starts with "#", but SDTs are
> > starting with "%".
> 
> Is this still the case?
> 
> [root@jouet perf]# perf probe --cache --list
> /usr/lib64/libc-2.23.so (88686319c72f1a9d9cd514af519aa5602880bab2):
> sdt_libc:setjmp=setjmp
> sdt_libc:longjmp=longjmp
> sdt_libc:longjmp_target=longjmp_target
> sdt_libc:memory_arena_reuse_free_list=memory_arena_reuse_free_list
> sdt_libc:memory_heap_new=memory_heap_new
> sdt_libc:memory_sbrk_less=memory_sbrk_less
> sdt_libc:memory_arena_reuse_wait=memory_arena_reuse_wait
> sdt_libc:memory_arena_reuse=memory_arena_reuse
> sdt_libc:memory_arena_new=memory_arena_new
> sdt_libc:memory_arena_retry=memory_arena_retry
> sdt_libc:memory_heap_free=memory_heap_free
> sdt_libc:memory_heap_less=memory_heap_less
> sdt_libc:memory_heap_more=memory_heap_more
> sdt_libc:memory_sbrk_more=memory_sbrk_more
> sdt_libc:memory_malloc_retry=memory_malloc_retry
> sdt_libc:memory_mallopt_free_dyn_thresholds=memory_mallopt_free_dyn_thresholds
> sdt_libc:memory_realloc_retry=memory_realloc_retry
> sdt_libc:memory_memalign_retry=memory_memalign_retry
> sdt_libc:memory_calloc_retry=memory_calloc_retry
> sdt_libc:memory_mallopt=memory_mallopt
> sdt_libc:memory_mallopt_mxfast=memory_mallopt_mxfast
> sdt_libc:memory_mallopt_arena_max=memory_mallopt_arena_max
> sdt_libc:memory_mallopt_arena_test=memory_mallopt_arena_test
> sdt_libc:memory_mallopt_mmap_max=memory_mallopt_mmap_max
> sdt_libc:memory_mallopt_mmap_threshold=memory_mallopt_mmap_threshold
> sdt_libc:memory_mallopt_top_pad=memory_mallopt_top_pad
> sdt_libc:memory_mallopt_trim_threshold=memory_mallopt_trim_threshold
> sdt_libc:memory_mallopt_perturb=memory_mallopt_perturb
> sdt_libc:memory_mallopt_check_action=memory_mallopt_check_action
> sdt_libc:lll_lock_wait_private=lll_lock_wait_private
> [root@jouet perf]# 
> 
> 
>  
> > Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@xxxxxxxxxxx>
> > Signed-off-by: Masami Hiramatsu <mhiramat@xxxxxxxxxx>
> > ---
> >  Changes in v11:
> >   - Warn if SDT scanning/updating are failed.
> >  Changes in v10:
> >   - Update Documentation/perf-buildid-cache.txt too.
> >  Changes in v4:
> >   - Fix a bug to copy correct group name to entries.
> >   - Fix to consolidate same-name entries.
> > ---
> >  tools/perf/Documentation/perf-buildid-cache.txt |    3 +
> >  tools/perf/util/build-id.c                      |   30 ++++++++++
> >  tools/perf/util/probe-file.c                    |   69 ++++++++++++++++++++++-
> >  tools/perf/util/probe-file.h                    |    2 +
> >  4 files changed, 101 insertions(+), 3 deletions(-)
> > 
> > diff --git a/tools/perf/Documentation/perf-buildid-cache.txt b/tools/perf/Documentation/perf-buildid-cache.txt
> > index dd07b55..058064d 100644
> > --- a/tools/perf/Documentation/perf-buildid-cache.txt
> > +++ b/tools/perf/Documentation/perf-buildid-cache.txt
> > @@ -15,6 +15,9 @@ DESCRIPTION
> >  This command manages the build-id cache. It can add, remove, update and purge
> >  files to/from the cache. In the future it should as well set upper limits for
> >  the space used by the cache, etc.
> > +This also scans the target binary for SDT (Statically Defined Tracing) and
> > +record it along with the buildid-cache, which will be used by perf-probe.
> > +For more details, see linkperf:perf-probe[1].
> >  
> >  OPTIONS
> >  -------
> > diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
> > index 1c49620..e1a1640 100644
> > --- a/tools/perf/util/build-id.c
> > +++ b/tools/perf/util/build-id.c
> > @@ -17,6 +17,7 @@
> >  #include "tool.h"
> >  #include "header.h"
> >  #include "vdso.h"
> > +#include "probe-file.h"
> >  
> >  
> >  static bool no_buildid_cache;
> > @@ -532,6 +533,30 @@ int build_id_cache__list_build_ids(const char *pathname,
> >  	return ret;
> >  }
> >  
> > +#ifdef HAVE_LIBELF_SUPPORT
> > +static int build_id_cache__add_sdt_cache(const char *sbuild_id,
> > +					  const char *realname)
> > +{
> > +	struct probe_cache *cache;
> > +	int ret;
> > +
> > +	cache = probe_cache__new(sbuild_id);
> > +	if (!cache)
> > +		return -1;
> > +
> > +	ret = probe_cache__scan_sdt(cache, realname);
> > +	if (ret >= 0) {
> > +		pr_debug("Found %d SDTs in %s\n", ret, realname);
> > +		if (probe_cache__commit(cache) < 0)
> > +			ret = -1;
> > +	}
> > +	probe_cache__delete(cache);
> > +	return ret;
> > +}
> > +#else
> > +#define build_id_cache__add_sdt_cache(sbuild_id, realname) (0)
> > +#endif
> > +
> >  int build_id_cache__add_s(const char *sbuild_id, const char *name,
> >  			  bool is_kallsyms, bool is_vdso)
> >  {
> > @@ -589,6 +614,11 @@ int build_id_cache__add_s(const char *sbuild_id, const char *name,
> >  
> >  	if (symlink(tmp, linkname) == 0)
> >  		err = 0;
> > +
> > +	/* Update SDT cache : error is just warned */
> > +	if (build_id_cache__add_sdt_cache(sbuild_id, realname) < 0)
> > +		pr_debug("Failed to update/scan SDT cache for %s\n", realname);
> > +
> >  out_free:
> >  	if (!is_kallsyms)
> >  		free(realname);
> > diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
> > index 6cb6ec0..5b563b2 100644
> > --- a/tools/perf/util/probe-file.c
> > +++ b/tools/perf/util/probe-file.c
> > @@ -434,12 +434,15 @@ static int probe_cache__load(struct probe_cache *pcache)
> >  		p = strchr(buf, '\n');
> >  		if (p)
> >  			*p = '\0';
> > -		if (buf[0] == '#') {	/* #perf_probe_event */
> > +		/* #perf_probe_event or %sdt_event */
> > +		if (buf[0] == '#' || buf[0] == '%') {
> >  			entry = probe_cache_entry__new(NULL);
> >  			if (!entry) {
> >  				ret = -ENOMEM;
> >  				goto out;
> >  			}
> > +			if (buf[0] == '%')
> > +				entry->sdt = true;
> >  			entry->spev = strdup(buf + 1);
> >  			if (entry->spev)
> >  				ret = parse_perf_probe_command(buf + 1,
> > @@ -621,19 +624,79 @@ out_err:
> >  	return ret;
> >  }
> >  
> > +static unsigned long long sdt_note__get_addr(struct sdt_note *note)
> > +{
> > +	return note->bit32 ? (unsigned long long)note->addr.a32[0]
> > +		 : (unsigned long long)note->addr.a64[0];
> > +}
> > +
> > +int probe_cache__scan_sdt(struct probe_cache *pcache, const char *pathname)
> > +{
> > +	struct probe_cache_entry *entry = NULL;
> > +	struct list_head sdtlist;
> > +	struct sdt_note *note;
> > +	char *buf;
> > +	char sdtgrp[64];
> > +	int ret;
> > +
> > +	INIT_LIST_HEAD(&sdtlist);
> > +	ret = get_sdt_note_list(&sdtlist, pathname);
> > +	if (ret < 0) {
> > +		pr_debug("Failed to get sdt note: %d\n", ret);
> > +		return ret;
> > +	}
> > +	list_for_each_entry(note, &sdtlist, note_list) {
> > +		ret = snprintf(sdtgrp, 64, "sdt_%s", note->provider);
> > +		if (ret < 0)
> > +			break;
> > +		/* Try to find same-name entry */
> > +		entry = probe_cache__find_by_name(pcache, sdtgrp, note->name);
> > +		if (!entry) {
> > +			entry = probe_cache_entry__new(NULL);
> > +			if (!entry) {
> > +				ret = -ENOMEM;
> > +				break;
> > +			}
> > +			entry->sdt = true;
> > +			ret = asprintf(&entry->spev, "%s:%s=%s", sdtgrp,
> > +					note->name, note->name);
> > +			if (ret < 0)
> > +				break;
> > +			entry->pev.event = strdup(note->name);
> > +			entry->pev.group = strdup(sdtgrp);
> > +			list_add_tail(&entry->node, &pcache->entries);
> > +		}
> > +		ret = asprintf(&buf, "p:%s/%s %s:0x%llx",
> > +				sdtgrp, note->name, pathname,
> > +				sdt_note__get_addr(note));
> > +		if (ret < 0)
> > +			break;
> > +		strlist__add(entry->tevlist, buf);
> > +		free(buf);
> > +		entry = NULL;
> > +	}
> > +	if (entry) {
> > +		list_del_init(&entry->node);
> > +		probe_cache_entry__delete(entry);
> > +	}
> > +	cleanup_sdt_note_list(&sdtlist);
> > +	return ret;
> > +}
> > +
> >  static int probe_cache_entry__write(struct probe_cache_entry *entry, int fd)
> >  {
> >  	struct str_node *snode;
> >  	struct stat st;
> >  	struct iovec iov[3];
> > +	const char *prefix = entry->sdt ? "%" : "#";
> >  	int ret;
> >  	/* Save stat for rollback */
> >  	ret = fstat(fd, &st);
> >  	if (ret < 0)
> >  		return ret;
> >  
> > -	pr_debug("Writing cache: #%s\n", entry->spev);
> > -	iov[0].iov_base = (void *)"#"; iov[0].iov_len = 1;
> > +	pr_debug("Writing cache: %s%s\n", prefix, entry->spev);
> > +	iov[0].iov_base = (void *)prefix; iov[0].iov_len = 1;
> >  	iov[1].iov_base = entry->spev; iov[1].iov_len = strlen(entry->spev);
> >  	iov[2].iov_base = (void *)"\n"; iov[2].iov_len = 1;
> >  	ret = writev(fd, iov, 3);
> > diff --git a/tools/perf/util/probe-file.h b/tools/perf/util/probe-file.h
> > index 0ed1fc5..ddf5ae2 100644
> > --- a/tools/perf/util/probe-file.h
> > +++ b/tools/perf/util/probe-file.h
> > @@ -8,6 +8,7 @@
> >  /* Cache of probe definitions */
> >  struct probe_cache_entry {
> >  	struct list_head	node;
> > +	bool			sdt;
> >  	struct perf_probe_event pev;
> >  	char			*spev;
> >  	struct strlist		*tevlist;
> > @@ -35,6 +36,7 @@ struct probe_cache *probe_cache__new(const char *target);
> >  int probe_cache__add_entry(struct probe_cache *pcache,
> >  			   struct perf_probe_event *pev,
> >  			   struct probe_trace_event *tevs, int ntevs);
> > +int probe_cache__scan_sdt(struct probe_cache *pcache, const char *pathname);
> >  int probe_cache__commit(struct probe_cache *pcache);
> >  void probe_cache__purge(struct probe_cache *pcache);
> >  void probe_cache__delete(struct probe_cache *pcache);


-- 
Masami Hiramatsu <mhiramat@xxxxxxxxxx>