Skip to contentSkip to conversation

perf-buildid-cache: Cleanup, enhance --update and add --remove-all

PATCH Superseded Masami Hiramatsu 4 participants 16 messages Feb 10, 2015 – Feb 18, 2015 perf events

v2 of 6 Superseded Latest is v6 →

lore
Share & tools
lore
#0 lore raw

v2 cover perf-buildid-cache: Cleanup, enhance --update and add --remove-all

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

#1 lore raw

PATCH v2 1/4 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.

---
 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(-)
tools/perf/builtin-buildid-cache.c +16 -21
index 50e6b66..d929d95 100644
@@ -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:
tools/perf/util/build-id.c +20 -24

127 lines not on this page · raw message

tools/perf/util/build-id.h +2 -2
index 8236319..31b3c63 100644
@@ -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
#2 lore raw

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

perf buildid-cache: Remove unneeded debugdir parameters
Commit-ID:  e35f7362bab455fb5c13ea4ce53f959f3e1610b2
Gitweb:     http://git.kernel.org/tip/e35f7362bab455fb5c13ea4ce53f959f3e1610b2
Author:     Masami Hiramatsu <[email protected]>
AuthorDate: Tue, 10 Feb 2015 18:18:51 +0900
Committer:  Arnaldo Carvalho de Melo <[email protected]>
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.

---
 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(-)
tools/perf/builtin-buildid-cache.c +16 -21
index 50e6b66..d929d95 100644
@@ -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:
tools/perf/util/build-id.c +20 -24

127 lines not on this page · raw message

tools/perf/util/build-id.h +2 -2
index 8236319..31b3c63 100644
@@ -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
#3 lore raw

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

In reply to 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.

---
 tools/perf/util/build-id.c |   58 +++++++++++++++++++++++++++++++-------------
 1 file changed, 41 insertions(+), 17 deletions(-)
tools/perf/util/build-id.c +41 -17
index 9f764f6..adbc360 100644
@@ -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;
#4 lore raw

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

Em Tue, Feb 10, 2015 at 06:18:53PM +0900, Masami Hiramatsu escreveu:

Quoted 11 lines
tools/perf/util/build-id.c +11 -0
+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);

tools/perf/util/build-id.c int build_id__sprintf(const u8 *build_id, int len, char *bf) · line 121

Quoted 10 lines
tools/perf/util/build-id.c +10 -0
+	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

#5 lore raw

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

In reply to Masami Hiramatsu
perf buildid-cache: Consolidate .build-id cache path generators
Commit-ID:  5cb113fd84f72b6e08c1970d612fd61327781d4e
Gitweb:     http://git.kernel.org/tip/5cb113fd84f72b6e08c1970d612fd61327781d4e
Author:     Masami Hiramatsu <[email protected]>
AuthorDate: Tue, 10 Feb 2015 18:18:53 +0900
Committer:  Arnaldo Carvalho de Melo <[email protected]>
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.

---
 tools/perf/util/build-id.c | 58 ++++++++++++++++++++++++++++++++--------------
 1 file changed, 41 insertions(+), 17 deletions(-)
tools/perf/util/build-id.c +41 -17
index 9f764f6..adbc360 100644
@@ -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;
#6 lore raw

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

In reply to 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
  #
  ----
---
 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(-)
tools/perf/Documentation/perf-buildid-cache.txt +2 -0
index 0294c57..6575dce 100644
@@ -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.
tools/perf/builtin-buildid-cache.c +4 -2
index d929d95..e7568f5 100644
@@ -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);
 
tools/perf/util/build-id.c +12 -0
index adbc360..0bc33be 100644
@@ -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;
tools/perf/util/build-id.h +1 -0
index 31b3c63..2a09498 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);
 
+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);
#7 lore raw

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

Em Tue, Feb 10, 2015 at 06:18:56PM +0900, Masami Hiramatsu escreveu:

Quoted 11 lines

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

Quoted 93 lines

#
----

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
  #
  ----
---
 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(-)
tools/perf/Documentation/perf-buildid-cache.txt +2 -0
index 0294c57..6575dce 100644
@@ -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.
tools/perf/builtin-buildid-cache.c +4 -2
index d929d95..e7568f5 100644
@@ -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);
 
