Subject: [perf/core PATCH v3 0/2] perf-buildid-cache: Enhance --update and add --purge

Hi,

Here is the 3rd version of of perf buildid-cache update.
This has a small updates for each patch. According to
Arnaldo's comment, I updated the document about --update
option in the first patch, and renamed --remove-all to
--purge in the second patch (it also updated document).

Thank you,


---

Masami Hiramatsu (2):
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


tools/perf/Documentation/perf-buildid-cache.txt | 24 ++++--
tools/perf/builtin-buildid-cache.c | 50 ++++++++++++
tools/perf/util/build-id.c | 95 +++++++++++++++++++----
tools/perf/util/build-id.h | 2
4 files changed, 146 insertions(+), 25 deletions(-)

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


Subject: [perf/core PATCH v3 1/2] 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
#
----

Signed-off-by: Masami Hiramatsu <[email protected]>
---
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(-)

diff --git a/tools/perf/Documentation/perf-buildid-cache.txt b/tools/perf/Documentation/perf-buildid-cache.txt
index 0294c57..cec6b57 100644
--- a/tools/perf/Documentation/perf-buildid-cache.txt
+++ b/tools/perf/Documentation/perf-buildid-cache.txt
@@ -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.
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);

Subject: [perf/core PATCH v3 2/2] perf buildid-cache: Add --purge FILE to remove all caches of FILE

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

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

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

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

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

Signed-off-by: Masami Hiramatsu <[email protected]>
---
tools/perf/Documentation/perf-buildid-cache.txt | 13 ++--
tools/perf/builtin-buildid-cache.c | 44 ++++++++++++
tools/perf/util/build-id.c | 83 +++++++++++++++++++----
tools/perf/util/build-id.h | 1
4 files changed, 121 insertions(+), 20 deletions(-)

diff --git a/tools/perf/Documentation/perf-buildid-cache.txt b/tools/perf/Documentation/perf-buildid-cache.txt
index cec6b57..dd07b55 100644
--- a/tools/perf/Documentation/perf-buildid-cache.txt
+++ b/tools/perf/Documentation/perf-buildid-cache.txt
@@ -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.
diff --git a/tools/perf/builtin-buildid-cache.c b/tools/perf/builtin-buildid-cache.c
index e7568f5..37182bb 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__purge_path(const char *pathname)
+{
+ struct strlist *list;
+ struct str_node *pos;
+ int err;
+
+ list = build_id_cache__list_build_ids(pathname);
+ if (!list)
+ return 0;
+
+ 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,
+ *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",
+ 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);

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

(2015/02/19 17:08), Masami Hiramatsu wrote:
> Add --purge FILE to remove all caches of FILE.
> Since the current --remove FILE removes a cache which has
> same build-id of given FILE. Since the command takes a
> FILE path, it can confuse user who tries to remove cache
> about FILE path.
>
> -----
> # ./perf buildid-cache -v --add ./perf
> Adding 133b7b5486d987a5ab5c3ebf4ea14941f45d4d4f ./perf: Ok
> # (update the ./perf binary)
> # ./perf buildid-cache -v --remove ./perf
> Removing 305bbd1be68f66eca7e2d78db294653031edfa79 ./perf: FAIL
> ./perf wasn't in the cache
> -----
> Actually, the --remove's FAIL is not shown, it just silently fails.
>
> So, this patch adds --purge FILE action for such usecase.
> perf buildid-cache --purge FILE removes all caches which
> has same FILE path.
> In other words, it removes all caches including old binaries.
>
> -----
> # ./perf buildid-cache -v --add ./perf
> Adding 133b7b5486d987a5ab5c3ebf4ea14941f45d4d4f ./perf: Ok
> # (update the ./perf binary)
> # ./perf buildid-cache -v --purge ./perf
> Removing 133b7b5486d987a5ab5c3ebf4ea14941f45d4d4f ./perf: Ok
> -----
>
> BTW, if you want to purge all the caches, remove ~/.debug/* .
>
> Signed-off-by: Masami Hiramatsu <[email protected]>
> ---
> tools/perf/Documentation/perf-buildid-cache.txt | 13 ++--
> tools/perf/builtin-buildid-cache.c | 44 ++++++++++++
> tools/perf/util/build-id.c | 83 +++++++++++++++++++----
> tools/perf/util/build-id.h | 1
> 4 files changed, 121 insertions(+), 20 deletions(-)
>
> diff --git a/tools/perf/Documentation/perf-buildid-cache.txt b/tools/perf/Documentation/perf-buildid-cache.txt
> index cec6b57..dd07b55 100644
> --- a/tools/perf/Documentation/perf-buildid-cache.txt
> +++ b/tools/perf/Documentation/perf-buildid-cache.txt
> @@ -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.
> diff --git a/tools/perf/builtin-buildid-cache.c b/tools/perf/builtin-buildid-cache.c
> index e7568f5..37182bb 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__purge_path(const char *pathname)
> +{
> + struct strlist *list;
> + struct str_node *pos;
> + int err;
> +
> + list = build_id_cache__list_build_ids(pathname);
> + if (!list)
> + return 0;
> +
> + 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,
> + *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",
> + 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) {

Oops, here is a memory leak. I'll update this.

Thank you,


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