2018-04-17 04:15:42

by Ravi Bangoria

[permalink] [raw]
Subject: [PATCH v2 0/3] perf/buildid-cache: Add --list and --purge-all options

First patch is a trivial error message fix. Second and third
adds new options --list and --purge-all to 'buildid-cache'
subcommand.

v2 changes:
- [PATCH v2 2/3] Display optput of 'perf buildid-cache -l' same as
'perf buildid-list'.
- [PATCH v2 2/3] Other minor changes as suggested by Jiri.

v1 can be found at:
https://lkml.org/lkml/2018/4/9/295

Ravi Bangoria (3):
tools/parse-options: Add '\n' at the end of error messages
perf/buildid-cache: Support --list option
perf/buildid-cache: Support --purge-all option

tools/lib/subcmd/parse-options.c | 6 +-
tools/perf/Documentation/perf-buildid-cache.txt | 7 ++-
tools/perf/builtin-buildid-cache.c | 77 ++++++++++++++++++++++++-
3 files changed, 83 insertions(+), 7 deletions(-)

--
2.14.3



2018-04-17 04:15:44

by Ravi Bangoria

[permalink] [raw]
Subject: [PATCH v2 2/3] perf/buildid-cache: Support --list option

Perf buildid-cache allows to add/remove files into cache but there
is no option to list all cached files. Add --list option to list
all _valid_ cached files.

Ex,
# perf buildid-cache --add /tmp/a.out
# perf buildid-cache -l
8a86ef73e44067bca52cc3f6cd3e5446c783391c /tmp/a.out

Signed-off-by: Ravi Bangoria <[email protected]>
---
tools/perf/Documentation/perf-buildid-cache.txt | 4 ++-
tools/perf/builtin-buildid-cache.c | 43 +++++++++++++++++++++++--
2 files changed, 43 insertions(+), 4 deletions(-)

diff --git a/tools/perf/Documentation/perf-buildid-cache.txt b/tools/perf/Documentation/perf-buildid-cache.txt
index 73c2650bd0db..3f285ba6e1f9 100644
--- a/tools/perf/Documentation/perf-buildid-cache.txt
+++ b/tools/perf/Documentation/perf-buildid-cache.txt
@@ -59,7 +59,9 @@ OPTIONS
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.
-
+-l::
+--list::
+ List all valid binaries from cache.
-v::
--verbose::
Be more verbose.
diff --git a/tools/perf/builtin-buildid-cache.c b/tools/perf/builtin-buildid-cache.c
index 41db2cba77eb..fd0a08661b42 100644
--- a/tools/perf/builtin-buildid-cache.c
+++ b/tools/perf/builtin-buildid-cache.c
@@ -25,6 +25,7 @@
#include "util/session.h"
#include "util/symbol.h"
#include "util/time-utils.h"
+#include "util/probe-file.h"

static int build_id_cache__kcore_buildid(const char *proc_dir, char *sbuildid)
{
@@ -297,6 +298,26 @@ static int build_id_cache__update_file(const char *filename, struct nsinfo *nsi)
return err;
}

+static int build_id_cache__show_all(void)
+{
+ struct strlist *bidlist;
+ struct str_node *nd;
+ char *buf;
+
+ bidlist = build_id_cache__list_all(true);
+ if (!bidlist) {
+ pr_debug("Failed to get buildids: -%d\n", errno);
+ return -1;
+ }
+ strlist__for_each_entry(nd, bidlist) {
+ buf = build_id_cache__origname(nd->s);
+ fprintf(stdout, "%s %s\n", nd->s, buf);
+ free(buf);
+ }
+ strlist__delete(bidlist);
+ return 0;
+}
+
int cmd_buildid_cache(int argc, const char **argv)
{
struct strlist *list;
@@ -304,6 +325,8 @@ int cmd_buildid_cache(int argc, const char **argv)
int ret = 0;
int ns_id = -1;
bool force = false;
+ bool list_files = false;
+ bool opts_flag = false;
char const *add_name_list_str = NULL,
*remove_name_list_str = NULL,
*purge_name_list_str = NULL,
@@ -327,6 +350,7 @@ int cmd_buildid_cache(int argc, const char **argv)
"file(s) to remove"),
OPT_STRING('p', "purge", &purge_name_list_str, "file list",
"file(s) to remove (remove old caches too)"),
+ OPT_BOOLEAN('l', "list", &list_files, "list all cached files"),
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"),
@@ -344,11 +368,19 @@ 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))
+ opts_flag = add_name_list_str || kcore_filename ||
+ remove_name_list_str || purge_name_list_str ||
+ missing_filename || update_name_list_str;
+
+ if (argc || !(list_files || opts_flag))
usage_with_options(buildid_cache_usage, buildid_cache_options);

+ /* -l is exclusive. It can not be used with other options. */
+ if (list_files && opts_flag) {
+ usage_with_options_msg(buildid_cache_usage,
+ buildid_cache_options, "-l is exclusive.\n");
+ }
+
if (ns_id > 0)
nsi = nsinfo__new(ns_id);

@@ -366,6 +398,11 @@ int cmd_buildid_cache(int argc, const char **argv)

setup_pager();

