Skip to contentSkip to conversation

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

PATCH Superseded Masami Hiramatsu 3 participants 10 messages Feb 26, 2015 – Feb 28, 2015 perf events

v5 of 6 Superseded Latest is v6 →

lore
Share & tools
lore
#0 lore raw

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

Hi,

Here is the 5th version of of perf buildid-cache update. This updates the 2nd patch and add 2 patches just for cleanup and improve usability a bit.
Here are the changes in v5.

- [2/4] Remove NULL check before calling strlist__delete() (Thanks to Hemant!)
- [3/4] Use pr_debug instead of verbose && pr_info (Thanks to Namhyung!)
- [4/4] Show usage with incorrect params

Thank you,

---

Masami Hiramatsu (4):
perf buildid-cache: Add new buildid cache if update target is not cached perf buildid-cache: Add --purge FILE to remove all caches of FILE
perf-buildid-cache: Use pr_debug instead of verbose && pr_info
perf-buildid-cache: Show usage with incorrect params

 tools/perf/Documentation/perf-buildid-cache.txt |   24 ++++--
 tools/perf/builtin-buildid-cache.c              |   69 ++++++++++++++--
 tools/perf/util/build-id.c                      |   97 +++++++++++++++++++----
 tools/perf/util/build-id.h                      |    2 
 4 files changed, 157 insertions(+), 35 deletions(-)

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

#1 lore raw

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

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 |   11 ++++++++---
 tools/perf/builtin-buildid-cache.c              |    6 ++++--
 tools/perf/util/build-id.c                      |   12 ++++++++++++
 tools/perf/util/build-id.h                      |    1 +
 4 files changed, 25 insertions(+), 5 deletions(-)
tools/perf/Documentation/perf-buildid-cache.txt +8 -3
index 0294c57..cec6b57 100644
@@ -41,9 +41,14 @@ OPTIONS
 --missing=::
 	List missing build ids in the cache for the specified file.
 -u::
---update::
-	Update specified file of the cache. It can be used to update kallsyms
-	kernel dso to vmlinux in order to support annotation.
+--update=::
+	Update specified file of the cache. Note that this doesn't remove
+	older entires since those may be still needed for annotating old
+	(or remote) perf.data. Only if there is already a cache which has
+	exactly same build-id, that is replaced by new one. It can be used
+	to update kallsyms and kernel dso to vmlinux in order to support
+	annotation.
+
 -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);
#2 lore raw

[tip:perf/core] perf buildid-cache: Add new buildid cache if update target is not cached

perf buildid-cache: Add new buildid cache if update target is not cached
Commit-ID:  a50d11a10c2db86d7383c281d4e249d5393661e9
Gitweb:     http://git.kernel.org/tip/a50d11a10c2db86d7383c281d4e249d5393661e9
Author:     Masami Hiramatsu <[email protected]>
AuthorDate: Thu, 26 Feb 2015 15:54:40 +0900
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Fri, 27 Feb 2015 10:08:37 -0300

perf buildid-cache: Add new buildid cache if update target is not cached

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 | 11 ++++++++---
 tools/perf/builtin-buildid-cache.c              |  6 ++++--
 tools/perf/util/build-id.c                      | 12 ++++++++++++
 tools/perf/util/build-id.h                      |  1 +
 4 files changed, 25 insertions(+), 5 deletions(-)
tools/perf/Documentation/perf-buildid-cache.txt +8 -3
index 0294c57..cec6b57 100644
@@ -41,9 +41,14 @@ OPTIONS
 --missing=::
 	List missing build ids in the cache for the specified file.
 -u::
---update::
-	Update specified file of the cache. It can be used to update kallsyms
-	kernel dso to vmlinux in order to support annotation.
+--update=::
+	Update specified file of the cache. Note that this doesn't remove
+	older entires since those may be still needed for annotating old
+	(or remote) perf.data. Only if there is already a cache which has
+	exactly same build-id, that is replaced by new one. It can be used
+	to update kallsyms and kernel dso to vmlinux in order to support
+	annotation.
+
 -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);
#3 lore raw

PATCH v5 2/4 perf buildid-cache: Add --purge FILE to remove all caches of FILE

In reply to Masami Hiramatsu

Add --purge FILE to remove all caches of FILE. Since the current --remove FILE removes a cache which has same build-id of given FILE. Since the command takes a FILE path, it can confuse user who tries to remove cache about FILE path.

-----

  # ./perf buildid-cache -v --add ./perf
  Adding 133b7b5486d987a5ab5c3ebf4ea14941f45d4d4f ./perf: Ok
  # (update the ./perf binary)
  # ./perf buildid-cache -v --remove ./perf
  Removing 305bbd1be68f66eca7e2d78db294653031edfa79 ./perf: FAIL
  ./perf wasn't in the cache
  -----
