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

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

[Ravi Bangoria]

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

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

[Ravi Bangoria]

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 <ravi.bangoria@linux.vnet.ibm.com>
---
 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

[PATCH 2/3] perf/buildid-cache: Support --list option

[Ravi Bangoria]

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 <ravi.bangoria@linux.vnet.ibm.com>
---
 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

[PATCH 3/3] perf/buildid-cache: Support --purge-all option

[Ravi Bangoria]

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 <ravi.bangoria@linux.vnet.ibm.com>
---
 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

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

[Jiri Olsa]

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 <ravi.bangoria@linux.vnet.ibm.com>
> ---
>  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
> 

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

[Jiri Olsa]

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

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

[Jiri Olsa]

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

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

[Ravi Bangoria]

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
>

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

[Masami Hiramatsu]

On Mon,  9 Apr 2018 16:36:31 +0530
Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> 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 <mhiramat@kernel.org>

Thanks!

> 
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
> ---
>  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 <mhiramat@kernel.org>

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

[Masami Hiramatsu]

On Mon,  9 Apr 2018 16:36:33 +0530
Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> 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 <ravi.bangoria@linux.vnet.ibm.com>
> ---
>  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 <mhiramat@kernel.org>

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

[Ravi Bangoria]

Hi Masami,

On 04/16/2018 02:57 PM, Masami Hiramatsu wrote:
> On Mon,  9 Apr 2018 16:36:33 +0530
> Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> 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

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

[Jiri Olsa]

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 <ravi.bangoria@linux.vnet.ibm.com> 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

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

[Masami Hiramatsu]

On Mon, 16 Apr 2018 12:30:17 +0200
Jiri Olsa <jolsa@redhat.com> 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 <ravi.bangoria@linux.vnet.ibm.com> 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 <mhiramat@kernel.org>