+ if (list_files) {
+ ret = build_id_cache__show_all();
+ goto out;
+ }
+
if (add_name_list_str) {
list = strlist__new(add_name_list_str, NULL);
if (list) {
--
2.14.3


2018-04-17 04:15:55

by Ravi Bangoria

[permalink] [raw]
Subject: [PATCH v2 3/3] perf/buildid-cache: Support --purge-all option

User can remove files from cache using --remove/--purge options
but both needs list of files as an argument. It's not convenient
when you want to flush out entire cache. Add an option to purge
all files from cache.

Ex,
# perf buildid-cache -l
8a86ef73e44067bca52cc3f6cd3e5446c783391c /tmp/a.out
ebe71fdcf4b366518cc154d570a33cd461a51c36 /tmp/a.out.1
# perf buildid-cache -P -v
Removing /tmp/a.out (8a86ef73e44067bca52cc3f6cd3e5446c783391c): Ok
Removing /tmp/a.out.1 (ebe71fdcf4b366518cc154d570a33cd461a51c36): Ok
Purged all: Ok

Signed-off-by: Ravi Bangoria <[email protected]>
---
tools/perf/Documentation/perf-buildid-cache.txt | 3 +++
tools/perf/builtin-buildid-cache.c | 36 ++++++++++++++++++++++++-
2 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/tools/perf/Documentation/perf-buildid-cache.txt b/tools/perf/Documentation/perf-buildid-cache.txt
index 3f285ba6e1f9..f6de0952ff3c 100644
--- a/tools/perf/Documentation/perf-buildid-cache.txt
+++ b/tools/perf/Documentation/perf-buildid-cache.txt
@@ -48,6 +48,9 @@ OPTIONS
--purge=::
Purge all cached binaries including older caches which have specified
path from the cache.
+-P::
+--purge-all::
+ Purge all cached binaries. This will flush out entire 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 fd0a08661b42..d11226f76fbc 100644
--- a/tools/perf/builtin-buildid-cache.c
+++ b/tools/perf/builtin-buildid-cache.c
@@ -240,6 +240,34 @@ static int build_id_cache__purge_path(const char *pathname, struct nsinfo *nsi)
return err;
}

+static int build_id_cache__purge_all(void)
+{
+ struct strlist *list;
+ struct str_node *pos;
+ int err;
+ char *buf;
+
+ list = build_id_cache__list_all(false);
+ if (!list) {
+ pr_debug("Failed to get buildids: -%d\n", errno);
+ return -EINVAL;
+ }
+
+ strlist__for_each_entry(pos, list) {
+ buf = build_id_cache__origname(pos->s);
+ err = build_id_cache__remove_s(pos->s);
+ pr_debug("Removing %s (%s): %s\n", buf, pos->s,
+ err ? "FAIL" : "Ok");
+ free(buf);
+ if (err)
+ break;
+ }
+ strlist__delete(list);
+
+ pr_debug("Purged all: %s\n", err ? "FAIL" : "Ok");
+ return err;
+}
+
static bool dso__missing_buildid_cache(struct dso *dso, int parm __maybe_unused)
{
char filename[PATH_MAX];
@@ -327,6 +355,7 @@ int cmd_buildid_cache(int argc, const char **argv)
bool force = false;
bool list_files = false;
bool opts_flag = false;
+ bool purge_all = false;
char const *add_name_list_str = NULL,
*remove_name_list_str = NULL,
*purge_name_list_str = NULL,
@@ -350,6 +379,7 @@ int cmd_buildid_cache(int argc, const char **argv)
"file(s) to remove"),
OPT_STRING('p', "purge", &purge_name_list_str, "file list",
"file(s) to remove (remove old caches too)"),
+ OPT_BOOLEAN('P', "purge-all", &purge_all, "purge all cached files"),
OPT_BOOLEAN('l', "list", &list_files, "list all cached files"),
OPT_STRING('M', "missing", &missing_filename, "file",
"to find missing build ids in the cache"),
@@ -370,7 +400,8 @@ int cmd_buildid_cache(int argc, const char **argv)

opts_flag = add_name_list_str || kcore_filename ||
remove_name_list_str || purge_name_list_str ||
- missing_filename || update_name_list_str;
+ missing_filename || update_name_list_str ||
+ purge_all;

if (argc || !(list_files || opts_flag))
usage_with_options(buildid_cache_usage, buildid_cache_options);
@@ -457,6 +488,9 @@ int cmd_buildid_cache(int argc, const char **argv)
}
}

+ if (purge_all)
+ ret = build_id_cache__purge_all();
+
if (missing_filename)
ret = build_id_cache__fprintf_missing(session, stdout);

--
2.14.3


2018-04-17 04:16:07

by Ravi Bangoria

[permalink] [raw]
Subject: [PATCH v2 1/3] tools/parse-options: Add '\n' at the end of error messages

Few error messages does not have '\n' at the end and thus next
prompt gets printed in the same line. Ex,

linux~$ perf buildid-cache -verbose --add ./a.out
Error: did you mean `--verbose` (with two dashes ?)linux~$

Fix it.

