Re: [PATCH perf/core 03/10] perf-probe: Make --list shows only available cached events

[Arnaldo Carvalho de Melo]

Em Tue, Jul 12, 2016 at 07:04:54PM +0900, Masami Hiramatsu escreveu:
> From: Masami Hiramatsu <masami.hiramatsu.pt@xxxxxxxxxxx>
> 
> Make "perf probe --cache --list" shows only available cached events
> by checking build-id validity.

Something is missing here, I tried following these steps and couldn't
get to the same results, can you retry them at this point in time, i.e.
after applying just the three first patches in this series?

I'm continuing anyway, as I want to check the rest...

- Arnaldo
 
> E.g. without this patch:
>   ----
>   $ ./perf probe --cache --add oldevent=cmd_probe
>   $ make #(to update ./perf)
>   $ ./perf probe --cache --add newevent=cmd_probe
>   $ ./perf probe --cache --list
>   /home/mhiramat/ksrc/linux/tools/perf/perf (061e90539bac69
>   probe_perf:newevent=cmd_probe
>   /home/mhiramat/ksrc/linux/tools/perf/perf (c2e44d614e33e1
>   probe_perf:oldevent=cmd_probe
>   ----
> It shows both of old and new events but user can not use old one.
> With this;
>   ----
>   $ ./perf probe --cache -l
>   /home/mhiramat/ksrc/linux/tools/perf/perf (061e90539bac69
>   probe_perf:newevent=cmd_probe
>   ----
> This show only new event which is on the existing binary.
> 
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@xxxxxxxxxxx>
> Signed-off-by: Masami Hiramatsu <mhiramat@xxxxxxxxxx>
> ---
>  Changes in v13:
>   - Instead of the perf-list, apply this change for perf-probe --list
>     because of avoiding to confuse user.
>  Changes in v7:
>   - Validate build-id via sysfs if it is for kallsyms,
>  Changes in v4:
>   - Rename a parameter 'valid' to 'validonly' :)
> ---
>  tools/perf/builtin-probe.c   |    2 +-
>  tools/perf/util/build-id.c   |   33 ++++++++++++++++++++++++++++++++-
>  tools/perf/util/build-id.h   |    2 +-
>  tools/perf/util/probe-file.c |    2 +-
>  4 files changed, 35 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/perf/builtin-probe.c b/tools/perf/builtin-probe.c
> index a1a5cd1..de75af5 100644
> --- a/tools/perf/builtin-probe.c
> +++ b/tools/perf/builtin-probe.c
> @@ -370,7 +370,7 @@ static int del_perf_probe_caches(struct strfilter *filter)
>  	struct str_node *nd;
>  	int ret;
>  
> -	bidlist = build_id_cache__list_all();
> +	bidlist = build_id_cache__list_all(false);
>  	if (!bidlist) {
>  		ret = -errno;
>  		pr_debug("Failed to get buildids: %d\n", ret);
> diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
> index e1a1640..8e86a83 100644
> --- a/tools/perf/util/build-id.c
> +++ b/tools/perf/util/build-id.c
> @@ -206,6 +206,31 @@ out:
>  	return ret;
>  }
>  
> +/* Check if the given build_id cache is valid on current running system */
> +static bool build_id_cache__valid_id(char *sbuild_id)
> +{
> +	char real_sbuild_id[SBUILD_ID_SIZE] = "";
> +	char *pathname;
> +	int ret = 0;
> +	bool result = false;
> +
> +	pathname = build_id_cache__origname(sbuild_id);
> +	if (!pathname)
> +		return false;
> +
> +	if (!strcmp(pathname, DSO__NAME_KALLSYMS))
> +		ret = sysfs__sprintf_build_id("/", real_sbuild_id);
> +	else if (pathname[0] == '/')
> +		ret = filename__sprintf_build_id(pathname, real_sbuild_id);
> +	else
> +		ret = -EINVAL;	/* Should we support other special DSO cache? */
> +	if (ret >= 0)
> +		result = (strcmp(sbuild_id, real_sbuild_id) == 0);
> +	free(pathname);
> +
> +	return result;
> +}
> +
>  static const char *build_id_cache__basename(bool is_kallsyms, bool is_vdso)
>  {
>  	return is_kallsyms ? "kallsyms" : (is_vdso ? "vdso" : "elf");
> @@ -433,13 +458,17 @@ static bool lsdir_bid_tail_filter(const char *name __maybe_unused,
>  	return (i == SBUILD_ID_SIZE - 3) && (d->d_name[i] == '\0');
>  }
>  
> -struct strlist *build_id_cache__list_all(void)
> +struct strlist *build_id_cache__list_all(bool validonly)
>  {
>  	struct strlist *toplist, *linklist = NULL, *bidlist;
>  	struct str_node *nd, *nd2;
>  	char *topdir, *linkdir = NULL;
>  	char sbuild_id[SBUILD_ID_SIZE];
>  
> +	/* for filename__ functions */
> +	if (validonly)
> +		symbol__init(NULL);
> +
>  	/* Open the top-level directory */
>  	if (asprintf(&topdir, "%s/.build-id/", buildid_dir) < 0)
>  		return NULL;
> @@ -470,6 +499,8 @@ struct strlist *build_id_cache__list_all(void)
>  			if (snprintf(sbuild_id, SBUILD_ID_SIZE, "%s%s",
>  				     nd->s, nd2->s) != SBUILD_ID_SIZE - 1)
>  				goto err_out;
> +			if (validonly && !build_id_cache__valid_id(sbuild_id))
> +				continue;
>  			if (strlist__add(bidlist, sbuild_id) < 0)
>  				goto err_out;
>  		}
> diff --git a/tools/perf/util/build-id.h b/tools/perf/util/build-id.h
> index b742e27..64e740f 100644
> --- a/tools/perf/util/build-id.h
> +++ b/tools/perf/util/build-id.h
> @@ -34,7 +34,7 @@ char *build_id_cache__origname(const char *sbuild_id);
>  char *build_id_cache__linkname(const char *sbuild_id, char *bf, size_t size);
>  char *build_id_cache__cachedir(const char *sbuild_id, const char *name,
>  			       bool is_kallsyms, bool is_vdso);
> -struct strlist *build_id_cache__list_all(void);
> +struct strlist *build_id_cache__list_all(bool validonly);
>  int build_id_cache__list_build_ids(const char *pathname,
>  				   struct strlist **result);
>  bool build_id_cache__cached(const char *sbuild_id);
> diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
> index b642d06..b062545 100644
> --- a/tools/perf/util/probe-file.c
> +++ b/tools/perf/util/probe-file.c
> @@ -806,7 +806,7 @@ int probe_cache__show_all_caches(struct strfilter *filter)
>  	pr_debug("list cache with filter: %s\n", buf);
>  	free(buf);
>  
> -	bidlist = build_id_cache__list_all();
> +	bidlist = build_id_cache__list_all(true);
>  	if (!bidlist) {
>  		pr_debug("Failed to get buildids: %d\n", errno);
>  		return -EINVAL;