Actually, the --remove's FAIL is not shown, it just silently fails.

So, this patch adds --purge FILE action for such usecase. perf buildid-cache --purge FILE removes all caches which has same FILE path.
In other words, it removes all caches including old binaries.

-----

  # ./perf buildid-cache -v --add ./perf
  Adding 133b7b5486d987a5ab5c3ebf4ea14941f45d4d4f ./perf: Ok
  # (update the ./perf binary)
  # ./perf buildid-cache -v --purge ./perf
  Removing 133b7b5486d987a5ab5c3ebf4ea14941f45d4d4f ./perf: Ok
  -----

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

---
 tools/perf/Documentation/perf-buildid-cache.txt |   13 ++--
 tools/perf/builtin-buildid-cache.c              |   44 ++++++++++++
 tools/perf/util/build-id.c                      |   85 ++++++++++++++++++-----
 tools/perf/util/build-id.h                      |    1 
 4 files changed, 122 insertions(+), 21 deletions(-)
tools/perf/Documentation/perf-buildid-cache.txt +9 -4
index cec6b57..dd07b55 100644
@@ -12,9 +12,9 @@ SYNOPSIS
 
 DESCRIPTION
 -----------
-This command manages the build-id cache. It can add and remove files to/from
-the cache. In the future it should as well purge older entries, set upper
-limits for the space used by the cache, etc.
+This command manages the build-id cache. It can add, remove, update and purge
+files to/from the cache. In the future it should as well set upper limits for
+the space used by the cache, etc.
 
 OPTIONS
 -------
@@ -36,7 +36,12 @@ OPTIONS
         actually made.
 -r::
 --remove=::
-        Remove specified file from the cache.
+        Remove a cached binary which has same build-id of specified file
+        from the cache.
+-p::
+--purge=::
+        Purge all cached binaries including older caches which have specified
+	path from the cache.
 -M::
 --missing=::
 	List missing build ids in the cache for the specified file.
tools/perf/builtin-buildid-cache.c +44 -0

68 lines not on this page · raw message

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

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

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

Em Thu, Feb 26, 2015 at 03:54:47PM +0900, Masami Hiramatsu escreveu:

Quoted 27 lines
tools/perf/builtin-buildid-cache.c +9 -0
@@ -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,
+		   *purge_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('p', "purge", &purge_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 (purge_name_list_str) {
+		list = strlist__new(true, purge_name_list_str);
+		if (list) {
+			strlist__for_each(pos, list)
+				if (build_id_cache__purge_path(pos->s)) {
+					if (errno == ENOENT) {

tools/perf/builtin-buildid-cache.c int cmd_buildid_cache(int argc, const char **argv, · line 412

Quoted 10 lines
tools/perf/builtin-buildid-cache.c +10 -0
+						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);
+		}

Here we need to at least warn the user that what he/she asked couldn't be done, i.e. strlist__new() failed.

Seeing --remove, --purge and --update makes me think eventually we need a OPT_STRLIST() :-)

I'm going thru the other patches, the first got in already.

Thanks for implementing --purge!

- Arnaldo

Quoted 133 lines
tools/perf/builtin-buildid-cache.c +2 -0
+	}
+
 	if (missing_filename)
 		ret = build_id_cache__fprintf_missing(session, stdout);
 
tools/perf/util/build-id.c +68 -17
index 0bc33be..dcaa964 100644
@@ -281,35 +281,85 @@ 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);
+	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, *dirname = NULL,
 	     *linkname = zalloc(size), *targetname, *tmp;
-	int len, err = -1;
-	bool slash = is_kallsyms || is_vdso;
+	int err = -1;
 
-	if (is_kallsyms) {
-		if (symbol_conf.kptr_restrict) {
-			pr_debug("Not caching a kptr_restrict'ed /proc/kallsyms\n");
-			err = 0;
-			goto out_free;
-		}
-		realname = (char *) name;
-	} else
+	if (!is_kallsyms) {
 		realname = realpath(name, NULL);
+		if (!realname)
+			goto out_free;
+	}
 
-	if (realname == NULL || filename == NULL || linkname == NULL)
+	dirname = build_id_cache__dirname_from_path(name, is_kallsyms, is_vdso);
+	if (!dirname)
 		goto out_free;
 
-	len = scnprintf(filename, size, "%s%s%s",
-		       buildid_dir, slash ? "/" : "",
-		       is_vdso ? DSO__NAME_VDSO : realname);
-	if (mkdir_p(filename, 0755))
+	if (mkdir_p(dirname, 0755))
 		goto out_free;
 
-	snprintf(filename + len, size - len, "/%s", sbuild_id);
+	if (asprintf(&filename, "%s/%s", dirname, sbuild_id) < 0) {
+		filename = NULL;
+		goto out_free;
+	}
 
 	if (access(filename, F_OK)) {
 		if (is_kallsyms) {
@@ -337,6 +387,7 @@ out_free:
 	if (!is_kallsyms)
 		free(realname);
 	free(filename);
+	free(dirname);
 	free(linkname);
 	return err;
 }
tools/perf/util/build-id.h +1 -0
index 2a09498..cbcadea 100644
@@ -22,6 +22,7 @@ bool perf_session__read_build_ids(struct perf_session *session, bool with_hits);
 int perf_session__write_buildid_table(struct perf_session *session, int fd);
 int perf_session__cache_build_ids(struct perf_session *session);
 
+struct strlist *build_id_cache__list_build_ids(const char *pathname);
 bool build_id_cache__cached(const char *sbuild_id);
 int build_id_cache__add_s(const char *sbuild_id,
 			  const char *name, bool is_kallsyms, bool is_vdso);
#5 lore raw

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

Quoted 33 lines · Arnaldo Carvalho de Melo

Em Thu, Feb 26, 2015 at 03:54:47PM +0900, Masami Hiramatsu escreveu:

Quoted 32 lines
tools/perf/builtin-buildid-cache.c +14 -0
@@ -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,
+		   *purge_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('p', "purge", &purge_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 (purge_name_list_str) {
+		list = strlist__new(true, purge_name_list_str);
+		if (list) {
+			strlist__for_each(pos, list)
+				if (build_id_cache__purge_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",
Quoted 8 lines

tools/perf/builtin-buildid-cache.c int cmd_buildid_cache(int argc, const char **argv, · line 412

Quoted 5 lines
tools/perf/builtin-buildid-cache.c +5 -0
+						   pos->s, strerror_r(errno, sbuf, sizeof(sbuf)));
+				}
+
+			strlist__delete(list);
+		}

Here we need to at least warn the user that what he/she asked couldn't be done, i.e. strlist__new() failed.

Ah, OK I'll update it.

Quoted 3 lines

Seeing --remove, --purge and --update makes me think eventually we need a OPT_STRLIST() :-)

Agreed :)

Quoted 2 lines

I'm going thru the other patches, the first got in already.

Thanks!

Quoted 144 lines

Thanks for implementing --purge!

- Arnaldo

Quoted 126 lines
tools/perf/builtin-buildid-cache.c +2 -0
+	}
+
 	if (missing_filename)
 		ret = build_id_cache__fprintf_missing(session, stdout);
 
tools/perf/util/build-id.c +68 -17
index 0bc33be..dcaa964 100644
@@ -281,35 +281,85 @@ 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);
+	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, *dirname = NULL,
 	     *linkname = zalloc(size), *targetname, *tmp;
-	int len, err = -1;
-	bool slash = is_kallsyms || is_vdso;
+	int err = -1;
 
-	if (is_kallsyms) {
-		if (symbol_conf.kptr_restrict) {
-			pr_debug("Not caching a kptr_restrict'ed /proc/kallsyms\n");
-			err = 0;
-			goto out_free;
-		}
-		realname = (char *) name;
-	} else
+	if (!is_kallsyms) {
 		realname = realpath(name, NULL);
+		if (!realname)
+			goto out_free;
+	}
 
-	if (realname == NULL || filename == NULL || linkname == NULL)
+	dirname = build_id_cache__dirname_from_path(name, is_kallsyms, is_vdso);
+	if (!dirname)
 		goto out_free;
 
-	len = scnprintf(filename, size, "%s%s%s",
-		       buildid_dir, slash ? "/" : "",
-		       is_vdso ? DSO__NAME_VDSO : realname);
-	if (mkdir_p(filename, 0755))
+	if (mkdir_p(dirname, 0755))
 		goto out_free;
 
-	snprintf(filename + len, size - len, "/%s", sbuild_id);
+	if (asprintf(&filename, "%s/%s", dirname, sbuild_id) < 0) {
+		filename = NULL;
+		goto out_free;
+	}
 
 	if (access(filename, F_OK)) {
 		if (is_kallsyms) {
@@ -337,6 +387,7 @@ out_free:
 	if (!is_kallsyms)
 		free(realname);
 	free(filename);
+	free(dirname);
 	free(linkname);
 	return err;
 }
tools/perf/util/build-id.h +0 -0
index 2a09498..cbcadea 100644

tools/perf/util/build-id.h bool perf_session__read_build_ids(struct perf_session *session, bool with_hits); · line 28

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

--
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]
#6 lore raw

PATCH v5 3/4 perf-buildid-cache: Use pr_debug instead of verbose && pr_info

In reply to Masami Hiramatsu

Use pr_debug instead of the combination of verbose and pr_info.

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

