[perf/core PATCH v2 0/4] perf-buildid-cache: Cleanup, enhance --update and add --remove-all

[perf/core PATCH v2 0/4] perf-buildid-cache: Cleanup, enhance --update and add --remove-all

[Masami Hiramatsu]

Hi,

Here is the 2nd version of of perf buildid-cache update.
I've fixed a bug in patch 4/4 reported by Namhyung, Thanks!

The first 2 patches are cleanup code, just reducing the
redundant code. The 3rd one modifies --update option not
to fail but just add new binary if there is no cache entry
which has same build-id of given binary. The 4th one adds
--remove-all option which allows us to cleanup all caches
which related to given path.


Thank you,


---

Masami Hiramatsu (4):
      perf-buildid-cache: Remove unneeded debugdir parameters
      perf buildid-cache: Consolidate .build-id cache path generators
      perf buildid-cache: Add new buildid cache if update target is not cached
      perf buildid-cache: Add --remove-all FILE to remove all caches of FILE


 tools/perf/Documentation/perf-buildid-cache.txt |    8 +
 tools/perf/builtin-buildid-cache.c              |   85 ++++++++--
 tools/perf/util/build-id.c                      |  189 +++++++++++++++++------
 tools/perf/util/build-id.h                      |    6 -
 4 files changed, 210 insertions(+), 78 deletions(-)

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

[perf/core PATCH v2 1/4] perf-buildid-cache: Remove unneeded debugdir parameters

[Masami Hiramatsu]

Functions related to buildid-cache subcommand use debugdir
parameters for passing buildid cache directory path. However
all callers just pass buildid_dir global variable. Moreover,
other functions which refer buildid cache use buildid_dir
directly.
This removes unneeded debugdir parameters from those functions
and use buildid_dir if needed.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
 tools/perf/builtin-buildid-cache.c |   37 +++++++++++++-----------------
 tools/perf/util/build-id.c         |   44 ++++++++++++++++--------------------
 tools/perf/util/build-id.h         |    4 ++-
 3 files changed, 38 insertions(+), 47 deletions(-)

diff --git a/tools/perf/builtin-buildid-cache.c b/tools/perf/builtin-buildid-cache.c
index 50e6b66..d929d95 100644
--- a/tools/perf/builtin-buildid-cache.c
+++ b/tools/perf/builtin-buildid-cache.c
@@ -125,8 +125,7 @@ static int build_id_cache__kcore_existing(const char *from_dir, char *to_dir,
 	return ret;
 }
 
