perf-buildid-cache: Enhance --update and add --purge

[SNIP]

tools/perf/util/build-id.c +21 -0

+
+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);

tools/perf/util/build-id.c +17 -17

+	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, *dirname = 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)
+	dirname = build_id_cache__dirname_from_path(name, is_kallsyms, is_vdso);
+	if (!dirname)
  		goto out_free;
 
-	len = scnprintf(filename, size, "%s%s%s",
-		       buildid_dir, slash ? "/" : "",
-		       is_vdso ? DSO__NAME_VDSO : realname);
-	if (mkdir_p(filename, 0755))
+	if (mkdir_p(dirname, 0755))
  		goto out_free;
 
-	snprintf(filename + len, size - len, "/%s", sbuild_id);
+	if (asprintf(&filename, "%s/%s", dirname, sbuild_id) < 0) {
+		filename = NULL;
+		goto out_free;
+	}
 
  	if (access(filename, F_OK)) {
  		if (is_kallsyms) {

@@ -337,6 +388,7 @@ out_free:
  	if (!is_kallsyms)
  		free(realname);
  	free(filename);
+	free(dirname);
  	free(linkname);
  	return err;
  }

tools/perf/util/build-id.h +1 -0

index 2a09498..cbcadea 100644
@@ -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);

Hi Masami,

Just a small suggestion below.

Quoted 27 lines · Masami Hiramatsu

[SNIP]

tools/perf/util/build-id.c +26 -0

+
+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;

tools/perf/util/build-id.c void disable_buildid_cache(void) · line 335

Quoted 5 lines

tools/perf/util/build-id.c +5 -0

+
+error_free:
+	free(dirname);
+	if (list)
+		strlist__delete(list);

Maybe we don't need the "if (list)" check here as strlist__delete already checks for this.

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 <[email protected]>

---
 tools/perf/Documentation/perf-buildid-cache.txt |   13 ++-
 tools/perf/builtin-buildid-cache.c              |   44 ++++++++++++
 tools/perf/util/build-id.c                      |   86 ++++++++++++++++++-----
 tools/perf/util/build-id.h                      |    1 
 4 files changed, 123 insertions(+), 21 deletions(-)

tools/perf/Documentation/perf-buildid-cache.txt +9 -4

index cec6b57..dd07b55 100644
@@ -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.

tools/perf/builtin-buildid-cache.c +5 -0

index e7568f5..37182bb 100644
@@ -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;

tools/perf/builtin-buildid-cache.c +8 -0

+		if (err)
+			break;
+	}
+	strlist__delete(list);
+
+	return err;
+}
+

Hi Masami,

Quoted 29 lines · Masami Hiramatsu

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
  -----

Quoted 3 lines

BTW, if you want to purge all the caches, remove ~/.debug/* .

Signed-off-by Masami Hiramatsu <[email protected]>

I have a nitpick below - other than that both patches look good.

Acked-by Namhyung Kim <[email protected]>

Thanks,
Namhyung

Quoted 9 lines

tools/perf/builtin-buildid-cache.c +9 -0

+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;

tools/perf/builtin-buildid-cache.c static int build_id_cache__remove_file(const char *filename) · line 240

Quoted 6 lines

tools/perf/builtin-buildid-cache.c +6 -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");

You can simply use pr_debug() here. :)

Thanks,
Namhyung

Quoted 8 lines

tools/perf/builtin-buildid-cache.c +8 -0

+		if (err)
+			break;
+	}
+	strlist__delete(list);
+
+	return err;
+}
+