From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S933391AbbBIXgP (ORCPT ); Mon, 9 Feb 2015 18:36:15 -0500 Received: from mail7.hitachi.co.jp ([133.145.228.42]:47705 "EHLO mail7.hitachi.co.jp" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932911AbbBIXgO (ORCPT ); Mon, 9 Feb 2015 18:36:14 -0500 Message-ID: <54D94466.2000804@hitachi.com> Date: Tue, 10 Feb 2015 08:36:06 +0900 From: Masami Hiramatsu Organization: Hitachi, Ltd., Japan User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:13.0) Gecko/20120614 Thunderbird/13.0.1 MIME-Version: 1.0 To: Namhyung Kim Cc: Arnaldo Carvalho de Melo , Peter Zijlstra , Adrian Hunter , linux-kernel@vger.kernel.org, Ingo Molnar , Paul Mackerras , Jiri Olsa , Borislav Petkov , Hemant Kumar Subject: Re: Re: [perf/core PATCH 4/4] perf buildid-cache: Add --remove-all FILE to remove all caches of FILE References: <20150207101411.11255.46578.stgit@localhost.localdomain> <20150207101419.11255.18296.stgit@localhost.localdomain> <20150209032253.GG24182@sejong> In-Reply-To: <20150209032253.GG24182@sejong> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org (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 >> --- >> 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@hitachi.com