Signed-off-by: Ravi Bangoria <[email protected]>
Reviewed-by: Masami Hiramatsu <[email protected]>
---
tools/lib/subcmd/parse-options.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/lib/subcmd/parse-options.c b/tools/lib/subcmd/parse-options.c
index f6a1babcbac4..cb7154eccbdc 100644
--- a/tools/lib/subcmd/parse-options.c
+++ b/tools/lib/subcmd/parse-options.c
@@ -433,7 +433,7 @@ static int parse_long_opt(struct parse_opt_ctx_t *p, const char *arg,

if (ambiguous_option) {
fprintf(stderr,
- " Error: Ambiguous option: %s (could be --%s%s or --%s%s)",
+ " Error: Ambiguous option: %s (could be --%s%s or --%s%s)\n",
arg,
(ambiguous_flags & OPT_UNSET) ? "no-" : "",
ambiguous_option->long_name,
@@ -458,7 +458,7 @@ static void check_typos(const char *arg, const struct option *options)
return;

if (strstarts(arg, "no-")) {
- fprintf(stderr, " Error: did you mean `--%s` (with two dashes ?)", arg);
+ fprintf(stderr, " Error: did you mean `--%s` (with two dashes ?)\n", arg);
exit(129);
}

@@ -466,7 +466,7 @@ static void check_typos(const char *arg, const struct option *options)
if (!options->long_name)
continue;
if (strstarts(options->long_name, arg)) {
- fprintf(stderr, " Error: did you mean `--%s` (with two dashes ?)", arg);
+ fprintf(stderr, " Error: did you mean `--%s` (with two dashes ?)\n", arg);
exit(129);
}
}
--
2.14.3


2018-04-17 07:59:24

by Jiri Olsa

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] perf/buildid-cache: Add --list and --purge-all options

On Tue, Apr 17, 2018 at 09:43:43AM +0530, Ravi Bangoria wrote:
> First patch is a trivial error message fix. Second and third
> adds new options --list and --purge-all to 'buildid-cache'
> subcommand.
>
> v2 changes:
> - [PATCH v2 2/3] Display optput of 'perf buildid-cache -l' same as
> 'perf buildid-list'.
> - [PATCH v2 2/3] Other minor changes as suggested by Jiri.

Acked-by: Jiri Olsa <[email protected]>

thanks,
jirka

>
> v1 can be found at:
> https://lkml.org/lkml/2018/4/9/295
>
> Ravi Bangoria (3):
> tools/parse-options: Add '\n' at the end of error messages
> perf/buildid-cache: Support --list option
> perf/buildid-cache: Support --purge-all option
>
> tools/lib/subcmd/parse-options.c | 6 +-
> tools/perf/Documentation/perf-buildid-cache.txt | 7 ++-
> tools/perf/builtin-buildid-cache.c | 77 ++++++++++++++++++++++++-
> 3 files changed, 83 insertions(+), 7 deletions(-)
>
> --
> 2.14.3
>

2018-04-17 14:00:13

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] perf/buildid-cache: Add --list and --purge-all options

Em Tue, Apr 17, 2018 at 09:57:50AM +0200, Jiri Olsa escreveu:
> On Tue, Apr 17, 2018 at 09:43:43AM +0530, Ravi Bangoria wrote:
> > First patch is a trivial error message fix. Second and third
> > adds new options --list and --purge-all to 'buildid-cache'
> > subcommand.
> >
> > v2 changes:
> > - [PATCH v2 2/3] Display optput of 'perf buildid-cache -l' same as
> > 'perf buildid-list'.
> > - [PATCH v2 2/3] Other minor changes as suggested by Jiri.
>
> Acked-by: Jiri Olsa <[email protected]>

Thanks, applied 1/3 to perf/urgent, 2-3/3 to perf/core.

- Arnaldo

2018-04-18 05:56:02

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [PATCH v2 0/3] perf/buildid-cache: Add --list and --purge-all options

On Tue, 17 Apr 2018 09:43:43 +0530
Ravi Bangoria <[email protected]> wrote:

> First patch is a trivial error message fix. Second and third
> adds new options --list and --purge-all to 'buildid-cache'
> subcommand.
>
> v2 changes:
> - [PATCH v2 2/3] Display optput of 'perf buildid-cache -l' same as
> 'perf buildid-list'.
> - [PATCH v2 2/3] Other minor changes as suggested by Jiri.
>
> v1 can be found at:
> https://lkml.org/lkml/2018/4/9/295

All patches in this series looks good to me.

Acked-by: Masami Hiramatsu <[email protected]>

Thanks,

>
> Ravi Bangoria (3):
> tools/parse-options: Add '\n' at the end of error messages
> perf/buildid-cache: Support --list option
> perf/buildid-cache: Support --purge-all option
>
> tools/lib/subcmd/parse-options.c | 6 +-
> tools/perf/Documentation/perf-buildid-cache.txt | 7 ++-
> tools/perf/builtin-buildid-cache.c | 77 ++++++++++++++++++++++++-
> 3 files changed, 83 insertions(+), 7 deletions(-)
>
> --
> 2.14.3
>


--
Masami Hiramatsu <[email protected]>

Subject: [tip:perf/urgent] perf tools: Add '\n' at the end of parse-options error messages

Commit-ID: 66f5a0779af2a4f28b1832f6c20bc9d3b1ab1886
Gitweb: https://git.kernel.org/tip/66f5a0779af2a4f28b1832f6c20bc9d3b1ab1886
Author: Ravi Bangoria <[email protected]>
AuthorDate: Tue, 17 Apr 2018 09:43:44 +0530
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Wed, 18 Apr 2018 15:35:51 -0300

perf tools: Add '\n' at the end of parse-options error messages

Few error messages does not have '\n' at the end and thus next prompt
gets printed in the same line. Ex,

linux~$ perf buildid-cache -verbose --add ./a.out
Error: did you mean `--verbose` (with two dashes ?)linux~$

Fix it.

Signed-off-by: Ravi Bangoria <[email protected]>
Reviewed-by: Masami Hiramatsu <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Kate Stewart <[email protected]>
Cc: Krister Johansen <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Philippe Ombredanne <[email protected]>
Cc: Sihyeon Jang <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/lib/subcmd/parse-options.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/lib/subcmd/parse-options.c b/tools/lib/subcmd/parse-options.c
index f6a1babcbac4..cb7154eccbdc 100644
--- a/tools/lib/subcmd/parse-options.c
+++ b/tools/lib/subcmd/parse-options.c
@@ -433,7 +433,7 @@ match:

