Re: [perf/core PATCH v3 2/2] perf buildid-cache: Add --purge FILE to remove all caches of FILE

[Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>]

(2015/02/19 17:08), Masami Hiramatsu wrote:
> Add --purge FILE to remove all caches of FILE.
> Since the current --remove FILE removes a cache which has
> same build-id of given FILE. Since the command takes a
> FILE path, it can confuse user who tries to remove cache
> about FILE path.
> 
>   -----
>   # ./perf buildid-cache -v --add ./perf
>   Adding 133b7b5486d987a5ab5c3ebf4ea14941f45d4d4f ./perf: Ok
>   # (update the ./perf binary)
>   # ./perf buildid-cache -v --remove ./perf
>   Removing 305bbd1be68f66eca7e2d78db294653031edfa79 ./perf: FAIL
>   ./perf wasn't in the cache
>   -----
> Actually, the --remove's FAIL is not shown, it just silently fails.
> 
> So, this patch adds --purge FILE action for such usecase.
> perf buildid-cache --purge FILE removes all caches which
> has same FILE path.
> In other words, it removes all caches including old binaries.
> 
>   -----
>   # ./perf buildid-cache -v --add ./perf
>   Adding 133b7b5486d987a5ab5c3ebf4ea14941f45d4d4f ./perf: Ok
>   # (update the ./perf binary)
>   # ./perf buildid-cache -v --purge ./perf
>   Removing 133b7b5486d987a5ab5c3ebf4ea14941f45d4d4f ./perf: Ok
>   -----
> 
> BTW, if you want to purge all the caches, remove ~/.debug/* .
> 
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> ---
>  tools/perf/Documentation/perf-buildid-cache.txt |   13 ++--
>  tools/perf/builtin-buildid-cache.c              |   44 ++++++++++++
>  tools/perf/util/build-id.c                      |   83 +++++++++++++++++++----
>  tools/perf/util/build-id.h                      |    1 
>  4 files changed, 121 insertions(+), 20 deletions(-)
> 
> diff --git a/tools/perf/Documentation/perf-buildid-cache.txt b/tools/perf/Documentation/perf-buildid-cache.txt
> index cec6b57..dd07b55 100644
> --- a/tools/perf/Documentation/perf-buildid-cache.txt
> +++ b/tools/perf/Documentation/perf-buildid-cache.txt
> @@ -12,9 +12,9 @@ SYNOPSIS
>  
>  DESCRIPTION
>  -----------
> -This command manages the build-id cache. It can add and remove files to/from
> -the cache. In the future it should as well purge older entries, set upper
> -limits for the space used by the cache, etc.
> +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.
>  
>  OPTIONS
>  -------
> @@ -36,7 +36,12 @@ OPTIONS
>          actually made.
>  -r::
>  --remove=::
> -        Remove specified file from the cache.
> +        Remove a cached binary which has same build-id of specified file
> +        from the cache.
> +-p::
> +--purge=::
> +        Purge all cached binaries including older caches which have specified
> +	path from the cache.
>  -M::
>  --missing=::
>  	List missing build ids in the cache for the specified file.
> diff --git a/tools/perf/builtin-buildid-cache.c b/tools/perf/builtin-buildid-cache.c
> index e7568f5..37182bb 100644
> --- a/tools/perf/builtin-buildid-cache.c
> +++ b/tools/perf/builtin-buildid-cache.c
> @@ -223,6 +223,29 @@ static int build_id_cache__remove_file(const char *filename)
>  	return err;
>  }
>  
> +static int build_id_cache__purge_path(const char *pathname)
> +{
> +	struct strlist *list;
> +	struct str_node *pos;
> +	int err;
> +
> +	list = build_id_cache__list_build_ids(pathname);
> +	if (!list)
> +		return 0;
> +
> +	strlist__for_each(pos, list) {
> +		err = build_id_cache__remove_s(pos->s);
> +		if (verbose)
> +			pr_info("Removing %s %s: %s\n", pos->s, pathname,
> +				err ? "FAIL" : "Ok");
> +		if (err)
> +			break;
> +	}
> +	strlist__delete(list);
> +
> +	return err;
> +}
> +
>  static bool dso__missing_buildid_cache(struct dso *dso, int parm __maybe_unused)
>  {
>  	char filename[PATH_MAX];
> @@ -285,6 +308,7 @@ int cmd_buildid_cache(int argc, const char **argv,
>  	bool force = false;
>  	char const *add_name_list_str = NULL,
>  		   *remove_name_list_str = NULL,
> +		   *purge_name_list_str = NULL,
>  		   *missing_filename = NULL,
>  		   *update_name_list_str = NULL,
>  		   *kcore_filename = NULL;
> @@ -302,6 +326,8 @@ int cmd_buildid_cache(int argc, const char **argv,
>  		   "file", "kcore file to add"),
>  	OPT_STRING('r', "remove", &remove_name_list_str, "file list",
>  		    "file(s) to remove"),
> +	OPT_STRING('p', "purge", &purge_name_list_str, "path list",
> +		    "path(s) to remove (remove old caches too)"),
>  	OPT_STRING('M', "missing", &missing_filename, "file",
>  		   "to find missing build ids in the cache"),
>  	OPT_BOOLEAN('f', "force", &force, "don't complain, do it"),
> @@ -368,6 +394,24 @@ int cmd_buildid_cache(int argc, const char **argv,
>  		}
>  	}
>  
> +	if (purge_name_list_str) {
> +		list = strlist__new(true, purge_name_list_str);
> +		if (list) {
> +			strlist__for_each(pos, list)
> +				if (build_id_cache__purge_path(pos->s)) {
> +					if (errno == ENOENT) {
> +						pr_debug("%s wasn't in the cache\n",
> +							 pos->s);
> +						continue;
> +					}
> +					pr_warning("Couldn't remove %s: %s\n",
> +						   pos->s, strerror_r(errno, sbuf, sizeof(sbuf)));
> +				}
> +
> +			strlist__delete(list);
> +		}
> +	}
> +
>  	if (missing_filename)
>  		ret = build_id_cache__fprintf_missing(session, stdout);
>  
> diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
> index 0bc33be..b664b06 100644
> --- a/tools/perf/util/build-id.c
> +++ b/tools/perf/util/build-id.c
> @@ -281,35 +281,86 @@ void disable_buildid_cache(void)
>  	no_buildid_cache = true;
>  }
>  
> +static char *build_id_cache__dirname_from_path(const char *name,
> +					       bool is_kallsyms, bool is_vdso)
> +{
> +	char *realname = (char *)name, *filename;
> +	bool slash = is_kallsyms || is_vdso;
> +
> +	if (!slash) {
> +		realname = realpath(name, NULL);
> +		if (!realname)
> +			return NULL;
> +	}
> +
> +	if (asprintf(&filename, "%s%s%s", buildid_dir, slash ? "/" : "",
> +		     is_vdso ? DSO__NAME_VDSO : realname) < 0)
> +		filename = NULL;
> +
> +	if (!slash)
> +		free(realname);
> +
> +	return filename;
> +}
> +
> +struct strlist *build_id_cache__list_build_ids(const char *pathname)
> +{
> +	struct strlist *list;
> +	char *dirname;
> +	DIR *dir;
> +	struct dirent *d;
> +
> +	list = strlist__new(true, NULL);
> +	dirname = build_id_cache__dirname_from_path(pathname, false, false);
> +	if (!list || !dirname)
> +		goto error_free;
> +
> +	/* List up all dirents */
> +	dir = opendir(dirname);
> +	if (!dir)
> +		goto error_free;
> +	while ((d = readdir(dir)) != NULL) {
> +		if (!strcmp(d->d_name, ".") || !strcmp(d->d_name, ".."))
> +			continue;
> +		strlist__add(list, d->d_name);
> +	}
> +	closedir(dir);
> +
> +	free(dirname);
> +	return list;
> +
> +error_free:
> +	free(dirname);
> +	if (list)
> +		strlist__delete(list);
> +	return NULL;
> +}
> +
>  int build_id_cache__add_s(const char *sbuild_id, const char *name,
>  			  bool is_kallsyms, bool is_vdso)
>  {
>  	const size_t size = PATH_MAX;
> -	char *realname, *filename = zalloc(size),
> +	char *realname = NULL, *filename = NULL,
>  	     *linkname = zalloc(size), *targetname, *tmp;
> -	int len, err = -1;
> -	bool slash = is_kallsyms || is_vdso;
> +	int err = -1;
>  
> -	if (is_kallsyms) {
> -		if (symbol_conf.kptr_restrict) {
> -			pr_debug("Not caching a kptr_restrict'ed /proc/kallsyms\n");
> -			err = 0;
> -			goto out_free;
> -		}
> -		realname = (char *) name;
> -	} else
> +	if (!is_kallsyms) {
>  		realname = realpath(name, NULL);
> +		if (!realname)
> +			goto out_free;
> +	}
>  
> -	if (realname == NULL || filename == NULL || linkname == NULL)
> +	filename = build_id_cache__dirname_from_path(name, is_kallsyms, is_vdso);
> +	if (!filename)
>  		goto out_free;
>  
> -	len = scnprintf(filename, size, "%s%s%s",
> -		       buildid_dir, slash ? "/" : "",
> -		       is_vdso ? DSO__NAME_VDSO : realname);
>  	if (mkdir_p(filename, 0755))
>  		goto out_free;
>  
> -	snprintf(filename + len, size - len, "/%s", sbuild_id);
> +	if (asprintf(&filename, "%s/%s", filename, sbuild_id) < 0) {

Oops, here is a memory leak. I'll update this.

Thank you,


-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com