---
 tools/perf/builtin-buildid-cache.c |   20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)
tools/perf/builtin-buildid-cache.c +8 -12
index 37182bb..20be743 100644
@@ -196,9 +196,8 @@ static int build_id_cache__add_file(const char *filename)
 	build_id__sprintf(build_id, sizeof(build_id), sbuild_id);
 	err = build_id_cache__add_s(sbuild_id, filename,
 				    false, false);
-	if (verbose)
-		pr_info("Adding %s %s: %s\n", sbuild_id, filename,
-			err ? "FAIL" : "Ok");
+	pr_debug("Adding %s %s: %s\n", sbuild_id, filename,
+		 err ? "FAIL" : "Ok");
 	return err;
 }
 
@@ -216,9 +215,8 @@ static int build_id_cache__remove_file(const char *filename)
 
 	build_id__sprintf(build_id, sizeof(build_id), sbuild_id);
 	err = build_id_cache__remove_s(sbuild_id);
-	if (verbose)
-		pr_info("Removing %s %s: %s\n", sbuild_id, filename,
-			err ? "FAIL" : "Ok");
+	pr_debug("Removing %s %s: %s\n", sbuild_id, filename,
+		 err ? "FAIL" : "Ok");
 
 	return err;
 }
@@ -235,9 +233,8 @@ static int build_id_cache__purge_path(const char *pathname)
 
 	strlist__for_each(pos, list) {
 		err = build_id_cache__remove_s(pos->s);
-		if (verbose)
-			pr_info("Removing %s %s: %s\n", pos->s, pathname,
-				err ? "FAIL" : "Ok");
+		pr_debug("Removing %s %s: %s\n", pos->s, pathname,
+			 err ? "FAIL" : "Ok");
 		if (err)
 			break;
 	}
@@ -292,9 +289,8 @@ static int build_id_cache__update_file(const char *filename)
 	if (!err)
 		err = build_id_cache__add_s(sbuild_id, filename, false, false);
 
-	if (verbose)
-		pr_info("Updating %s %s: %s\n", sbuild_id, filename,
-			err ? "FAIL" : "Ok");
+	pr_debug("Updating %s %s: %s\n", sbuild_id, filename,
+		 err ? "FAIL" : "Ok");
 
 	return err;
 }
#7 lore raw

Re: [perf/core PATCH v5 3/4] perf-buildid-cache: Use pr_debug instead of verbose && pr_info

Em Thu, Feb 26, 2015 at 03:54:54PM +0900, Masami Hiramatsu escreveu:

Quoted 3 lines

Use pr_debug instead of the combination of verbose and pr_info.

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

This one doesn't apply because 2/4 got delayed.

- Arnaldo

#8 lore raw

PATCH v5 4/4 perf-buildid-cache: Show usage with incorrect params

In reply to Masami Hiramatsu

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

---
 tools/perf/builtin-buildid-cache.c |    5 +++++
 1 file changed, 5 insertions(+)
tools/perf/builtin-buildid-cache.c +5 -0
index 20be743..5eaa9bf 100644
@@ -340,6 +340,11 @@ int cmd_buildid_cache(int argc, const char **argv,
 	argc = parse_options(argc, argv, buildid_cache_options,
 			     buildid_cache_usage, 0);
 
+	if (argc || (!add_name_list_str && !kcore_filename &&
+		     !remove_name_list_str && !purge_name_list_str &&
+		     !missing_filename && !update_name_list_str))
+		usage_with_options(buildid_cache_usage, buildid_cache_options);
+
 	if (missing_filename) {
 		file.path = missing_filename;
 		file.force = force;
#9 lore raw

Re: [perf/core PATCH v5 4/4] perf-buildid-cache: Show usage with incorrect params

Em Thu, Feb 26, 2015 at 03:55:01PM +0900, Masami Hiramatsu escreveu:

Quoted 2 lines

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

Thanks for doing that!

Doesn't apply due to previous patches being delayer at the moment.

- Arnaldo

Quoted 22 lines
---
 tools/perf/builtin-buildid-cache.c |    5 +++++
 1 file changed, 5 insertions(+)
tools/perf/builtin-buildid-cache.c +5 -0
index 20be743..5eaa9bf 100644
@@ -340,6 +340,11 @@ int cmd_buildid_cache(int argc, const char **argv,
 	argc = parse_options(argc, argv, buildid_cache_options,
 			     buildid_cache_usage, 0);
 
+	if (argc || (!add_name_list_str && !kcore_filename &&
+		     !remove_name_list_str && !purge_name_list_str &&
+		     !missing_filename && !update_name_list_str))
+		usage_with_options(buildid_cache_usage, buildid_cache_options);
+
 	if (missing_filename) {
 		file.path = missing_filename;
 		file.force = force;