Re: Re: [perf/core PATCH 4/4] perf buildid-cache: Add --remove-all FILE to remove all caches of FILE
From: Masami Hiramatsu
Date: Mon Feb 09 2015 - 18:36:18 EST
(2015/02/09 12:22), Namhyung Kim wrote:
> Hi Masami,
>
> On Sat, Feb 07, 2015 at 07:14:19PM +0900, Masami Hiramatsu wrote:
>> Add --remove-all FILE to remove all caches of FILE.
>> Since the current --remove FILE removes a cache which has
>> same build-id of given FILE. Since the command takes a
>> FILE path, it can confuse user who tries to remove cache
>> about FILE path.
>>
>> -----
>> # ./perf buildid-cache -v --add ./perf
>> Adding 133b7b5486d987a5ab5c3ebf4ea14941f45d4d4f ./perf: Ok
>> # (update the ./perf binary)
>> # ./perf buildid-cache -v --remove ./perf
>> Removing 305bbd1be68f66eca7e2d78db294653031edfa79 ./perf: FAIL
>> ./perf wasn't in the cache
>> -----
>> Actually, the --remove's FAIL is not shown, it just silently fails.
>>
>> So, this patch adds --remove-all FILE action for such usecase.
>> perf buildid-cache --remove-all FILE removes all caches which
>> has same FILE path.
>> In other words, it removes all caches including old binaries.
>>
>> -----
>> # ./perf buildid-cache -v --add ./perf
>> Adding 133b7b5486d987a5ab5c3ebf4ea14941f45d4d4f ./perf: Ok
>> # (update the ./perf binary)
>> # ./perf buildid-cache -v --remove-all ./perf
>> Removing 133b7b5486d987a5ab5c3ebf4ea14941f45d4d4f ./perf: Ok
>> -----
>>
>> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@xxxxxxxxxxx>
>> ---
>> tools/perf/Documentation/perf-buildid-cache.txt | 6 +-
>> tools/perf/builtin-buildid-cache.c | 44 ++++++++++++
>> tools/perf/util/build-id.c | 81 ++++++++++++++++++-----
>> tools/perf/util/build-id.h | 1
>> 4 files changed, 115 insertions(+), 17 deletions(-)
>>
>> diff --git a/tools/perf/Documentation/perf-buildid-cache.txt b/tools/perf/Documentation/perf-buildid-cache.txt
>> index 6575dce..01f295e 100644
>> --- a/tools/perf/Documentation/perf-buildid-cache.txt
>> +++ b/tools/perf/Documentation/perf-buildid-cache.txt
>> @@ -36,7 +36,11 @@ OPTIONS
>> actually made.
>> -r::
>> --remove=::
>> - Remove specified file from the cache.
>> + Remove a cached binary which has same build-id of specified file
>> + from the cache.
>> +-R::
>> +--remove-all=::
>> + Remove all cached binary which has specified path from the cache.
>> -M::
>> --missing=::
>> List missing build ids in the cache for the specified file.
>> diff --git a/tools/perf/builtin-buildid-cache.c b/tools/perf/builtin-buildid-cache.c
>> index e7568f5..3a76d51 100644
>> --- a/tools/perf/builtin-buildid-cache.c
>> +++ b/tools/perf/builtin-buildid-cache.c
>> @@ -223,6 +223,29 @@ static int build_id_cache__remove_file(const char *filename)
>> return err;
>> }
>>
>> +static int build_id_cache__remove_path(const char *pathname)
>> +{
>> + struct strlist *list;
>> + struct str_node *pos;
>> + int err;
>> +
>> + list = build_id_cache__list_build_ids(pathname);
>> + if (!list)
>> + return 0;
>> +
>> + strlist__for_each(pos, list) {
>> + err = build_id_cache__remove_s(pos->s);
>> + if (verbose)
>> + pr_info("Removing %s %s: %s\n", pos->s, pathname,
>> + err ? "FAIL" : "Ok");
>> + if (err)
>> + break;
>> + }
>> + strlist__delete(list);
>> +
>> + return err;
>> +}
>> +
>> static bool dso__missing_buildid_cache(struct dso *dso, int parm __maybe_unused)
>> {
>> char filename[PATH_MAX];
>> @@ -285,6 +308,7 @@ int cmd_buildid_cache(int argc, const char **argv,
>> bool force = false;
>> char const *add_name_list_str = NULL,
>> *remove_name_list_str = NULL,
>> + *remove_all_name_list_str = NULL,
>> *missing_filename = NULL,
>> *update_name_list_str = NULL,
>> *kcore_filename = NULL;
>> @@ -302,6 +326,8 @@ int cmd_buildid_cache(int argc, const char **argv,
>> "file", "kcore file to add"),
>> OPT_STRING('r', "remove", &remove_name_list_str, "file list",
>> "file(s) to remove"),
>> + OPT_STRING('R', "remove-all", &remove_all_name_list_str, "path list",
>> + "path(s) to remove (remove old caches too)"),
>> OPT_STRING('M', "missing", &missing_filename, "file",
>> "to find missing build ids in the cache"),
>> OPT_BOOLEAN('f', "force", &force, "don't complain, do it"),
>> @@ -368,6 +394,24 @@ int cmd_buildid_cache(int argc, const char **argv,
>> }
>> }
>>
>> + if (remove_all_name_list_str) {
>> + list = strlist__new(true, remove_all_name_list_str);
>> + if (list) {
>> + strlist__for_each(pos, list)
>> + if (build_id_cache__remove_path(pos->s)) {
>> + if (errno == ENOENT) {
>> + pr_debug("%s wasn't in the cache\n",
>> + pos->s);
>> + continue;
>> + }
>> + pr_warning("Couldn't remove %s: %s\n",
>> + pos->s, strerror_r(errno, sbuf, sizeof(sbuf)));
>> + }
>> +
>> + strlist__delete(list);
>> + }
>> + }
>> +
>> if (missing_filename)
>> ret = build_id_cache__fprintf_missing(session, stdout);
>>
>> diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
>> index 0bc33be..e688311 100644
>> --- a/tools/perf/util/build-id.c
>> +++ b/tools/perf/util/build-id.c
>> @@ -281,35 +281,84 @@ 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;
>> + free(realname);
>
> Shouldn't it be:
>
> if (!slash)
> free(realname);
Ah! right:)
Thank you
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@xxxxxxxxxxx
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/