if (ambiguous_option) {
fprintf(stderr,
- " Error: Ambiguous option: %s (could be --%s%s or --%s%s)",
+ " Error: Ambiguous option: %s (could be --%s%s or --%s%s)\n",
arg,
(ambiguous_flags & OPT_UNSET) ? "no-" : "",
ambiguous_option->long_name,
@@ -458,7 +458,7 @@ static void check_typos(const char *arg, const struct option *options)
return;

if (strstarts(arg, "no-")) {
- fprintf(stderr, " Error: did you mean `--%s` (with two dashes ?)", arg);
+ fprintf(stderr, " Error: did you mean `--%s` (with two dashes ?)\n", arg);
exit(129);
}

@@ -466,7 +466,7 @@ static void check_typos(const char *arg, const struct option *options)
if (!options->long_name)
continue;
if (strstarts(options->long_name, arg)) {
- fprintf(stderr, " Error: did you mean `--%s` (with two dashes ?)", arg);
+ fprintf(stderr, " Error: did you mean `--%s` (with two dashes ?)\n", arg);
exit(129);
}
}

2018-04-23 07:02:38

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [PATCH v2 2/3] perf/buildid-cache: Support --list option

On Tue, 17 Apr 2018 09:43:45 +0530
Ravi Bangoria <[email protected]> wrote:

> Perf buildid-cache allows to add/remove files into cache but there
> is no option to list all cached files. Add --list option to list
> all _valid_ cached files.
>
> Ex,
> # perf buildid-cache --add /tmp/a.out
> # perf buildid-cache -l
> 8a86ef73e44067bca52cc3f6cd3e5446c783391c /tmp/a.out
>

Looks good to me.

Acked-by: Masami Hiramatsu <[email protected]>

Thanks!

> Signed-off-by: Ravi Bangoria <[email protected]>
> ---
> tools/perf/Documentation/perf-buildid-cache.txt | 4 ++-
> tools/perf/builtin-buildid-cache.c | 43 +++++++++++++++++++++++--
> 2 files changed, 43 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/Documentation/perf-buildid-cache.txt b/tools/perf/Documentation/perf-buildid-cache.txt
> index 73c2650bd0db..3f285ba6e1f9 100644
> --- a/tools/perf/Documentation/perf-buildid-cache.txt
> +++ b/tools/perf/Documentation/perf-buildid-cache.txt
> @@ -59,7 +59,9 @@ OPTIONS
> 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.
> -
> +-l::
> +--list::
> + List all valid binaries from cache.
> -v::
> --verbose::
> Be more verbose.
> diff --git a/tools/perf/builtin-buildid-cache.c b/tools/perf/builtin-buildid-cache.c
> index 41db2cba77eb..fd0a08661b42 100644
> --- a/tools/perf/builtin-buildid-cache.c
> +++ b/tools/perf/builtin-buildid-cache.c
> @@ -25,6 +25,7 @@
> #include "util/session.h"
> #include "util/symbol.h"
> #include "util/time-utils.h"
> +#include "util/probe-file.h"
>
> static int build_id_cache__kcore_buildid(const char *proc_dir, char *sbuildid)
> {
> @@ -297,6 +298,26 @@ static int build_id_cache__update_file(const char *filename, struct nsinfo *nsi)
> return err;
> }
>
> +static int build_id_cache__show_all(void)
> +{
> + struct strlist *bidlist;
> + struct str_node *nd;
> + char *buf;
> +
> + bidlist = build_id_cache__list_all(true);
> + if (!bidlist) {
> + pr_debug("Failed to get buildids: -%d\n", errno);
> + return -1;
> + }
> + strlist__for_each_entry(nd, bidlist) {
> + buf = build_id_cache__origname(nd->s);
> + fprintf(stdout, "%s %s\n", nd->s, buf);
> + free(buf);
> + }
> + strlist__delete(bidlist);
> + return 0;
> +}
> +
> int cmd_buildid_cache(int argc, const char **argv)
> {
> struct strlist *list;
> @@ -304,6 +325,8 @@ int cmd_buildid_cache(int argc, const char **argv)
> int ret = 0;
> int ns_id = -1;
> bool force = false;
> + bool list_files = false;
> + bool opts_flag = false;
> char const *add_name_list_str = NULL,
> *remove_name_list_str = NULL,
> *purge_name_list_str = NULL,
> @@ -327,6 +350,7 @@ int cmd_buildid_cache(int argc, const char **argv)
> "file(s) to remove"),
> OPT_STRING('p', "purge", &purge_name_list_str, "file list",
> "file(s) to remove (remove old caches too)"),
> + OPT_BOOLEAN('l', "list", &list_files, "list all cached files"),
> 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"),
> @@ -344,11 +368,19 @@ 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))
> + opts_flag = add_name_list_str || kcore_filename ||
> + remove_name_list_str || purge_name_list_str ||
> + missing_filename || update_name_list_str;
> +
> + if (argc || !(list_files || opts_flag))
> usage_with_options(buildid_cache_usage, buildid_cache_options);
>
> + /* -l is exclusive. It can not be used with other options. */
> + if (list_files && opts_flag) {
> + usage_with_options_msg(buildid_cache_usage,
> + buildid_cache_options, "-l is exclusive.\n");
> + }
> +
> if (ns_id > 0)
> nsi = nsinfo__new(ns_id);
>
> @@ -366,6 +398,11 @@ int cmd_buildid_cache(int argc, const char **argv)
>
> setup_pager();
>
> + if (list_files) {
> + ret = build_id_cache__show_all();
> + goto out;
> + }
> +
> if (add_name_list_str) {
> list = strlist__new(add_name_list_str, NULL);
> if (list) {
> --
> 2.14.3
>


--
Masami Hiramatsu <[email protected]>

2018-04-23 07:03:50

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] perf/buildid-cache: Support --purge-all option

On Tue, 17 Apr 2018 09:43:46 +0530
Ravi Bangoria <[email protected]> wrote:

> User can remove files from cache using --remove/--purge options
> but both needs list of files as an argument. It's not convenient
> when you want to flush out entire cache. Add an option to purge
> all files from cache.
>
> Ex,
> # perf buildid-cache -l
> 8a86ef73e44067bca52cc3f6cd3e5446c783391c /tmp/a.out
> ebe71fdcf4b366518cc154d570a33cd461a51c36 /tmp/a.out.1
> # perf buildid-cache -P -v
> Removing /tmp/a.out (8a86ef73e44067bca52cc3f6cd3e5446c783391c): Ok
> Removing /tmp/a.out.1 (ebe71fdcf4b366518cc154d570a33cd461a51c36): Ok
> Purged all: Ok
>
> Signed-off-by: Ravi Bangoria <[email protected]>
> ---
> tools/perf/Documentation/perf-buildid-cache.txt | 3 +++
> tools/perf/builtin-buildid-cache.c | 36 ++++++++++++++++++++++++-
> 2 files changed, 38 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/Documentation/perf-buildid-cache.txt b/tools/perf/Documentation/perf-buildid-cache.txt
> index 3f285ba6e1f9..f6de0952ff3c 100644
> --- a/tools/perf/Documentation/perf-buildid-cache.txt
> +++ b/tools/perf/Documentation/perf-buildid-cache.txt
> @@ -48,6 +48,9 @@ OPTIONS
> --purge=::
> Purge all cached binaries including older caches which have specified
> path from the cache.
> +-P::
> +--purge-all::
> + Purge all cached binaries. This will flush out entire 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 fd0a08661b42..d11226f76fbc 100644
> --- a/tools/perf/builtin-buildid-cache.c
> +++ b/tools/perf/builtin-buildid-cache.c
> @@ -240,6 +240,34 @@ static int build_id_cache__purge_path(const char *pathname, struct nsinfo *nsi)
> return err;
> }
>
> +static int build_id_cache__purge_all(void)
> +{
> + struct strlist *list;
> + struct str_node *pos;
> + int err;
> + char *buf;
> +
> + list = build_id_cache__list_all(false);
> + if (!list) {
> + pr_debug("Failed to get buildids: -%d\n", errno);
> + return -EINVAL;
> + }
> +
> + strlist__for_each_entry(pos, list) {
> + buf = build_id_cache__origname(pos->s);
> + err = build_id_cache__remove_s(pos->s);
> + pr_debug("Removing %s (%s): %s\n", buf, pos->s,
> + err ? "FAIL" : "Ok");
> + free(buf);
> + if (err)
> + break;
> + }
> + strlist__delete(list);
> +
> + pr_debug("Purged all: %s\n", err ? "FAIL" : "Ok");

Hmm, I think this should use pr_info(or pr_warning), at least for Failure case.

Thanks,

> + return err;
> +}
> +
> static bool dso__missing_buildid_cache(struct dso *dso, int parm __maybe_unused)
> {
> char filename[PATH_MAX];
> @@ -327,6 +355,7 @@ int cmd_buildid_cache(int argc, const char **argv)
> bool force = false;
> bool list_files = false;
> bool opts_flag = false;
> + bool purge_all = false;
> char const *add_name_list_str = NULL,
> *remove_name_list_str = NULL,
> *purge_name_list_str = NULL,
> @@ -350,6 +379,7 @@ int cmd_buildid_cache(int argc, const char **argv)
> "file(s) to remove"),
> OPT_STRING('p', "purge", &purge_name_list_str, "file list",
> "file(s) to remove (remove old caches too)"),
> + OPT_BOOLEAN('P', "purge-all", &purge_all, "purge all cached files"),
> OPT_BOOLEAN('l', "list", &list_files, "list all cached files"),
> OPT_STRING('M', "missing", &missing_filename, "file",
> "to find missing build ids in the cache"),
> @@ -370,7 +400,8 @@ int cmd_buildid_cache(int argc, const char **argv)
>
> opts_flag = add_name_list_str || kcore_filename ||
> remove_name_list_str || purge_name_list_str ||
> - missing_filename || update_name_list_str;
> + missing_filename || update_name_list_str ||
> + purge_all;
>
> if (argc || !(list_files || opts_flag))
> usage_with_options(buildid_cache_usage, buildid_cache_options);
> @@ -457,6 +488,9 @@ int cmd_buildid_cache(int argc, const char **argv)
> }
> }
>
> + if (purge_all)
> + ret = build_id_cache__purge_all();
> +
> if (missing_filename)
> ret = build_id_cache__fprintf_missing(session, stdout);
>
> --
> 2.14.3
>


