Re: [perf/core PATCH v2 4/4] perf buildid-cache: Add --remove-all FILE to remove all caches of FILE

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

(2015/02/12 0:00), Arnaldo Carvalho de Melo wrote:
> Em Tue, Feb 10, 2015 at 06:18:58PM +0900, Masami Hiramatsu escreveu:
>> Add --remove-all FILE to remove all caches of FILE.
> 
> I like --purge better, shorter form, like in 'man fpurge'

Thanks for good suggestion :)
I'll update this.


Thank you,


> 
> - Arnaldo
> 
>> 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 --remove-all FILE action for such usecase.
>> perf buildid-cache --remove-all 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 --remove-all ./perf
>>   Removing 133b7b5486d987a5ab5c3ebf4ea14941f45d4d4f ./perf: Ok
>>   -----
>>
>> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>> ---
>>  tools/perf/Documentation/perf-buildid-cache.txt |    6 +-
>>  tools/perf/builtin-buildid-cache.c              |   44 ++++++++++++
>>  tools/perf/util/build-id.c                      |   83 +++++++++++++++++++----
>>  tools/perf/util/build-id.h                      |    1 
>>  4 files changed, 117 insertions(+), 17 deletions(-)
>>
>> diff --git a/tools/perf/Documentation/perf-buildid-cache.txt b/tools/perf/Documentation/perf-buildid-cache.txt
>> index 6575dce..01f295e 100644
>> --- a/tools/perf/Documentation/perf-buildid-cache.txt
>> +++ b/tools/perf/Documentation/perf-buildid-cache.txt
>> @@ -36,7 +36,11 @@ 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.
>> +-R::
>> +--remove-all=::
>> +        Remove all cached binary which has 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..3a76d51 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__remove_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,
>> +		   *remove_all_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('R', "remove-all", &remove_all_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 (remove_all_name_list_str) {
>> +		list = strlist__new(true, remove_all_name_list_str);
>> +		if (list) {
>> +			strlist__for_each(pos, list)
>> +				if (build_id_cache__remove_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) {
>> +		filename = NULL;
>> +		goto out_free;
>> +	}
>>  
>>  	if (access(filename, F_OK)) {
>>  		if (is_kallsyms) {
>> diff --git a/tools/perf/util/build-id.h b/tools/perf/util/build-id.h
>> index 2a09498..cbcadea 100644
>> --- a/tools/perf/util/build-id.h
>> +++ b/tools/perf/util/build-id.h
>> @@ -22,6 +22,7 @@ bool perf_session__read_build_ids(struct perf_session *session, bool with_hits);
>>  int perf_session__write_buildid_table(struct perf_session *session, int fd);
>>  int perf_session__cache_build_ids(struct perf_session *session);
>>  
>> +struct strlist *build_id_cache__list_build_ids(const char *pathname);
>>  bool build_id_cache__cached(const char *sbuild_id);
>>  int build_id_cache__add_s(const char *sbuild_id,
>>  			  const char *name, bool is_kallsyms, bool is_vdso);
> 


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