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

Hi,

Here is the 6th version of of perf buildid-cache update. This updates the 1st and 2nd patches according to Arnaldo's comment.

Here are the changes in v6.

- [1/3] Add verbose error message if failed to list buildid-dir. (this also updates build_id_cache__list_build_ids())
- [2/3] Update for the previous change.

Thank you,

---

Masami Hiramatsu (3):
perf buildid-cache: Add --purge FILE to remove all caches of FILE
perf-buildid-cache: Use pr_debug instead of verbose && pr_info
perf-buildid-cache: Show usage with incorrect params

 tools/perf/Documentation/perf-buildid-cache.txt |   13 ++-
 tools/perf/builtin-buildid-cache.c              |   66 ++++++++++++++--
 tools/perf/util/build-id.c                      |   93 +++++++++++++++++++----
 tools/perf/util/build-id.h                      |    3 +
 4 files changed, 145 insertions(+), 30 deletions(-)

--
Masami HIRAMATSU
Software Platform Research Dpt. Linux Technology Center Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]

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/* .

---
 tools/perf/Documentation/perf-buildid-cache.txt |   13 ++-
 tools/perf/builtin-buildid-cache.c              |   48 ++++++++++++
 tools/perf/util/build-id.c                      |   93 +++++++++++++++++++----
 tools/perf/util/build-id.h                      |    3 +
 4 files changed, 136 insertions(+), 21 deletions(-)

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.

index 2a09498..8501122 100644
@@ -4,6 +4,7 @@
 #define BUILD_ID_SIZE 20
 
 #include "tool.h"
+#include "strlist.h"
 #include <linux/types.h>
 
 extern struct perf_tool build_id__mark_dso_hit_ops;

@@ -22,6 +23,8 @@ 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);
 
+int build_id_cache__list_build_ids(const char *pathname,
+				   struct strlist **result);
 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);

Use pr_debug instead of the combination of verbose and pr_info.

"if (verbose) pr_info(...)" is same as "pr_debug(...)", replace it.

---
 tools/perf/builtin-buildid-cache.c |   23 +++++++++--------------
 1 file changed, 9 insertions(+), 14 deletions(-)

index 86f9d78..04466c4 100644
@@ -196,9 +196,8 @@ static int build_id_cache__add_file(const char *filename)
 	build_id__sprintf(build_id, sizeof(build_id), sbuild_id);
 	err = build_id_cache__add_s(sbuild_id, filename,
 				    false, false);
-	if (verbose)
-		pr_info("Adding %s %s: %s\n", sbuild_id, filename,
-			err ? "FAIL" : "Ok");
+	pr_debug("Adding %s %s: %s\n", sbuild_id, filename,
+		 err ? "FAIL" : "Ok");
 	return err;
 }
 

@@ -216,9 +215,8 @@ static int build_id_cache__remove_file(const char *filename)
 
 	build_id__sprintf(build_id, sizeof(build_id), sbuild_id);
 	err = build_id_cache__remove_s(sbuild_id);
-	if (verbose)
-		pr_info("Removing %s %s: %s\n", sbuild_id, filename,
-			err ? "FAIL" : "Ok");
+	pr_debug("Removing %s %s: %s\n", sbuild_id, filename,
+		 err ? "FAIL" : "Ok");
 
 	return err;
 }

@@ -235,17 +233,15 @@ static int build_id_cache__purge_path(const char *pathname)
 
 	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");
+		pr_debug("Removing %s %s: %s\n", pos->s, pathname,
+			 err ? "FAIL" : "Ok");
 		if (err)
 			break;
 	}
 	strlist__delete(list);
 
 out:
-	if (verbose)
-		pr_info("Purging %s: %s\n", pathname, err ? "FAIL" : "Ok");
+	pr_debug("Purging %s: %s\n", pathname, err ? "FAIL" : "Ok");
 
 	return err;
 }

@@ -296,9 +292,8 @@ static int build_id_cache__update_file(const char *filename)
 	if (!err)
 		err = build_id_cache__add_s(sbuild_id, filename, false, false);
 
-	if (verbose)
-		pr_info("Updating %s %s: %s\n", sbuild_id, filename,
-			err ? "FAIL" : "Ok");
+	pr_debug("Updating %s %s: %s\n", sbuild_id, filename,
+		 err ? "FAIL" : "Ok");
 
 	return err;
 }

Show usage if no action is specified or unexpected parameter is given. In other words, be more user friendly.

---
 tools/perf/builtin-buildid-cache.c |    5 +++++
 1 file changed, 5 insertions(+)

index 04466c4..d47a0cd 100644
@@ -343,6 +343,11 @@ int cmd_buildid_cache(int argc, const char **argv,
 	argc = parse_options(argc, argv, buildid_cache_options,
 			     buildid_cache_usage, 0);
 
+	if (argc || (!add_name_list_str && !kcore_filename &&
+		     !remove_name_list_str && !purge_name_list_str &&
+		     !missing_filename && !update_name_list_str))
+		usage_with_options(buildid_cache_usage, buildid_cache_options);
+
 	if (missing_filename) {
 		file.path = missing_filename;
 		file.force = force;

Em Fri, Feb 27, 2015 at 01:50:24PM +0900, Masami Hiramatsu escreveu:

Applied, that silent failure when strlist__new() fails was there for other string list arguments, will fix that in a separate patch...

- Arnaldo