--
Masami Hiramatsu <[email protected]>

2018-04-23 19:00:49

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] perf/buildid-cache: Support --purge-all option

Em Tue, Apr 17, 2018 at 09:43:46AM +0530, Ravi Bangoria escreveu:
> User can remove files from cache using --remove/--purge options
> but both needs list of files as an argument. It's not convenient
> when you want to flush out entire cache. Add an option to purge
> all files from cache.
>
> Ex,
> # perf buildid-cache -l
> 8a86ef73e44067bca52cc3f6cd3e5446c783391c /tmp/a.out
> ebe71fdcf4b366518cc154d570a33cd461a51c36 /tmp/a.out.1
> # perf buildid-cache -P -v
> Removing /tmp/a.out (8a86ef73e44067bca52cc3f6cd3e5446c783391c): Ok
> Removing /tmp/a.out.1 (ebe71fdcf4b366518cc154d570a33cd461a51c36): Ok
> Purged all: Ok
>
> Signed-off-by: Ravi Bangoria <[email protected]>
> ---
> tools/perf/Documentation/perf-buildid-cache.txt | 3 +++
> tools/perf/builtin-buildid-cache.c | 36 ++++++++++++++++++++++++-
> 2 files changed, 38 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/Documentation/perf-buildid-cache.txt b/tools/perf/Documentation/perf-buildid-cache.txt
> index 3f285ba6e1f9..f6de0952ff3c 100644
> --- a/tools/perf/Documentation/perf-buildid-cache.txt
> +++ b/tools/perf/Documentation/perf-buildid-cache.txt
> @@ -48,6 +48,9 @@ OPTIONS
> --purge=::
> Purge all cached binaries including older caches which have specified
> path from the cache.
> +-P::
> +--purge-all::
> + Purge all cached binaries. This will flush out entire 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 fd0a08661b42..d11226f76fbc 100644
> --- a/tools/perf/builtin-buildid-cache.c
> +++ b/tools/perf/builtin-buildid-cache.c
> @@ -240,6 +240,34 @@ static int build_id_cache__purge_path(const char *pathname, struct nsinfo *nsi)
> return err;
> }
>
> +static int build_id_cache__purge_all(void)
> +{
> + struct strlist *list;
> + struct str_node *pos;
> + int err;
> + char *buf;
> +
> + list = build_id_cache__list_all(false);
> + if (!list) {
> + pr_debug("Failed to get buildids: -%d\n", errno);
> + return -EINVAL;
> + }
> +
> + strlist__for_each_entry(pos, list) {
> + buf = build_id_cache__origname(pos->s);
> + err = build_id_cache__remove_s(pos->s);
> + pr_debug("Removing %s (%s): %s\n", buf, pos->s,
> + err ? "FAIL" : "Ok");
> + free(buf);
> + if (err)
> + break;
> + }
> + strlist__delete(list);
> +
> + pr_debug("Purged all: %s\n", err ? "FAIL" : "Ok");
> + return err;
> +}