-static int build_id_cache__add_kcore(const char *filename, const char *debugdir,
-				     bool force)
+static int build_id_cache__add_kcore(const char *filename, bool force)
 {
 	char dir[32], sbuildid[BUILD_ID_SIZE * 2 + 1];
 	char from_dir[PATH_MAX], to_dir[PATH_MAX];
@@ -143,7 +142,7 @@ static int build_id_cache__add_kcore(const char *filename, const char *debugdir,
 		return -1;
 
 	scnprintf(to_dir, sizeof(to_dir), "%s/[kernel.kcore]/%s",
-		  debugdir, sbuildid);
+		  buildid_dir, sbuildid);
 
 	if (!force &&
 	    !build_id_cache__kcore_existing(from_dir, to_dir, sizeof(to_dir))) {
@@ -155,7 +154,7 @@ static int build_id_cache__add_kcore(const char *filename, const char *debugdir,
 		return -1;
 
 	scnprintf(to_dir, sizeof(to_dir), "%s/[kernel.kcore]/%s/%s",
-		  debugdir, sbuildid, dir);
+		  buildid_dir, sbuildid, dir);
 
 	if (mkdir_p(to_dir, 0755))
 		return -1;
@@ -183,7 +182,7 @@ static int build_id_cache__add_kcore(const char *filename, const char *debugdir,
 	return 0;
 }
 
-static int build_id_cache__add_file(const char *filename, const char *debugdir)
+static int build_id_cache__add_file(const char *filename)
 {
 	char sbuild_id[BUILD_ID_SIZE * 2 + 1];
 	u8 build_id[BUILD_ID_SIZE];
@@ -195,7 +194,7 @@ static int build_id_cache__add_file(const char *filename, const char *debugdir)
 	}
 
 	build_id__sprintf(build_id, sizeof(build_id), sbuild_id);
-	err = build_id_cache__add_s(sbuild_id, debugdir, filename,
+	err = build_id_cache__add_s(sbuild_id, filename,
 				    false, false);
 	if (verbose)
 		pr_info("Adding %s %s: %s\n", sbuild_id, filename,
@@ -203,8 +202,7 @@ static int build_id_cache__add_file(const char *filename, const char *debugdir)
 	return err;
 }
 
-static int build_id_cache__remove_file(const char *filename,
-				       const char *debugdir)
+static int build_id_cache__remove_file(const char *filename)
 {
 	u8 build_id[BUILD_ID_SIZE];
 	char sbuild_id[BUILD_ID_SIZE * 2 + 1];
@@ -217,7 +215,7 @@ 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, debugdir);
+	err = build_id_cache__remove_s(sbuild_id);
 	if (verbose)
 		pr_info("Removing %s %s: %s\n", sbuild_id, filename,
 			err ? "FAIL" : "Ok");
@@ -252,8 +250,7 @@ static int build_id_cache__fprintf_missing(struct perf_session *session, FILE *f
 	return 0;
 }
 
-static int build_id_cache__update_file(const char *filename,
-				       const char *debugdir)
+static int build_id_cache__update_file(const char *filename)
 {
 	u8 build_id[BUILD_ID_SIZE];
 	char sbuild_id[BUILD_ID_SIZE * 2 + 1];
@@ -266,11 +263,10 @@ static int build_id_cache__update_file(const char *filename,
 	}
 
 	build_id__sprintf(build_id, sizeof(build_id), sbuild_id);
-	err = build_id_cache__remove_s(sbuild_id, debugdir);
-	if (!err) {
-		err = build_id_cache__add_s(sbuild_id, debugdir, filename,
-					    false, false);
-	}
+	err = build_id_cache__remove_s(sbuild_id);
+	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");
@@ -338,7 +334,7 @@ int cmd_buildid_cache(int argc, const char **argv,
 		list = strlist__new(true, add_name_list_str);
 		if (list) {
 			strlist__for_each(pos, list)
-				if (build_id_cache__add_file(pos->s, buildid_dir)) {
+				if (build_id_cache__add_file(pos->s)) {
 					if (errno == EEXIST) {
 						pr_debug("%s already in the cache\n",
 							 pos->s);
@@ -356,7 +352,7 @@ int cmd_buildid_cache(int argc, const char **argv,
 		list = strlist__new(true, remove_name_list_str);
 		if (list) {
 			strlist__for_each(pos, list)
-				if (build_id_cache__remove_file(pos->s, buildid_dir)) {
+				if (build_id_cache__remove_file(pos->s)) {
 					if (errno == ENOENT) {
 						pr_debug("%s wasn't in the cache\n",
 							 pos->s);
@@ -377,7 +373,7 @@ int cmd_buildid_cache(int argc, const char **argv,
 		list = strlist__new(true, update_name_list_str);
 		if (list) {
 			strlist__for_each(pos, list)
-				if (build_id_cache__update_file(pos->s, buildid_dir)) {
+				if (build_id_cache__update_file(pos->s)) {
 					if (errno == ENOENT) {
 						pr_debug("%s wasn't in the cache\n",
 							 pos->s);
@@ -391,8 +387,7 @@ int cmd_buildid_cache(int argc, const char **argv,
 		}
 	}
 
-	if (kcore_filename &&
-	    build_id_cache__add_kcore(kcore_filename, buildid_dir, force))
+	if (kcore_filename && build_id_cache__add_kcore(kcore_filename, force))
 		pr_warning("Couldn't add %s\n", kcore_filename);
 
 out:
diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index 0c72680..9f764f6 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -259,8 +259,8 @@ void disable_buildid_cache(void)
 	no_buildid_cache = true;
 }
 
-int build_id_cache__add_s(const char *sbuild_id, const char *debugdir,
-			  const char *name, bool is_kallsyms, bool is_vdso)
+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),
@@ -282,7 +282,7 @@ int build_id_cache__add_s(const char *sbuild_id, const char *debugdir,
 		goto out_free;
 
 	len = scnprintf(filename, size, "%s%s%s",
-		       debugdir, slash ? "/" : "",
+		       buildid_dir, slash ? "/" : "",
 		       is_vdso ? DSO__NAME_VDSO : realname);
 	if (mkdir_p(filename, 0755))
 		goto out_free;
@@ -298,13 +298,13 @@ int build_id_cache__add_s(const char *sbuild_id, const char *debugdir,
 	}
 
 	len = scnprintf(linkname, size, "%s/.build-id/%.2s",
-		       debugdir, sbuild_id);
+			buildid_dir, sbuild_id);
 
 	if (access(linkname, X_OK) && mkdir_p(linkname, 0755))
 		goto out_free;
 
 	snprintf(linkname + len, size - len, "/%s", sbuild_id + 2);
-	targetname = filename + strlen(debugdir) - 5;
+	targetname = filename + strlen(buildid_dir) - 5;
 	memcpy(targetname, "../..", 5);
 
 	if (symlink(targetname, linkname) == 0)
@@ -318,18 +318,17 @@ out_free:
 }
 
 static int build_id_cache__add_b(const u8 *build_id, size_t build_id_size,
-				 const char *name, const char *debugdir,
-				 bool is_kallsyms, bool is_vdso)
+				 const char *name, bool is_kallsyms,
+				 bool is_vdso)
 {
 	char sbuild_id[BUILD_ID_SIZE * 2 + 1];
 
 	build_id__sprintf(build_id, build_id_size, sbuild_id);
 
-	return build_id_cache__add_s(sbuild_id, debugdir, name,
-				     is_kallsyms, is_vdso);
+	return build_id_cache__add_s(sbuild_id, name, is_kallsyms, is_vdso);
 }
 
-int build_id_cache__remove_s(const char *sbuild_id, const char *debugdir)
+int build_id_cache__remove_s(const char *sbuild_id)
 {
 	const size_t size = PATH_MAX;
 	char *filename = zalloc(size),
@@ -340,7 +339,7 @@ int build_id_cache__remove_s(const char *sbuild_id, const char *debugdir)
 		goto out_free;
 
 	snprintf(linkname, size, "%s/.build-id/%.2s/%s",
-		 debugdir, sbuild_id, sbuild_id + 2);
+		 buildid_dir, sbuild_id, sbuild_id + 2);
 
 	if (access(linkname, F_OK))
 		goto out_free;
@@ -355,7 +354,7 @@ int build_id_cache__remove_s(const char *sbuild_id, const char *debugdir)
 	 * Since the link is relative, we must make it absolute:
 	 */
 	snprintf(linkname, size, "%s/.build-id/%.2s/%s",
-		 debugdir, sbuild_id, filename);
+		 buildid_dir, sbuild_id, filename);
 
 	if (unlink(linkname))
 		goto out_free;
@@ -367,8 +366,7 @@ out_free:
 	return err;
 }
 
-static int dso__cache_build_id(struct dso *dso, struct machine *machine,
-			       const char *debugdir)
+static int dso__cache_build_id(struct dso *dso, struct machine *machine)
 {
 	bool is_kallsyms = dso->kernel && dso->long_name[0] != '/';
 	bool is_vdso = dso__is_vdso(dso);
@@ -381,28 +379,26 @@ static int dso__cache_build_id(struct dso *dso, struct machine *machine,
 		name = nm;
 	}
 	return build_id_cache__add_b(dso->build_id, sizeof(dso->build_id), name,
-				     debugdir, is_kallsyms, is_vdso);
+				     is_kallsyms, is_vdso);
 }
 
 static int __dsos__cache_build_ids(struct list_head *head,
-				   struct machine *machine, const char *debugdir)
+				   struct machine *machine)
 {
 	struct dso *pos;
 	int err = 0;
 
 	dsos__for_each_with_build_id(pos, head)
-		if (dso__cache_build_id(pos, machine, debugdir))
+		if (dso__cache_build_id(pos, machine))
 			err = -1;
 
 	return err;
 }
 
-static int machine__cache_build_ids(struct machine *machine, const char *debugdir)
+static int machine__cache_build_ids(struct machine *machine)
 {
-	int ret = __dsos__cache_build_ids(&machine->kernel_dsos.head, machine,
-					  debugdir);
-	ret |= __dsos__cache_build_ids(&machine->user_dsos.head, machine,
-				       debugdir);
+	int ret = __dsos__cache_build_ids(&machine->kernel_dsos.head, machine);
+	ret |= __dsos__cache_build_ids(&machine->user_dsos.head, machine);
 	return ret;
 }
 
@@ -417,11 +413,11 @@ int perf_session__cache_build_ids(struct perf_session *session)
 	if (mkdir(buildid_dir, 0755) != 0 && errno != EEXIST)
 		return -1;
 
-	ret = machine__cache_build_ids(&session->machines.host, buildid_dir);
+	ret = machine__cache_build_ids(&session->machines.host);
 
 	for (nd = rb_first(&session->machines.guests); nd; nd = rb_next(nd)) {
 		struct machine *pos = rb_entry(nd, struct machine, rb_node);
-		ret |= machine__cache_build_ids(pos, buildid_dir);
+		ret |= machine__cache_build_ids(pos);
 	}
 	return ret ? -1 : 0;
 }
diff --git a/tools/perf/util/build-id.h b/tools/perf/util/build-id.h
index 8236319..31b3c63 100644
--- a/tools/perf/util/build-id.h
+++ b/tools/perf/util/build-id.h
@@ -22,9 +22,9 @@ 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__add_s(const char *sbuild_id, const char *debugdir,
+int build_id_cache__add_s(const char *sbuild_id,
 			  const char *name, bool is_kallsyms, bool is_vdso);
-int build_id_cache__remove_s(const char *sbuild_id, const char *debugdir);
+int build_id_cache__remove_s(const char *sbuild_id);
 void disable_buildid_cache(void);
 
 #endif

[tip:perf/core] perf buildid-cache: Remove unneeded debugdir parameters

[tip-bot for Masami Hiramatsu]

Commit-ID:  e35f7362bab455fb5c13ea4ce53f959f3e1610b2
Gitweb:     http://git.kernel.org/tip/e35f7362bab455fb5c13ea4ce53f959f3e1610b2
Author:     Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
AuthorDate: Tue, 10 Feb 2015 18:18:51 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 11 Feb 2015 12:37:33 -0300

perf buildid-cache: Remove unneeded debugdir parameters

Functions related to buildid-cache subcommand use debugdir parameters
for passing buildid cache directory path. However all callers just pass
buildid_dir global variable. Moreover, other functions which refer
buildid cache use buildid_dir directly.

This removes unneeded debugdir parameters from those functions and use
buildid_dir if needed.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: Hemant Kumar <hemant@linux.vnet.ibm.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/20150210091851.19264.72741.stgit@localhost.localdomain
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-buildid-cache.c | 37 ++++++++++++++------------------
 tools/perf/util/build-id.c         | 44 +++++++++++++++++---------------------
 tools/perf/util/build-id.h         |  4 ++--
 3 files changed, 38 insertions(+), 47 deletions(-)

diff --git a/tools/perf/builtin-buildid-cache.c b/tools/perf/builtin-buildid-cache.c
index 50e6b66..d929d95 100644
--- a/tools/perf/builtin-buildid-cache.c
+++ b/tools/perf/builtin-buildid-cache.c
@@ -125,8 +125,7 @@ static int build_id_cache__kcore_existing(const char *from_dir, char *to_dir,
 	return ret;
 }
 
-static int build_id_cache__add_kcore(const char *filename, const char *debugdir,
-				     bool force)
+static int build_id_cache__add_kcore(const char *filename, bool force)
 {
 	char dir[32], sbuildid[BUILD_ID_SIZE * 2 + 1];
 	char from_dir[PATH_MAX], to_dir[PATH_MAX];
@@ -143,7 +142,7 @@ static int build_id_cache__add_kcore(const char *filename, const char *debugdir,
 		return -1;
 
 	scnprintf(to_dir, sizeof(to_dir), "%s/[kernel.kcore]/%s",
-		  debugdir, sbuildid);
+		  buildid_dir, sbuildid);
 
 	if (!force &&
 	    !build_id_cache__kcore_existing(from_dir, to_dir, sizeof(to_dir))) {
@@ -155,7 +154,7 @@ static int build_id_cache__add_kcore(const char *filename, const char *debugdir,
 		return -1;
 
 	scnprintf(to_dir, sizeof(to_dir), "%s/[kernel.kcore]/%s/%s",
-		  debugdir, sbuildid, dir);
+		  buildid_dir, sbuildid, dir);
 
 	if (mkdir_p(to_dir, 0755))
 		return -1;
@@ -183,7 +182,7 @@ static int build_id_cache__add_kcore(const char *filename, const char *debugdir,
 	return 0;
 }
 
-static int build_id_cache__add_file(const char *filename, const char *debugdir)
+static int build_id_cache__add_file(const char *filename)
 {
 	char sbuild_id[BUILD_ID_SIZE * 2 + 1];
 	u8 build_id[BUILD_ID_SIZE];
@@ -195,7 +194,7 @@ static int build_id_cache__add_file(const char *filename, const char *debugdir)
 	}
 
 	build_id__sprintf(build_id, sizeof(build_id), sbuild_id);
-	err = build_id_cache__add_s(sbuild_id, debugdir, filename,
+	err = build_id_cache__add_s(sbuild_id, filename,
 				    false, false);
 	if (verbose)
 		pr_info("Adding %s %s: %s\n", sbuild_id, filename,
@@ -203,8 +202,7 @@ static int build_id_cache__add_file(const char *filename, const char *debugdir)
 	return err;
 }
 
-static int build_id_cache__remove_file(const char *filename,
-				       const char *debugdir)
+static int build_id_cache__remove_file(const char *filename)
 {
 	u8 build_id[BUILD_ID_SIZE];
 	char sbuild_id[BUILD_ID_SIZE * 2 + 1];
@@ -217,7 +215,7 @@ 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, debugdir);
+	err = build_id_cache__remove_s(sbuild_id);
 	if (verbose)
 		pr_info("Removing %s %s: %s\n", sbuild_id, filename,
 			err ? "FAIL" : "Ok");
@@ -252,8 +250,7 @@ static int build_id_cache__fprintf_missing(struct perf_session *session, FILE *f
 	return 0;
 }
 
-static int build_id_cache__update_file(const char *filename,
-				       const char *debugdir)
+static int build_id_cache__update_file(const char *filename)
 {
 	u8 build_id[BUILD_ID_SIZE];
 	char sbuild_id[BUILD_ID_SIZE * 2 + 1];
@@ -266,11 +263,10 @@ static int build_id_cache__update_file(const char *filename,
 	}
 
 	build_id__sprintf(build_id, sizeof(build_id), sbuild_id);
-	err = build_id_cache__remove_s(sbuild_id, debugdir);
-	if (!err) {
-		err = build_id_cache__add_s(sbuild_id, debugdir, filename,
-					    false, false);
-	}
+	err = build_id_cache__remove_s(sbuild_id);
+	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");
@@ -338,7 +334,7 @@ int cmd_buildid_cache(int argc, const char **argv,
 		list = strlist__new(true, add_name_list_str);
 		if (list) {
 			strlist__for_each(pos, list)
-				if (build_id_cache__add_file(pos->s, buildid_dir)) {
+				if (build_id_cache__add_file(pos->s)) {
 					if (errno == EEXIST) {
 						pr_debug("%s already in the cache\n",
 							 pos->s);
@@ -356,7 +352,7 @@ int cmd_buildid_cache(int argc, const char **argv,
 		list = strlist__new(true, remove_name_list_str);
 		if (list) {
 			strlist__for_each(pos, list)
-				if (build_id_cache__remove_file(pos->s, buildid_dir)) {
+				if (build_id_cache__remove_file(pos->s)) {
 					if (errno == ENOENT) {
 						pr_debug("%s wasn't in the cache\n",
 							 pos->s);
@@ -377,7 +373,7 @@ int cmd_buildid_cache(int argc, const char **argv,
 		list = strlist__new(true, update_name_list_str);
 		if (list) {
 			strlist__for_each(pos, list)
-				if (build_id_cache__update_file(pos->s, buildid_dir)) {
+				if (build_id_cache__update_file(pos->s)) {
 					if (errno == ENOENT) {
 						pr_debug("%s wasn't in the cache\n",
 							 pos->s);
@@ -391,8 +387,7 @@ int cmd_buildid_cache(int argc, const char **argv,
 		}
 	}
 
-	if (kcore_filename &&
-	    build_id_cache__add_kcore(kcore_filename, buildid_dir, force))
+	if (kcore_filename && build_id_cache__add_kcore(kcore_filename, force))
 		pr_warning("Couldn't add %s\n", kcore_filename);
 
 out:
diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index 0c72680..9f764f6 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -259,8 +259,8 @@ void disable_buildid_cache(void)
 	no_buildid_cache = true;
 }
 
-int build_id_cache__add_s(const char *sbuild_id, const char *debugdir,
-			  const char *name, bool is_kallsyms, bool is_vdso)
+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),
@@ -282,7 +282,7 @@ int build_id_cache__add_s(const char *sbuild_id, const char *debugdir,
 		goto out_free;
 
 	len = scnprintf(filename, size, "%s%s%s",
-		       debugdir, slash ? "/" : "",
+		       buildid_dir, slash ? "/" : "",
 		       is_vdso ? DSO__NAME_VDSO : realname);
 	if (mkdir_p(filename, 0755))
 		goto out_free;
@@ -298,13 +298,13 @@ int build_id_cache__add_s(const char *sbuild_id, const char *debugdir,
 	}
 
 	len = scnprintf(linkname, size, "%s/.build-id/%.2s",
-		       debugdir, sbuild_id);
+			buildid_dir, sbuild_id);
 
 	if (access(linkname, X_OK) && mkdir_p(linkname, 0755))
 		goto out_free;
 
 	snprintf(linkname + len, size - len, "/%s", sbuild_id + 2);
-	targetname = filename + strlen(debugdir) - 5;
+	targetname = filename + strlen(buildid_dir) - 5;
 	memcpy(targetname, "../..", 5);
 
 	if (symlink(targetname, linkname) == 0)
@@ -318,18 +318,17 @@ out_free:
 }
 
 static int build_id_cache__add_b(const u8 *build_id, size_t build_id_size,
-				 const char *name, const char *debugdir,
-				 bool is_kallsyms, bool is_vdso)
+				 const char *name, bool is_kallsyms,
+				 bool is_vdso)
 {
 	char sbuild_id[BUILD_ID_SIZE * 2 + 1];
 
 	build_id__sprintf(build_id, build_id_size, sbuild_id);
 
-	return build_id_cache__add_s(sbuild_id, debugdir, name,
-				     is_kallsyms, is_vdso);
+	return build_id_cache__add_s(sbuild_id, name, is_kallsyms, is_vdso);
 }
 
-int build_id_cache__remove_s(const char *sbuild_id, const char *debugdir)
+int build_id_cache__remove_s(const char *sbuild_id)
 {
 	const size_t size = PATH_MAX;
 	char *filename = zalloc(size),
@@ -340,7 +339,7 @@ int build_id_cache__remove_s(const char *sbuild_id, const char *debugdir)
 		goto out_free;
 
 	snprintf(linkname, size, "%s/.build-id/%.2s/%s",
-		 debugdir, sbuild_id, sbuild_id + 2);
+		 buildid_dir, sbuild_id, sbuild_id + 2);
 
 	if (access(linkname, F_OK))
 		goto out_free;
@@ -355,7 +354,7 @@ int build_id_cache__remove_s(const char *sbuild_id, const char *debugdir)
 	 * Since the link is relative, we must make it absolute:
 	 */
 	snprintf(linkname, size, "%s/.build-id/%.2s/%s",
-		 debugdir, sbuild_id, filename);
+		 buildid_dir, sbuild_id, filename);
 
 	if (unlink(linkname))
 		goto out_free;
@@ -367,8 +366,7 @@ out_free:
 	return err;
 }
 
-static int dso__cache_build_id(struct dso *dso, struct machine *machine,
-			       const char *debugdir)
+static int dso__cache_build_id(struct dso *dso, struct machine *machine)
 {
 	bool is_kallsyms = dso->kernel && dso->long_name[0] != '/';
 	bool is_vdso = dso__is_vdso(dso);
@@ -381,28 +379,26 @@ static int dso__cache_build_id(struct dso *dso, struct machine *machine,
 		name = nm;
 	}
 	return build_id_cache__add_b(dso->build_id, sizeof(dso->build_id), name,
-				     debugdir, is_kallsyms, is_vdso);
+				     is_kallsyms, is_vdso);
 }
 
 static int __dsos__cache_build_ids(struct list_head *head,
-				   struct machine *machine, const char *debugdir)
+				   struct machine *machine)
 {
 	struct dso *pos;
 	int err = 0;
 
 	dsos__for_each_with_build_id(pos, head)
-		if (dso__cache_build_id(pos, machine, debugdir))
+		if (dso__cache_build_id(pos, machine))
 			err = -1;
 
 	return err;
 }
 
-static int machine__cache_build_ids(struct machine *machine, const char *debugdir)
+static int machine__cache_build_ids(struct machine *machine)
 {
-	int ret = __dsos__cache_build_ids(&machine->kernel_dsos.head, machine,
-					  debugdir);
-	ret |= __dsos__cache_build_ids(&machine->user_dsos.head, machine,
-				       debugdir);
+	int ret = __dsos__cache_build_ids(&machine->kernel_dsos.head, machine);
+	ret |= __dsos__cache_build_ids(&machine->user_dsos.head, machine);
 	return ret;
 }
 
@@ -417,11 +413,11 @@ int perf_session__cache_build_ids(struct perf_session *session)
 	if (mkdir(buildid_dir, 0755) != 0 && errno != EEXIST)
 		return -1;
 
-	ret = machine__cache_build_ids(&session->machines.host, buildid_dir);
+	ret = machine__cache_build_ids(&session->machines.host);
 
 	for (nd = rb_first(&session->machines.guests); nd; nd = rb_next(nd)) {
 		struct machine *pos = rb_entry(nd, struct machine, rb_node);
-		ret |= machine__cache_build_ids(pos, buildid_dir);
+		ret |= machine__cache_build_ids(pos);
 	}
 	return ret ? -1 : 0;
 }
diff --git a/tools/perf/util/build-id.h b/tools/perf/util/build-id.h
index 8236319..31b3c63 100644
--- a/tools/perf/util/build-id.h
+++ b/tools/perf/util/build-id.h
@@ -22,9 +22,9 @@ 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__add_s(const char *sbuild_id, const char *debugdir,
+int build_id_cache__add_s(const char *sbuild_id,
 			  const char *name, bool is_kallsyms, bool is_vdso);
-int build_id_cache__remove_s(const char *sbuild_id, const char *debugdir);
+int build_id_cache__remove_s(const char *sbuild_id);
 void disable_buildid_cache(void);
 
 #endif

[perf/core PATCH v2 2/4] perf buildid-cache: Consolidate .build-id cache path generators

[Masami Hiramatsu]

Consolidate .build-id cache path generating routines to
build_id__filename() function. Other functions must use
it to get the buildid cache path (link path) from build-id.
This can reduce the risk of partial-update.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
 tools/perf/util/build-id.c |   58 +++++++++++++++++++++++++++++++-------------
 1 file changed, 41 insertions(+), 17 deletions(-)

diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index 9f764f6..adbc360 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -93,6 +93,35 @@ int build_id__sprintf(const u8 *build_id, int len, char *bf)
 	return raw - build_id;
 }
 
+/* asnprintf consolidates asprintf and snprintf */
+static int asnprintf(char **strp, size_t size, const char *fmt, ...)
+{
+	va_list ap;
+	int ret;
+
+	if (!strp)
+		return -EINVAL;
+
+	va_start(ap, fmt);
+	if (*strp)
+		ret = vsnprintf(*strp, size, fmt, ap);
+	else
+		ret = vasprintf(strp, fmt, ap);
+	va_end(ap);
+
+	return ret;
+}
+
+static char *build_id__filename(const char *sbuild_id, char *bf, size_t size)
+{
+	char *tmp = bf;
+	int ret = asnprintf(&bf, size, "%s/.build-id/%.2s/%s", buildid_dir,
+			    sbuild_id, sbuild_id + 2);
+	if (ret < 0 || (tmp && size < (unsigned int)ret))
+		return NULL;
+	return bf;
+}
+
 char *dso__build_id_filename(const struct dso *dso, char *bf, size_t size)
 {
 	char build_id_hex[BUILD_ID_SIZE * 2 + 1];
@@ -101,14 +130,7 @@ char *dso__build_id_filename(const struct dso *dso, char *bf, size_t size)
 		return NULL;
 
 	build_id__sprintf(dso->build_id, sizeof(dso->build_id), build_id_hex);
-	if (bf == NULL) {
-		if (asprintf(&bf, "%s/.build-id/%.2s/%s", buildid_dir,
-			     build_id_hex, build_id_hex + 2) < 0)
-			return NULL;
-	} else
-		snprintf(bf, size, "%s/.build-id/%.2s/%s", buildid_dir,
-			 build_id_hex, build_id_hex + 2);
-	return bf;
+	return build_id__filename(build_id_hex, bf, size);
 }
 
 #define dsos__for_each_with_build_id(pos, head)	\
@@ -264,7 +286,7 @@ int build_id_cache__add_s(const char *sbuild_id, const char *name,
 {
 	const size_t size = PATH_MAX;
 	char *realname, *filename = zalloc(size),
-	     *linkname = zalloc(size), *targetname;
+	     *linkname = zalloc(size), *targetname, *tmp;
 	int len, err = -1;
 	bool slash = is_kallsyms || is_vdso;
 
@@ -297,13 +319,15 @@ int build_id_cache__add_s(const char *sbuild_id, const char *name,
 			goto out_free;
 	}
 
-	len = scnprintf(linkname, size, "%s/.build-id/%.2s",
-			buildid_dir, sbuild_id);
+	if (!build_id__filename(sbuild_id, linkname, size))
+		goto out_free;
+	tmp = strrchr(linkname, '/');
+	*tmp = '\0';
 
 	if (access(linkname, X_OK) && mkdir_p(linkname, 0755))
 		goto out_free;
 
-	snprintf(linkname + len, size - len, "/%s", sbuild_id + 2);
+	*tmp = '/';
 	targetname = filename + strlen(buildid_dir) - 5;
 	memcpy(targetname, "../..", 5);
 
@@ -332,14 +356,14 @@ int build_id_cache__remove_s(const char *sbuild_id)
 {
 	const size_t size = PATH_MAX;
 	char *filename = zalloc(size),
-	     *linkname = zalloc(size);
+	     *linkname = zalloc(size), *tmp;
 	int err = -1;
 
 	if (filename == NULL || linkname == NULL)
 		goto out_free;
 
-	snprintf(linkname, size, "%s/.build-id/%.2s/%s",
-		 buildid_dir, sbuild_id, sbuild_id + 2);
+	if (!build_id__filename(sbuild_id, linkname, size))
+		goto out_free;
 
 	if (access(linkname, F_OK))
 		goto out_free;
@@ -353,8 +377,8 @@ int build_id_cache__remove_s(const char *sbuild_id)
 	/*
 	 * Since the link is relative, we must make it absolute:
 	 */
-	snprintf(linkname, size, "%s/.build-id/%.2s/%s",
-		 buildid_dir, sbuild_id, filename);
+	tmp = strrchr(linkname, '/') + 1;
+	snprintf(tmp, size - (tmp - linkname), "%s", filename);
 
 	if (unlink(linkname))
 		goto out_free;

Re: [perf/core PATCH v2 2/4] perf buildid-cache: Consolidate .build-id cache path generators

[Arnaldo Carvalho de Melo]

Em Tue, Feb 10, 2015 at 06:18:53PM +0900, Masami Hiramatsu escreveu:
> +static int asnprintf(char **strp, size_t size, const char *fmt, ...)
> +{
> +	va_list ap;
> +	int ret;
> +	if (!strp)
> +		return -EINVAL;
> +	va_start(ap, fmt);
> +	if (*strp)
> +		ret = vsnprintf(*strp, size, fmt, ap);
> +	else
> +		ret = vasprintf(strp, fmt, ap);
> +	va_end(ap);
> +	return ret;
> +}
> +static char *build_id__filename(const char *sbuild_id, char *bf, size_t size)
> +{
> +	char *tmp = bf;
> +	int ret = asnprintf(&bf, size, "%s/.build-id/%.2s/%s", buildid_dir,
> +			    sbuild_id, sbuild_id + 2);
> +	if (ret < 0 || (tmp && size < (unsigned int)ret))
> +		return NULL;

This is a good thing, i.e. checking if the formatting truncated
something, i.e. vsnprintf honours 'size' but may return more than that,
but next time try to get it into a separate patch :-)

Anyway, after scratching my head with this extra thing/re-reading the
vsnprintf man page, I'm applying this patch, thanks.

- Arnaldo

[tip:perf/core] perf buildid-cache: Consolidate .build-id cache path generators

[tip-bot for Masami Hiramatsu]

Commit-ID:  5cb113fd84f72b6e08c1970d612fd61327781d4e
Gitweb:     http://git.kernel.org/tip/5cb113fd84f72b6e08c1970d612fd61327781d4e
Author:     Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
AuthorDate: Tue, 10 Feb 2015 18:18:53 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Wed, 11 Feb 2015 12:37:33 -0300

perf buildid-cache: Consolidate .build-id cache path generators

Consolidate .build-id cache path generating routines to
build_id__filename() function. Other functions must use it to get the
buildid cache path (link path) from build-id.  This can reduce the risk
of partial-update.

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Borislav Petkov <bp@suse.de>
Cc: Hemant Kumar <hemant@linux.vnet.ibm.com>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/20150210091853.19264.58513.stgit@localhost.localdomain
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/build-id.c | 58 ++++++++++++++++++++++++++++++++--------------
 1 file changed, 41 insertions(+), 17 deletions(-)

diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index 9f764f6..adbc360 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -93,6 +93,35 @@ int build_id__sprintf(const u8 *build_id, int len, char *bf)
 	return raw - build_id;
 }
 
+/* asnprintf consolidates asprintf and snprintf */
+static int asnprintf(char **strp, size_t size, const char *fmt, ...)
+{
+	va_list ap;
+	int ret;
+
+	if (!strp)
+		return -EINVAL;
+
+	va_start(ap, fmt);
+	if (*strp)
+		ret = vsnprintf(*strp, size, fmt, ap);
+	else
+		ret = vasprintf(strp, fmt, ap);
+	va_end(ap);
+
+	return ret;
+}
+
+static char *build_id__filename(const char *sbuild_id, char *bf, size_t size)
+{
+	char *tmp = bf;
+	int ret = asnprintf(&bf, size, "%s/.build-id/%.2s/%s", buildid_dir,
+			    sbuild_id, sbuild_id + 2);
+	if (ret < 0 || (tmp && size < (unsigned int)ret))
+		return NULL;
+	return bf;
+}
+
 char *dso__build_id_filename(const struct dso *dso, char *bf, size_t size)
 {
 	char build_id_hex[BUILD_ID_SIZE * 2 + 1];
@@ -101,14 +130,7 @@ char *dso__build_id_filename(const struct dso *dso, char *bf, size_t size)
 		return NULL;
 
 	build_id__sprintf(dso->build_id, sizeof(dso->build_id), build_id_hex);
-	if (bf == NULL) {
-		if (asprintf(&bf, "%s/.build-id/%.2s/%s", buildid_dir,
-			     build_id_hex, build_id_hex + 2) < 0)
-			return NULL;
-	} else
-		snprintf(bf, size, "%s/.build-id/%.2s/%s", buildid_dir,
-			 build_id_hex, build_id_hex + 2);
-	return bf;
+	return build_id__filename(build_id_hex, bf, size);
 }
 
 #define dsos__for_each_with_build_id(pos, head)	\
@@ -264,7 +286,7 @@ int build_id_cache__add_s(const char *sbuild_id, const char *name,
 {
 	const size_t size = PATH_MAX;
 	char *realname, *filename = zalloc(size),
-	     *linkname = zalloc(size), *targetname;
+	     *linkname = zalloc(size), *targetname, *tmp;
 	int len, err = -1;
 	bool slash = is_kallsyms || is_vdso;
 
@@ -297,13 +319,15 @@ int build_id_cache__add_s(const char *sbuild_id, const char *name,
 			goto out_free;
 	}
 
-	len = scnprintf(linkname, size, "%s/.build-id/%.2s",
-			buildid_dir, sbuild_id);
+	if (!build_id__filename(sbuild_id, linkname, size))
+		goto out_free;
+	tmp = strrchr(linkname, '/');
+	*tmp = '\0';
 
 	if (access(linkname, X_OK) && mkdir_p(linkname, 0755))
 		goto out_free;
 
-	snprintf(linkname + len, size - len, "/%s", sbuild_id + 2);
+	*tmp = '/';
 	targetname = filename + strlen(buildid_dir) - 5;
 	memcpy(targetname, "../..", 5);
 
@@ -332,14 +356,14 @@ int build_id_cache__remove_s(const char *sbuild_id)
 {
 	const size_t size = PATH_MAX;
 	char *filename = zalloc(size),
-	     *linkname = zalloc(size);
+	     *linkname = zalloc(size), *tmp;
 	int err = -1;
 
 	if (filename == NULL || linkname == NULL)
 		goto out_free;
 
-	snprintf(linkname, size, "%s/.build-id/%.2s/%s",
-		 buildid_dir, sbuild_id, sbuild_id + 2);
+	if (!build_id__filename(sbuild_id, linkname, size))
+		goto out_free;
 
 	if (access(linkname, F_OK))
 		goto out_free;
@@ -353,8 +377,8 @@ int build_id_cache__remove_s(const char *sbuild_id)
 	/*
 	 * Since the link is relative, we must make it absolute:
 	 */
-	snprintf(linkname, size, "%s/.build-id/%.2s/%s",
-		 buildid_dir, sbuild_id, filename);
+	tmp = strrchr(linkname, '/') + 1;
+	snprintf(tmp, size - (tmp - linkname), "%s", filename);
 
 	if (unlink(linkname))
 		goto out_free;

[perf/core PATCH v2 3/4] perf buildid-cache: Add new buildid cache if update target is not cached

[Masami Hiramatsu]

Add new buildid cache if the update target file is not cached.
This can happen when an old binary is replaced by new one
after caching the old one. In this case, user sees his operation
just failed. But it does not look straight, since user just
pass the binary "path", not "build-id".

  ----
  # ./perf buildid-cache --add ./perf
  (update ./perf to new binary)
  # ./perf buildid-cache --update ./perf
  ./perf wasn't in the cache
  #
  ----

This patch adds given new binary to cache if the new binary is
not cached. So we'll not see the above error.

  ----
  # ./perf buildid-cache --add ./perf
  (update ./perf to new binary)
  # ./perf buildid-cache --update ./perf
  #
  ----

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
---
 tools/perf/Documentation/perf-buildid-cache.txt |    2 ++
 tools/perf/builtin-buildid-cache.c              |    6 ++++--
 tools/perf/util/build-id.c                      |   12 ++++++++++++
 tools/perf/util/build-id.h                      |    1 +
 4 files changed, 19 insertions(+), 2 deletions(-)

diff --git a/tools/perf/Documentation/perf-buildid-cache.txt b/tools/perf/Documentation/perf-buildid-cache.txt
index 0294c57..6575dce 100644
--- a/tools/perf/Documentation/perf-buildid-cache.txt
+++ b/tools/perf/Documentation/perf-buildid-cache.txt
@@ -44,6 +44,8 @@ OPTIONS
 --update::
 	Update specified file of the cache. It can be used to update kallsyms
 	kernel dso to vmlinux in order to support annotation.
+	If there is no cache which has given binary's build-id, this just adds
+	new one.
 -v::
 --verbose::
 	Be more verbose.
diff --git a/tools/perf/builtin-buildid-cache.c b/tools/perf/builtin-buildid-cache.c
index d929d95..e7568f5 100644
--- a/tools/perf/builtin-buildid-cache.c
+++ b/tools/perf/builtin-buildid-cache.c
@@ -255,7 +255,7 @@ static int build_id_cache__update_file(const char *filename)
 	u8 build_id[BUILD_ID_SIZE];
 	char sbuild_id[BUILD_ID_SIZE * 2 + 1];
 
-	int err;
+	int err = 0;
 
 	if (filename__read_build_id(filename, &build_id, sizeof(build_id)) < 0) {
 		pr_debug("Couldn't read a build-id in %s\n", filename);
@@ -263,7 +263,9 @@ static int build_id_cache__update_file(const char *filename)
 	}
 
 	build_id__sprintf(build_id, sizeof(build_id), sbuild_id);
-	err = build_id_cache__remove_s(sbuild_id);
+	if (build_id_cache__cached(sbuild_id))
+		err = build_id_cache__remove_s(sbuild_id);
+
 	if (!err)
 		err = build_id_cache__add_s(sbuild_id, filename, false, false);
 
diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index adbc360..0bc33be 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -352,6 +352,18 @@ static int build_id_cache__add_b(const u8 *build_id, size_t build_id_size,
 	return build_id_cache__add_s(sbuild_id, name, is_kallsyms, is_vdso);
 }
 
+bool build_id_cache__cached(const char *sbuild_id)
+{
+	bool ret = false;
+	char *filename = build_id__filename(sbuild_id, NULL, 0);
+
+	if (filename && !access(filename, F_OK))
+		ret = true;
+	free(filename);
+
+	return ret;
+}
+
 int build_id_cache__remove_s(const char *sbuild_id)
 {
 	const size_t size = PATH_MAX;
diff --git a/tools/perf/util/build-id.h b/tools/perf/util/build-id.h
index 31b3c63..2a09498 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);
 
+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);
 int build_id_cache__remove_s(const char *sbuild_id);

Re: [perf/core PATCH v2 3/4] perf buildid-cache: Add new buildid cache if update target is not cached

[Arnaldo Carvalho de Melo]

Em Tue, Feb 10, 2015 at 06:18:56PM +0900, Masami Hiramatsu escreveu:
> Add new buildid cache if the update target file is not cached.
> This can happen when an old binary is replaced by new one
> after caching the old one. In this case, user sees his operation
> just failed. But it does not look straight, since user just
> pass the binary "path", not "build-id".
> 
>   ----
>   # ./perf buildid-cache --add ./perf
>   (update ./perf to new binary)
>   # ./perf buildid-cache --update ./perf
>   ./perf wasn't in the cache

Humm, without re-reading the original motivation for the '--update'
operation I would think it was about finding all build-ids in the cache
that are for a binary with that path, remove them and insert this new
one, no?

Checking...

>   #
>   ----
> 
> This patch adds given new binary to cache if the new binary is
> not cached. So we'll not see the above error.
> 
>   ----
>   # ./perf buildid-cache --add ./perf
>   (update ./perf to new binary)
>   # ./perf buildid-cache --update ./perf
>   #
>   ----
> 
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> ---
>  tools/perf/Documentation/perf-buildid-cache.txt |    2 ++
>  tools/perf/builtin-buildid-cache.c              |    6 ++++--
>  tools/perf/util/build-id.c                      |   12 ++++++++++++
>  tools/perf/util/build-id.h                      |    1 +
>  4 files changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/Documentation/perf-buildid-cache.txt b/tools/perf/Documentation/perf-buildid-cache.txt
> index 0294c57..6575dce 100644
> --- a/tools/perf/Documentation/perf-buildid-cache.txt
> +++ b/tools/perf/Documentation/perf-buildid-cache.txt
> @@ -44,6 +44,8 @@ OPTIONS
>  --update::
>  	Update specified file of the cache. It can be used to update kallsyms
>  	kernel dso to vmlinux in order to support annotation.
> +	If there is no cache which has given binary's build-id, this just adds
> +	new one.
>  -v::
>  --verbose::
>  	Be more verbose.
> diff --git a/tools/perf/builtin-buildid-cache.c b/tools/perf/builtin-buildid-cache.c
> index d929d95..e7568f5 100644
> --- a/tools/perf/builtin-buildid-cache.c
> +++ b/tools/perf/builtin-buildid-cache.c
> @@ -255,7 +255,7 @@ static int build_id_cache__update_file(const char *filename)
>  	u8 build_id[BUILD_ID_SIZE];
>  	char sbuild_id[BUILD_ID_SIZE * 2 + 1];
>  
> -	int err;
> +	int err = 0;
>  
>  	if (filename__read_build_id(filename, &build_id, sizeof(build_id)) < 0) {
>  		pr_debug("Couldn't read a build-id in %s\n", filename);
> @@ -263,7 +263,9 @@ static int build_id_cache__update_file(const char *filename)
>  	}
>  
>  	build_id__sprintf(build_id, sizeof(build_id), sbuild_id);
> -	err = build_id_cache__remove_s(sbuild_id);
> +	if (build_id_cache__cached(sbuild_id))
> +		err = build_id_cache__remove_s(sbuild_id);
> +
>  	if (!err)
>  		err = build_id_cache__add_s(sbuild_id, filename, false, false);
>  
> diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
> index adbc360..0bc33be 100644
> --- a/tools/perf/util/build-id.c
> +++ b/tools/perf/util/build-id.c
> @@ -352,6 +352,18 @@ static int build_id_cache__add_b(const u8 *build_id, size_t build_id_size,
>  	return build_id_cache__add_s(sbuild_id, name, is_kallsyms, is_vdso);
>  }
>  
> +bool build_id_cache__cached(const char *sbuild_id)
> +{
> +	bool ret = false;
> +	char *filename = build_id__filename(sbuild_id, NULL, 0);
> +
> +	if (filename && !access(filename, F_OK))
> +		ret = true;
> +	free(filename);
> +
> +	return ret;
> +}
> +
>  int build_id_cache__remove_s(const char *sbuild_id)
>  {
>  	const size_t size = PATH_MAX;
> diff --git a/tools/perf/util/build-id.h b/tools/perf/util/build-id.h
> index 31b3c63..2a09498 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);
>  
> +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);
>  int build_id_cache__remove_s(const char *sbuild_id);

Re: [perf/core PATCH v2 3/4] perf buildid-cache: Add new buildid cache if update target is not cached

[Arnaldo Carvalho de Melo]

Em Wed, Feb 11, 2015 at 11:49:28AM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Feb 10, 2015 at 06:18:56PM +0900, Masami Hiramatsu escreveu:
> > Add new buildid cache if the update target file is not cached.
> > This can happen when an old binary is replaced by new one
> > after caching the old one. In this case, user sees his operation
> > just failed. But it does not look straight, since user just
> > pass the binary "path", not "build-id".
> > 
> >   ----
> >   # ./perf buildid-cache --add ./perf
> >   (update ./perf to new binary)
> >   # ./perf buildid-cache --update ./perf
> >   ./perf wasn't in the cache
> 
> Humm, without re-reading the original motivation for the '--update'
> operation I would think it was about finding all build-ids in the cache
> that are for a binary with that path, remove them and insert this new
> one, no?
> 
> Checking...

commit eeb49845425375481f14c0e5721f88242642e88e
Author: Namhyung Kim <namhyung.kim@lge.com>
Date:   Thu Feb 7 18:02:11 2013 +0900

    perf buildid-cache: Add --update option
    
    When adding vmlinux file to build-id cache, it'd be fail since kallsyms
    dso with a same build-id was already added by perf record.
    
    So one needs to remove the kallsyms first to add vmlinux into the cache.
    Add --update option for doing it at once.

----------------------------------------------------------------------------------

So this was really a 'remove the file that is pointed by this build-id' and
replace with this new file, i.e. there is both a vmlinux _and_ a kallsyms file
for the same build-id. When wanting to use one or the other and existing a
link in the cache, one uses this --update thing.

That is ok for cases where there are multiple symtabs in the cache for a given
build-id, but as you mention above, it is confusing for updating by _pathname_.

What a 'update' by pathname would mean? I guess it would be to remove 'old',
i.e. 'not up-to-date' stuff, i.e. older build-ids that that pathname had in the
past, and leave just this new stuff.

The way you did it make '--update path' be equivalent to '--add path'.

- Arnaldo

Re: Re: [perf/core PATCH v2 3/4] perf buildid-cache: Add new buildid cache if update target is not cached

[Masami Hiramatsu]

(2015/02/11 23:57), Arnaldo Carvalho de Melo wrote:
> Em Wed, Feb 11, 2015 at 11:49:28AM -0300, Arnaldo Carvalho de Melo escreveu:
>> Em Tue, Feb 10, 2015 at 06:18:56PM +0900, Masami Hiramatsu escreveu:
>>> Add new buildid cache if the update target file is not cached.
>>> This can happen when an old binary is replaced by new one
>>> after caching the old one. In this case, user sees his operation
>>> just failed. But it does not look straight, since user just
>>> pass the binary "path", not "build-id".
>>>
>>>   ----
>>>   # ./perf buildid-cache --add ./perf
>>>   (update ./perf to new binary)
>>>   # ./perf buildid-cache --update ./perf
>>>   ./perf wasn't in the cache
>>
>> Humm, without re-reading the original motivation for the '--update'
>> operation I would think it was about finding all build-ids in the cache
>> that are for a binary with that path, remove them and insert this new
>> one, no?
>>
>> Checking...
> 
> commit eeb49845425375481f14c0e5721f88242642e88e
> Author: Namhyung Kim <namhyung.kim@lge.com>
> Date:   Thu Feb 7 18:02:11 2013 +0900
> 
>     perf buildid-cache: Add --update option
>     
>     When adding vmlinux file to build-id cache, it'd be fail since kallsyms
>     dso with a same build-id was already added by perf record.
>     
>     So one needs to remove the kallsyms first to add vmlinux into the cache.
>     Add --update option for doing it at once.
> 
> ----------------------------------------------------------------------------------
> 
> So this was really a 'remove the file that is pointed by this build-id' and
> replace with this new file, i.e. there is both a vmlinux _and_ a kallsyms file
> for the same build-id. When wanting to use one or the other and existing a
> link in the cache, one uses this --update thing.
> 
> That is ok for cases where there are multiple symtabs in the cache for a given
> build-id, but as you mention above, it is confusing for updating by _pathname_.
> 
> What a 'update' by pathname would mean? I guess it would be to remove 'old',
> i.e. 'not up-to-date' stuff, i.e. older build-ids that that pathname had in the
> past, and leave just this new stuff.

> 
> The way you did it make '--update path' be equivalent to '--add path'.

Right, if there is no same buildid cache, it does that.
Actually, this is a kind of middle point solution. As far as I can see,
buildid-cache is (currently) used for annotating the old, or remote perf.data.
In that case, we may have several different versions of buildid-cache(binaries)
for one pathname. If --update destructively updates all caches related to
given pathname, it also means to loose annotatability(?) on old or remote
perf.data. I think that may not the user wants.
So, if user really wants to clean up their cache, he/she can use --purge
(--remove-all).
And I think it would be better that --update just adds given current binary
to cache if there is no cache.

Thank you,


> 
> - Arnaldo
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 


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

Re: Re: [perf/core PATCH v2 3/4] perf buildid-cache: Add new buildid cache if update target is not cached

[Arnaldo Carvalho de Melo]

Em Thu, Feb 12, 2015 at 03:36:58PM +0900, Masami Hiramatsu escreveu:
> (2015/02/11 23:57), Arnaldo Carvalho de Melo wrote:
> > Em Wed, Feb 11, 2015 at 11:49:28AM -0300, Arnaldo Carvalho de Melo escreveu:
> >> Em Tue, Feb 10, 2015 at 06:18:56PM +0900, Masami Hiramatsu escreveu:
> >>> Add new buildid cache if the update target file is not cached.
> >>> This can happen when an old binary is replaced by new one
> >>> after caching the old one. In this case, user sees his operation
> >>> just failed. But it does not look straight, since user just
> >>> pass the binary "path", not "build-id".
> >>>
> >>>   ----
> >>>   # ./perf buildid-cache --add ./perf
> >>>   (update ./perf to new binary)
> >>>   # ./perf buildid-cache --update ./perf
> >>>   ./perf wasn't in the cache
> >>
> >> Humm, without re-reading the original motivation for the '--update'
> >> operation I would think it was about finding all build-ids in the cache
> >> that are for a binary with that path, remove them and insert this new
> >> one, no?
> >>
> >> Checking...
> > 
> > commit eeb49845425375481f14c0e5721f88242642e88e
> > Author: Namhyung Kim <namhyung.kim@lge.com>
> > Date:   Thu Feb 7 18:02:11 2013 +0900
> > 
> >     perf buildid-cache: Add --update option
> >     
> >     When adding vmlinux file to build-id cache, it'd be fail since kallsyms
> >     dso with a same build-id was already added by perf record.
> >     
> >     So one needs to remove the kallsyms first to add vmlinux into the cache.
> >     Add --update option for doing it at once.
> > 
> > ----------------------------------------------------------------------------------
> > 
> > So this was really a 'remove the file that is pointed by this build-id' and
> > replace with this new file, i.e. there is both a vmlinux _and_ a kallsyms file
> > for the same build-id. When wanting to use one or the other and existing a
> > link in the cache, one uses this --update thing.

> > That is ok for cases where there are multiple symtabs in the cache for a given
> > build-id, but as you mention above, it is confusing for updating by _pathname_.

> > What a 'update' by pathname would mean? I guess it would be to remove 'old',
> > i.e. 'not up-to-date' stuff, i.e. older build-ids that that pathname had in the
> > past, and leave just this new stuff.
 
> > The way you did it make '--update path' be equivalent to '--add path'.
 
> Right, if there is no same buildid cache, it does that.
> Actually, this is a kind of middle point solution. As far as I can see,
> buildid-cache is (currently) used for annotating the old, or remote perf.data.
> In that case, we may have several different versions of buildid-cache(binaries)
> for one pathname. If --update destructively updates all caches related to
> given pathname, it also means to loose annotatability(?) on old or remote
> perf.data. I think that may not the user wants.

> So, if user really wants to clean up their cache, he/she can use --purge
> (--remove-all).

Ok, I thought about this, i.e. if you want to remove older stuff, purge
it.

Ah, at some point it would be nice to have --purge receive a parameter,
telling that things that are older/newer/within some range should be
removed, but sure, that can be left for later 8-)

> And I think it would be better that --update just adds given current binary
> to cache if there is no cache.

Agreed, and it would be really nice to have parts of this conversation,
edited, available on tools/perf/Documentation/perf-buildid--cache.txt
:-)

- Arnaldo

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

[Masami Hiramatsu]

Add --remove-all 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 --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);

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

[Arnaldo Carvalho de Melo]

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'

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

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

[Masami Hiramatsu]

(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

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

[Arnaldo Carvalho de Melo]

Em Thu, Feb 12, 2015 at 03:13:02PM +0900, Masami Hiramatsu escreveu:
> (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.

Ok, please note that I picked the first two patches, even sent a pull
request to Ingo where those are included, so just resend what is left,

Thanks,

- Arnaldo

Re: [perf/core PATCH v2 0/4] perf-buildid-cache: Cleanup, enhance --update and add --remove-all

[Namhyung Kim]

Hi Masami,

On Tue, Feb 10, 2015 at 06:18:49PM +0900, Masami Hiramatsu wrote:
> Hi,
> 
> Here is the 2nd version of of perf buildid-cache update.
> I've fixed a bug in patch 4/4 reported by Namhyung, Thanks!
> 
> The first 2 patches are cleanup code, just reducing the
> redundant code. The 3rd one modifies --update option not
> to fail but just add new binary if there is no cache entry
> which has same build-id of given binary. The 4th one adds
> --remove-all option which allows us to cleanup all caches
> which related to given path.

For this series,

Acked-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung


> 
> 
> Thank you,
> 
> 
> ---
> 
> Masami Hiramatsu (4):
>       perf-buildid-cache: Remove unneeded debugdir parameters
>       perf buildid-cache: Consolidate .build-id cache path generators
>       perf buildid-cache: Add new buildid cache if update target is not cached
>       perf buildid-cache: Add --remove-all FILE to remove all caches of FILE
> 
> 
>  tools/perf/Documentation/perf-buildid-cache.txt |    8 +
>  tools/perf/builtin-buildid-cache.c              |   85 ++++++++--
>  tools/perf/util/build-id.c                      |  189 +++++++++++++++++------
>  tools/perf/util/build-id.h                      |    6 -
>  4 files changed, 210 insertions(+), 78 deletions(-)
> 
> --
> Masami HIRAMATSU
> Software Platform Research Dpt. Linux Technology Center
> Hitachi, Ltd., Yokohama Research Laboratory
> E-mail: masami.hiramatsu.pt@hitachi.com