2018-04-09 11:10:30

by Ravi Bangoria

[permalink] [raw]
Subject: [PATCH 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.

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 | 75 ++++++++++++++++++++++++-
3 files changed, 81 insertions(+), 7 deletions(-)

--
2.14.3



2018-04-09 11:10:39

by Ravi Bangoria

[permalink] [raw]
Subject: [PATCH 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]>
---
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-09 11:10:42

by Ravi Bangoria

[permalink] [raw]
Subject: [PATCH 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
/tmp/a.out (8a86ef73e44067bca52cc3f6cd3e5446c783391c)

Signed-off-by: Ravi Bangoria <[email protected]>
---
tools/perf/Documentation/perf-buildid-cache.txt | 4 ++-
tools/perf/builtin-buildid-cache.c | 41 +++++++++++++++++++++++--
2 files changed, 41 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..50db05bd0cc6 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,25 @@ static int build_id_cache__update_file(const char *filename, struct nsinfo *nsi)
return err;
}

+static void 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;
+ }
+ strlist__for_each_entry(nd, bidlist) {
+ buf = build_id_cache__origname(nd->s);
+ printf("%s (%s)\n", buf, nd->s);
+ free(buf);
+ }
+ strlist__delete(bidlist);
+}
+
int cmd_buildid_cache(int argc, const char **argv)
{
struct strlist *list;
@@ -304,6 +324,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 +349,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 +367,18 @@ 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 +396,11 @@ int cmd_buildid_cache(int argc, const char **argv)

setup_pager();

+ if (list_files) {
+ 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-09 11:11:07

by Ravi Bangoria

[permalink] [raw]
Subject: [PATCH 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
/tmp/a.out (8a86ef73e44067bca52cc3f6cd3e5446c783391c)
/tmp/a.out.1 (ebe71fdcf4b366518cc154d570a33cd461a51c36)
# 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 50db05bd0cc6..3b4373cabd49 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];
@@ -326,6 +354,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,
@@ -349,6 +378,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"),
@@ -369,7 +399,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);
@@ -455,6 +486,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-13 12:56:46

by Jiri Olsa

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

On Mon, Apr 09, 2018 at 04:36:32PM +0530, Ravi Bangoria 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
> /tmp/a.out (8a86ef73e44067bca52cc3f6cd3e5446c783391c)

could you please make them displayed the same way as in perf buildid-list

[jolsa@krava perf]$ ./perf buildid-list
8b581c1243ade7d7b0f535add0b89405c89dd170 [kernel.kallsyms]
050d0ac0b68aebdcace1290ed4007442ecaef562 /usr/lib64/libc-2.25.so
ac4597b8b29cc9b89a3afbac37332a5d954ee6a4 /usr/lib64/libpthread-2.25.so

thanks,
jirka

>
> Signed-off-by: Ravi Bangoria <[email protected]>
> ---
> tools/perf/Documentation/perf-buildid-cache.txt | 4 ++-
> tools/perf/builtin-buildid-cache.c | 41 +++++++++++++++++++++++--
> 2 files changed, 41 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..50db05bd0cc6 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,25 @@ static int build_id_cache__update_file(const char *filename, struct nsinfo *nsi)
> return err;
> }
>
> +static void 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;
> + }
> + strlist__for_each_entry(nd, bidlist) {
> + buf = build_id_cache__origname(nd->s);
> + printf("%s (%s)\n", buf, nd->s);
> + free(buf);
> + }
> + strlist__delete(bidlist);
> +}
> +
> int cmd_buildid_cache(int argc, const char **argv)
> {
> struct strlist *list;
> @@ -304,6 +324,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 +349,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 +367,18 @@ 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 +396,11 @@ int cmd_buildid_cache(int argc, const char **argv)
>
> setup_pager();
>
> + if (list_files) {
> + 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-13 12:57:25

by Jiri Olsa

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

On Mon, Apr 09, 2018 at 04:36:32PM +0530, Ravi Bangoria wrote:

SNIP

> int cmd_buildid_cache(int argc, const char **argv)
> {
> struct strlist *list;
> @@ -304,6 +324,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 +349,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 +367,18 @@ 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");

missing {} on multiline condition leg

jirka

2018-04-13 13:45:07

by Jiri Olsa

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

On Mon, Apr 09, 2018 at 04:36:32PM +0530, Ravi Bangoria wrote:

SNIP

> - !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 +396,11 @@ int cmd_buildid_cache(int argc, const char **argv)
>
> setup_pager();
>
> + if (list_files) {
> + build_id_cache__show_all();
> + goto out;

make build_id_cache__show_all return value and store it to ret before going out

jirka

2018-04-16 07:28:54

by Ravi Bangoria

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

Thanks Jiri for the reviews. Will post v2 soon.

-Ravi

On 04/13/2018 06:28 PM, Jiri Olsa wrote:
> On Mon, Apr 09, 2018 at 04:36:32PM +0530, Ravi Bangoria wrote:
>
> SNIP
>
>> - !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 +396,11 @@ int cmd_buildid_cache(int argc, const char **argv)
>>
>> setup_pager();
>>
>> + if (list_files) {
>> + build_id_cache__show_all();
>> + goto out;
> make build_id_cache__show_all return value and store it to ret before going out
>
> jirka
>


2018-04-16 10:31:55

by Jiri Olsa

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

On Mon, Apr 16, 2018 at 03:10:40PM +0530, Ravi Bangoria wrote:
> Hi Masami,
>
> On 04/16/2018 02:57 PM, Masami Hiramatsu wrote:
> > On Mon, 9 Apr 2018 16:36:33 +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
> >> /tmp/a.out (8a86ef73e44067bca52cc3f6cd3e5446c783391c)
> >> /tmp/a.out.1 (ebe71fdcf4b366518cc154d570a33cd461a51c36)
> >> # perf buildid-cache -P -v
> >> Removing /tmp/a.out (8a86ef73e44067bca52cc3f6cd3e5446c783391c): Ok
> >> Removing /tmp/a.out.1 (ebe71fdcf4b366518cc154d570a33cd461a51c36): Ok
> >> Purged all: Ok
> > Hmm, for purging all caches will be done by
> >
> > $ rm -rf ~/.debug
> >
> > Are there any difference?
>
> No logical difference if you know it's ~/.debug where it goes. :)
> I also used to do rm -rf earlier.
>
> This option is for a perf users. But I'm fine if it's not really needed.
> Will drop it.

I'd keep it.. as you said it could be configured at some other dir

jirka

2018-04-16 11:07:48

by Masami Hiramatsu

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

On Mon, 9 Apr 2018 16:36:31 +0530
Ravi Bangoria <[email protected]> wrote:

> 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.

Looks good to me.

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

Thanks!

>
> Signed-off-by: Ravi Bangoria <[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
>


--
Masami Hiramatsu <[email protected]>

2018-04-16 11:09:42

by Masami Hiramatsu

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

On Mon, 9 Apr 2018 16:36:33 +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
> /tmp/a.out (8a86ef73e44067bca52cc3f6cd3e5446c783391c)
> /tmp/a.out.1 (ebe71fdcf4b366518cc154d570a33cd461a51c36)
> # perf buildid-cache -P -v
> Removing /tmp/a.out (8a86ef73e44067bca52cc3f6cd3e5446c783391c): Ok
> Removing /tmp/a.out.1 (ebe71fdcf4b366518cc154d570a33cd461a51c36): Ok
> Purged all: Ok

Hmm, for purging all caches will be done by

$ rm -rf ~/.debug

Are there any difference?

Thanks,

>
> 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 50db05bd0cc6..3b4373cabd49 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];
> @@ -326,6 +354,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,
> @@ -349,6 +378,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"),
> @@ -369,7 +399,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);
> @@ -455,6 +486,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-16 11:13:21

by Ravi Bangoria

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

Hi Masami,

On 04/16/2018 02:57 PM, Masami Hiramatsu wrote:
> On Mon, 9 Apr 2018 16:36:33 +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
>> /tmp/a.out (8a86ef73e44067bca52cc3f6cd3e5446c783391c)
>> /tmp/a.out.1 (ebe71fdcf4b366518cc154d570a33cd461a51c36)
>> # perf buildid-cache -P -v
>> Removing /tmp/a.out (8a86ef73e44067bca52cc3f6cd3e5446c783391c): Ok
>> Removing /tmp/a.out.1 (ebe71fdcf4b366518cc154d570a33cd461a51c36): Ok
>> Purged all: Ok
> Hmm, for purging all caches will be done by
>
> $ rm -rf ~/.debug
>
> Are there any difference?

No logical difference if you know it's ~/.debug where it goes. :)
I also used to do rm -rf earlier.

This option is for a perf users. But I'm fine if it's not really needed.
Will drop it.

Thanks,
Ravi


2018-04-18 05:54:48

by Masami Hiramatsu

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

On Mon, 16 Apr 2018 12:30:17 +0200
Jiri Olsa <[email protected]> wrote:

> On Mon, Apr 16, 2018 at 03:10:40PM +0530, Ravi Bangoria wrote:
> > Hi Masami,
> >
> > On 04/16/2018 02:57 PM, Masami Hiramatsu wrote:
> > > On Mon, 9 Apr 2018 16:36:33 +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
> > >> /tmp/a.out (8a86ef73e44067bca52cc3f6cd3e5446c783391c)
> > >> /tmp/a.out.1 (ebe71fdcf4b366518cc154d570a33cd461a51c36)
> > >> # perf buildid-cache -P -v
> > >> Removing /tmp/a.out (8a86ef73e44067bca52cc3f6cd3e5446c783391c): Ok
> > >> Removing /tmp/a.out.1 (ebe71fdcf4b366518cc154d570a33cd461a51c36): Ok
> > >> Purged all: Ok
> > > Hmm, for purging all caches will be done by
> > >
> > > $ rm -rf ~/.debug
> > >
> > > Are there any difference?
> >
> > No logical difference if you know it's ~/.debug where it goes. :)
> > I also used to do rm -rf earlier.
> >
> > This option is for a perf users. But I'm fine if it's not really needed.
> > Will drop it.
>
> I'd keep it.. as you said it could be configured at some other dir

Sounds reasonable. :)

Thanks,

>
> jirka


--
Masami Hiramatsu <[email protected]>