err may be used uninitialized, and debian:7 complains with:

CC /tmp/build/perf/util/ctype.o
builtin-buildid-cache.c: In function 'cmd_buildid_cache':
builtin-buildid-cache.c:267:2: error: 'err' may be used uninitialized in this function [-Werror=uninitialized]
builtin-buildid-cache.c:247:6: note: 'err' was declared here
MKDIR /tmp/build/perf/util/
cc1: all warnings being treated as errors
CC /tmp/build/perf/util/db-export.o
mv: cannot stat `/tmp/build/perf/.builtin-buildid-cache.o.tmp': No such file or directory
make[3]: *** [/tmp/build/perf/builtin-buildid-cache.o] Error 1
make[3]: *** Waiting for unfinished jobs....


I'm initializing it to zero.

- Arnaldo

Subject: [tip:perf/core] perf buildid-cache: Support --list option

Commit-ID: 8e1e0d74672f84e6c4b1e26ce2bb2e529aaf8676
Gitweb: https://git.kernel.org/tip/8e1e0d74672f84e6c4b1e26ce2bb2e529aaf8676
Author: Ravi Bangoria <[email protected]>
AuthorDate: Tue, 17 Apr 2018 09:43:45 +0530
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Thu, 26 Apr 2018 09:30:26 -0300

perf buildid-cache: Support --list option

'perf buildid-cache' allows to add/remove files into cache but there is
no option to list all cached files. Add --list option to list all
_valid_ cached files.

Ex,
# perf buildid-cache --add /tmp/a.out
# perf buildid-cache -l
8a86ef73e44067bca52cc3f6cd3e5446c783391c /tmp/a.out

Signed-off-by: Ravi Bangoria <[email protected]>
Acked-by: Jiri Olsa <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Kate Stewart <[email protected]>
Cc: Krister Johansen <[email protected]>
Cc: Masami Hiramatsu <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Philippe Ombredanne <[email protected]>
Cc: Sihyeon Jang <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/Documentation/perf-buildid-cache.txt | 4 ++-
tools/perf/builtin-buildid-cache.c | 43 +++++++++++++++++++++++--
2 files changed, 43 insertions(+), 4 deletions(-)

diff --git a/tools/perf/Documentation/perf-buildid-cache.txt b/tools/perf/Documentation/perf-buildid-cache.txt
index 73c2650bd0db..3f285ba6e1f9 100644
--- a/tools/perf/Documentation/perf-buildid-cache.txt
+++ b/tools/perf/Documentation/perf-buildid-cache.txt
@@ -59,7 +59,9 @@ OPTIONS
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.
-
+-l::
+--list::
+ List all valid binaries from cache.
-v::
--verbose::
Be more verbose.
diff --git a/tools/perf/builtin-buildid-cache.c b/tools/perf/builtin-buildid-cache.c
index 41db2cba77eb..fd0a08661b42 100644
--- a/tools/perf/builtin-buildid-cache.c
+++ b/tools/perf/builtin-buildid-cache.c
@@ -25,6 +25,7 @@
#include "util/session.h"
#include "util/symbol.h"
#include "util/time-utils.h"
+#include "util/probe-file.h"

static int build_id_cache__kcore_buildid(const char *proc_dir, char *sbuildid)
{
@@ -297,6 +298,26 @@ static int build_id_cache__update_file(const char *filename, struct nsinfo *nsi)
return err;
}

+static int build_id_cache__show_all(void)
+{
+ struct strlist *bidlist;
+ struct str_node *nd;
+ char *buf;
+
+ bidlist = build_id_cache__list_all(true);
+ if (!bidlist) {
+ pr_debug("Failed to get buildids: -%d\n", errno);
+ return -1;
+ }
+ strlist__for_each_entry(nd, bidlist) {
+ buf = build_id_cache__origname(nd->s);
+ fprintf(stdout, "%s %s\n", nd->s, buf);
+ free(buf);
+ }
+ strlist__delete(bidlist);
+ return 0;
+}
+
int cmd_buildid_cache(int argc, const char **argv)
{
struct strlist *list;
@@ -304,6 +325,8 @@ int cmd_buildid_cache(int argc, const char **argv)
int ret = 0;
int ns_id = -1;
bool force = false;
+ bool list_files = false;
+ bool opts_flag = false;
char const *add_name_list_str = NULL,
*remove_name_list_str = NULL,
*purge_name_list_str = NULL,
@@ -327,6 +350,7 @@ int cmd_buildid_cache(int argc, const char **argv)
"file(s) to remove"),
OPT_STRING('p', "purge", &purge_name_list_str, "file list",
"file(s) to remove (remove old caches too)"),
+ OPT_BOOLEAN('l', "list", &list_files, "list all cached files"),
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"),
@@ -344,11 +368,19 @@ 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))
+ opts_flag = add_name_list_str || kcore_filename ||
+ remove_name_list_str || purge_name_list_str ||
+ missing_filename || update_name_list_str;
+
+ if (argc || !(list_files || opts_flag))
usage_with_options(buildid_cache_usage, buildid_cache_options);

+ /* -l is exclusive. It can not be used with other options. */
+ if (list_files && opts_flag) {
+ usage_with_options_msg(buildid_cache_usage,
+ buildid_cache_options, "-l is exclusive.\n");
+ }
+
if (ns_id > 0)
nsi = nsinfo__new(ns_id);

@@ -366,6 +398,11 @@ int cmd_buildid_cache(int argc, const char **argv)

setup_pager();

+ if (list_files) {
+ ret = build_id_cache__show_all();
+ goto out;
+ }
+
if (add_name_list_str) {
list = strlist__new(add_name_list_str, NULL);
if (list) {

Subject: [tip:perf/core] perf buildid-cache: Support --purge-all option

Commit-ID: 9a73c308542bea74df921e9bb9fb07d39bbf2cfa
Gitweb: https://git.kernel.org/tip/9a73c308542bea74df921e9bb9fb07d39bbf2cfa
Author: Ravi Bangoria <[email protected]>
AuthorDate: Tue, 17 Apr 2018 09:43:46 +0530
Committer: Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Thu, 26 Apr 2018 09:30:26 -0300

perf buildid-cache: Support --purge-all option

User can remove files from cache using --remove/--purge options but both
needs list of files as an argument. It's not convenient when you want to
flush out entire cache. Add an option to purge all files from cache.

Ex,
# perf buildid-cache -l
8a86ef73e44067bca52cc3f6cd3e5446c783391c /tmp/a.out
ebe71fdcf4b366518cc154d570a33cd461a51c36 /tmp/a.out.1
# perf buildid-cache -P -v
Removing /tmp/a.out (8a86ef73e44067bca52cc3f6cd3e5446c783391c): Ok
Removing /tmp/a.out.1 (ebe71fdcf4b366518cc154d570a33cd461a51c36): Ok
Purged all: Ok

Signed-off-by: Ravi Bangoria <[email protected]>
Acked-by: Jiri Olsa <[email protected]>
Cc: Alexander Shishkin <[email protected]>
Cc: Kate Stewart <[email protected]>
Cc: Krister Johansen <[email protected]>
Cc: Masami Hiramatsu <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Philippe Ombredanne <[email protected]>
Cc: Sihyeon Jang <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
[ Initialize 'err' in build_id_cache__purge_all(), to fix build on debian:7, as it can be used uninitialized ]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/Documentation/perf-buildid-cache.txt | 3 +++
tools/perf/builtin-buildid-cache.c | 36 ++++++++++++++++++++++++-
2 files changed, 38 insertions(+), 1 deletion(-)

diff --git a/tools/perf/Documentation/perf-buildid-cache.txt b/tools/perf/Documentation/perf-buildid-cache.txt
index 3f285ba6e1f9..f6de0952ff3c 100644
--- a/tools/perf/Documentation/perf-buildid-cache.txt
+++ b/tools/perf/Documentation/perf-buildid-cache.txt
@@ -48,6 +48,9 @@ OPTIONS
--purge=::
Purge all cached binaries including older caches which have specified
path from the cache.
+-P::
+--purge-all::
+ Purge all cached binaries. This will flush out entire 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 fd0a08661b42..7a7403913b57 100644
--- a/tools/perf/builtin-buildid-cache.c
+++ b/tools/perf/builtin-buildid-cache.c
@@ -240,6 +240,34 @@ out:
return err;
}

+static int build_id_cache__purge_all(void)
+{
+ struct strlist *list;
+ struct str_node *pos;
+ int err = 0;
+ char *buf;
+
+ list = build_id_cache__list_all(false);
+ if (!list) {
+ pr_debug("Failed to get buildids: -%d\n", errno);
+ return -EINVAL;
+ }
+
+ strlist__for_each_entry(pos, list) {
+ buf = build_id_cache__origname(pos->s);
+ err = build_id_cache__remove_s(pos->s);
+ pr_debug("Removing %s (%s): %s\n", buf, pos->s,
+ err ? "FAIL" : "Ok");
+ free(buf);
+ if (err)
+ break;
+ }
+ strlist__delete(list);
+
+ pr_debug("Purged all: %s\n", err ? "FAIL" : "Ok");
+ return err;
+}
+
static bool dso__missing_buildid_cache(struct dso *dso, int parm __maybe_unused)
{
char filename[PATH_MAX];
@@ -327,6 +355,7 @@ int cmd_buildid_cache(int argc, const char **argv)
bool force = false;
bool list_files = false;
bool opts_flag = false;
+ bool purge_all = false;
char const *add_name_list_str = NULL,
*remove_name_list_str = NULL,
*purge_name_list_str = NULL,
@@ -350,6 +379,7 @@ int cmd_buildid_cache(int argc, const char **argv)
"file(s) to remove"),
OPT_STRING('p', "purge", &purge_name_list_str, "file list",
"file(s) to remove (remove old caches too)"),
+ OPT_BOOLEAN('P', "purge-all", &purge_all, "purge all cached files"),
OPT_BOOLEAN('l', "list", &list_files, "list all cached files"),
OPT_STRING('M', "missing", &missing_filename, "file",
"to find missing build ids in the cache"),
@@ -370,7 +400,8 @@ int cmd_buildid_cache(int argc, const char **argv)

opts_flag = add_name_list_str || kcore_filename ||
remove_name_list_str || purge_name_list_str ||
- missing_filename || update_name_list_str;
+ missing_filename || update_name_list_str ||
+ purge_all;

if (argc || !(list_files || opts_flag))
usage_with_options(buildid_cache_usage, buildid_cache_options);
@@ -457,6 +488,9 @@ int cmd_buildid_cache(int argc, const char **argv)
}
}

+ if (purge_all)
+ ret = build_id_cache__purge_all();
+
if (missing_filename)
ret = build_id_cache__fprintf_missing(session, stdout);


2018-05-03 08:54:50

by Ravi Bangoria

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] perf/buildid-cache: Support --purge-all option

Hi Masami,

On 04/23/2018 12:32 PM, Masami Hiramatsu wrote:
> On Tue, 17 Apr 2018 09:43:46 +0530
> Ravi Bangoria <[email protected]> wrote:
>
>> + pr_debug("Purged all: %s\n", err ? "FAIL" : "Ok");
> Hmm, I think this should use pr_info(or pr_warning), at least for Failure case.

Actually, I followed what is already there for other options.

Thanks,
Ravi


2018-05-08 15:44:41

by Masami Hiramatsu

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] perf/buildid-cache: Support --purge-all option

On Thu, 3 May 2018 14:24:04 +0530
Ravi Bangoria <[email protected]> wrote:

> Hi Masami,
>
> On 04/23/2018 12:32 PM, Masami Hiramatsu wrote:
> > On Tue, 17 Apr 2018 09:43:46 +0530
> > Ravi Bangoria <[email protected]> wrote:
> >
> >> + pr_debug("Purged all: %s\n", err ? "FAIL" : "Ok");
> > Hmm, I think this should use pr_info(or pr_warning), at least for Failure case.
>
> Actually, I followed what is already there for other options.

Please see cmd_buildid_cache(), if we failed to purge one buildid-cache,
it is warned with pr_warning.

if (purge_name_list_str) {
list = strlist__new(purge_name_list_str, NULL);
if (list) {
strlist__for_each_entry(pos, list)
if (build_id_cache__purge_path(pos->s, nsi)) {
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, str_error_r(errno, sbuf, sizeof(sbuf)));
}

strlist__delete(list);
}
}

Thank you,

--
Masami Hiramatsu <[email protected]>

2018-05-09 06:01:40

by Ravi Bangoria

[permalink] [raw]
Subject: Re: [PATCH v2 3/3] perf/buildid-cache: Support --purge-all option



On 05/08/2018 09:12 PM, Masami Hiramatsu wrote:
> On Thu, 3 May 2018 14:24:04 +0530
> Ravi Bangoria <[email protected]> wrote:
>
>> Hi Masami,
>>
>> On 04/23/2018 12:32 PM, Masami Hiramatsu wrote:
>>> On Tue, 17 Apr 2018 09:43:46 +0530
>>> Ravi Bangoria <[email protected]> wrote:
>>>
>>>> + pr_debug("Purged all: %s\n", err ? "FAIL" : "Ok");
>>> Hmm, I think this should use pr_info(or pr_warning), at least for Failure case.
>> Actually, I followed what is already there for other options.
> Please see cmd_buildid_cache(), if we failed to purge one buildid-cache,
> it is warned with pr_warning.

Oh ok. I thought you are asking me to change pr_debug().
I'll quickly prepare and send a patch.

Thanks,
Ravi

>
> if (purge_name_list_str) {
> list = strlist__new(purge_name_list_str, NULL);
> if (list) {
> strlist__for_each_entry(pos, list)
> if (build_id_cache__purge_path(pos->s, nsi)) {
> 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, str_error_r(errno, sbuf, sizeof(sbuf)));
> }
>
> strlist__delete(list);
> }
> }
>
> Thank you,
>