tools/perf/util/build-id.c +12 -0
index adbc360..0bc33be 100644
@@ -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;
tools/perf/util/build-id.h +1 -0
index 31b3c63..2a09498 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);
 
+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);
#8 lore raw

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

Em Wed, Feb 11, 2015 at 11:49:28AM -0300, Arnaldo Carvalho de Melo escreveu:

Quoted 13 lines

Em Tue, Feb 10, 2015 at 06:18:56PM +0900, Masami Hiramatsu escreveu:

Quoted 11 lines

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
Quoted 6 lines

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

#9 lore raw

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

Quoted 43 lines · Arnaldo Carvalho de Melo

Em Wed, Feb 11, 2015 at 11:49:28AM -0300, Arnaldo Carvalho de Melo escreveu:

Quoted 13 lines

Em Tue, Feb 10, 2015 at 06:18:56PM +0900, Masami Hiramatsu escreveu:

Quoted 11 lines

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
Quoted 6 lines

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

Quoted 6 lines

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,

Quoted 8 lines

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

Signature · 4 lines
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]
#10 lore raw

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

Em Thu, Feb 12, 2015 at 03:36:58PM +0900, Masami Hiramatsu escreveu:

Quoted 58 lines
Quoted 40 lines · Arnaldo Carvalho de Melo

Em Wed, Feb 11, 2015 at 11:49:28AM -0300, Arnaldo Carvalho de Melo escreveu:

Quoted 13 lines

Em Tue, Feb 10, 2015 at 06:18:56PM +0900, Masami Hiramatsu escreveu:

Quoted 11 lines

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
Quoted 6 lines

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

Quoted 8 lines

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.

Quoted 2 lines

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

Quoted 2 lines

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

#11 lore raw

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

In reply to 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
  -----
---
 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(-)
tools/perf/Documentation/perf-buildid-cache.txt +5 -1
index 6575dce..01f295e 100644
@@ -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.
tools/perf/builtin-buildid-cache.c +44 -0

68 lines not on this page · raw message

tools/perf/util/build-id.c +67 -16

102 lines not on this page · raw message

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);
#12 lore raw

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

Em Tue, Feb 10, 2015 at 06:18:58PM +0900, Masami Hiramatsu escreveu:

Quoted 1 line

Add --remove-all FILE to remove all caches of FILE.

I like --purge better, shorter form, like in 'man fpurge'

- Arnaldo

Quoted 248 lines

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
  -----
---
 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(-)
tools/perf/Documentation/perf-buildid-cache.txt +5 -1
index 6575dce..01f295e 100644
@@ -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.
tools/perf/builtin-buildid-cache.c +44 -0
index e7568f5..3a76d51 100644
@@ -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);
 
tools/perf/util/build-id.c +67 -16
index 0bc33be..b664b06 100644
@@ -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) {
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);
#13 lore raw

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

Quoted 4 lines · Arnaldo Carvalho de Melo

Em Tue, Feb 10, 2015 at 06:18:58PM +0900, Masami Hiramatsu escreveu:

Quoted 1 line

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,

Quoted 252 lines

- Arnaldo

Quoted 248 lines

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
  -----
---
 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(-)
tools/perf/Documentation/perf-buildid-cache.txt +5 -1
index 6575dce..01f295e 100644
@@ -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.
tools/perf/builtin-buildid-cache.c +44 -0
index e7568f5..3a76d51 100644
@@ -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);
 
tools/perf/util/build-id.c +67 -16
index 0bc33be..b664b06 100644
@@ -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) {
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);
Signature · 4 lines
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: [email protected]
#14 lore raw

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

Em Thu, Feb 12, 2015 at 03:13:02PM +0900, Masami Hiramatsu escreveu:

Quoted 8 lines
Quoted 4 lines · Arnaldo Carvalho de Melo

Em Tue, Feb 10, 2015 at 06:18:58PM +0900, Masami Hiramatsu escreveu:

Quoted 1 line

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

maintainer Acked-by on the v2 series
#15 lore raw
In reply to Masami Hiramatsu

Hi Masami,

Quoted 11 lines · 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.

For this series,

Thanks,
Namhyung

Quoted 25 lines

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