[PATCH v2 0/7] perf config: Add support for config read/write

[PATCH v2 0/7] perf config: Add support for config read/write

[Taeung Song]

Hello, :)

Add simple config read/write functionalities.

I had worked at the related patchset https://lkml.org/lkml/2016/2/22/38 
But I remake new this patchset for only support for read/write config file.
And There're Namhyung's requests https://lkml.org/lkml/2016/10/24/572

In particular, I agonized implement way for write functionality.
I especially wonder other opinions about new boolean variable
'from_system_config' I use.

If someone review this patchset and give me some feedback,
I'd appreciated it. :)

Thanks,
Taeung

v2:
- a bit change commit messages
- refactor parts of parse_config_arg

Taeung Song (7):
  perf config: Add support for getting functionality
  perf config: Document examples getting config info in man page
  perf config: Parse arguments before getting functionality
  perf config: Parse arguments before setting functionality
  perf config: Add support for write functionality
  perf config: Document config setting examples in man page
  perf config: Mark where are config items from (user or system)

 tools/perf/Documentation/perf-config.txt |  35 +++++++
 tools/perf/builtin-config.c              | 155 ++++++++++++++++++++++++++++++-
 tools/perf/util/config.c                 |  20 ++++
 tools/perf/util/config.h                 |   4 +
 4 files changed, 211 insertions(+), 3 deletions(-)

-- 
2.7.4

[PATCH v2 1/7] perf config: Add support for getting functionality

[Taeung Song]

Add a functionality getting particular config key-value pairs.
For the syntax examples,

    perf config [<file-option>] [section.name ...]

e.g. To query config items 'report.queue-size' and 'report.children', do

    # perf config report.queue-size report.children

Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Wang Nan <wangnan0@huawei.com>
Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
---
 tools/perf/builtin-config.c | 42 +++++++++++++++++++++++++++++++++++++++---
 1 file changed, 39 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c
index e4207a2..7b24789 100644
--- a/tools/perf/builtin-config.c
+++ b/tools/perf/builtin-config.c
@@ -17,7 +17,7 @@
 static bool use_system_config, use_user_config;
 
 static const char * const config_usage[] = {
-	"perf config [<file-option>] [options]",
+	"perf config [<file-option>] [options] [section.name ...]",
 	NULL
 };
 
@@ -33,6 +33,36 @@ static struct option config_options[] = {
 	OPT_END()
 };
 
+static int show_spec_config(struct perf_config_set *set, const char *var)
+{
+	struct perf_config_section *section;
+	struct perf_config_item *item;
+
+	if (set == NULL || var == NULL)
+		return -1;
+
+	perf_config_items__for_each_entry(&set->sections, section) {
+		if (prefixcmp(var, section->name) != 0)
+			continue;
+
+		perf_config_items__for_each_entry(&section->items, item) {
+			const char *name = var + strlen(section->name) + 1;
+
+			if (strcmp(name, item->name) == 0) {
+				char *value = item->value;
+
+				if (value) {
+					printf("%s=%s\n", var, value);
+					return 0;
+				}
+			}
+
+		}
+	}
+
+	return 0;
+}
+
 static int show_config(struct perf_config_set *set)
 {
 	struct perf_config_section *section;
@@ -54,7 +84,7 @@ static int show_config(struct perf_config_set *set)
 
 int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)
 {
-	int ret = 0;
+	int i, ret = 0;
 	struct perf_config_set *set;
 	char *user_config = mkpath("%s/.perfconfig", getenv("HOME"));
 
@@ -100,7 +130,13 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)
 		}
 		break;
 	default:
-		usage_with_options(config_usage, config_options);
+		if (argc == 0) {
+			usage_with_options(config_usage, config_options);
+			break;
+		}
+
+		for (i = 0; argv[i]; i++)
+			ret = show_spec_config(set, argv[i]);
 	}
 
 	perf_config_set__delete(set);
-- 
2.7.4

[PATCH v2 2/7] perf config: Document examples getting config info in man page

[Taeung Song]

Explain how to query particular config items in config file
and how to get several config items from user or system config file
using '--user' or '--system' options.

Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Wang Nan <wangnan0@huawei.com>
Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
---
 tools/perf/Documentation/perf-config.txt | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/tools/perf/Documentation/perf-config.txt b/tools/perf/Documentation/perf-config.txt
index cb081ac5..1714b0c 100644
--- a/tools/perf/Documentation/perf-config.txt
+++ b/tools/perf/Documentation/perf-config.txt
@@ -8,6 +8,8 @@ perf-config - Get and set variables in a configuration file.
 SYNOPSIS
 --------
 [verse]
+'perf config' [<file-option>] [section.name ...]
+or
 'perf config' [<file-option>] -l | --list
 
 DESCRIPTION
@@ -118,6 +120,22 @@ Given a $HOME/.perfconfig like this:
 		children = true
 		group = true
 
+To query the record mode of call graph, do
+
+	% perf config call-graph.record-mode
+
+If you want to know multiple config key/value pairs, you can do like
+
+	% perf config report.queue-size call-graph.order report.children
+
+To query the config value of sort order of call graph in user config file (i.e. `~/.perfconfig`), do
+
+	% perf config --user call-graph.sort-order
+
+To query the config value of buildid directory in system config file (i.e. `$(sysconf)/perfconfig`), do
+
+	% perf config --system buildid.dir
+
 Variables
 ~~~~~~~~~
 
-- 
2.7.4

[PATCH v2 3/7] perf config: Parse arguments before getting functionality

[Taeung Song]

You can get several config items as below,

    # perf config report.queue-size call-graph.record-mode

but it would be needed to more precisely check arguments,
before show_spec_config() takes over the arguments.
The function would be also used for config write functionality
in the near future.

Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Wang Nan <wangnan0@huawei.com>
Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
---
 tools/perf/builtin-config.c | 42 ++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 40 insertions(+), 2 deletions(-)

diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c
index 7b24789..02def73 100644
--- a/tools/perf/builtin-config.c
+++ b/tools/perf/builtin-config.c
@@ -82,6 +82,38 @@ static int show_config(struct perf_config_set *set)
 	return 0;
 }
 
+static int parse_config_arg(const char *arg, char **var)
+{
+	const char *last_dot;
+	char *key;
+
+	key = strdup(arg);
+	if (!key) {
+		pr_err("%s: strdup failed\n", __func__);
+		return -1;
+	}
+
+	last_dot = strchr(key, '.');
+	/*
+	 * Since "key" actually contains the section name and the real
+	 * config variable name separated by a dot, we have to know where the dot is.
+	 */
+	if (last_dot == NULL || last_dot == key) {
+		pr_err("The config variable does not contain a section: %s\n", arg);
+		goto out_err;
+	}
+	if (!last_dot[1]) {
+		pr_err("The config varible does not contain variable name: %s\n", arg);
+		goto out_err;
+	}
+
+	*var = key;
+	return 0;
+out_err:
+	free(key);
+	return -1;
+}
+
 int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)
 {
 	int i, ret = 0;
@@ -135,8 +167,14 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)
 			break;
 		}
 
-		for (i = 0; argv[i]; i++)
-			ret = show_spec_config(set, argv[i]);
+		for (i = 0; argv[i]; i++) {
+			char *var = NULL;
+
+			if (parse_config_arg(argv[i], &var) < 0)
+				break;
+			ret = show_spec_config(set, var);
+			free(var);
+		}
 	}
 
 	perf_config_set__delete(set);
-- 
2.7.4

[PATCH v2 4/7] perf config: Parse arguments before setting functionality

[Taeung Song]

For upcoming setting functionality,
parse both config variables and values of arguments.
If setting the sort order of report functionality to srcline,

    # perf config report.sort-order=srcline

we can parse config variables and values with parse_config_arg()

Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Wang Nan <wangnan0@huawei.com>
Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
---
 tools/perf/builtin-config.c | 43 ++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 38 insertions(+), 5 deletions(-)

diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c
index 02def73..098d2df 100644
--- a/tools/perf/builtin-config.c
+++ b/tools/perf/builtin-config.c
@@ -82,10 +82,10 @@ static int show_config(struct perf_config_set *set)
 	return 0;
 }
 
-static int parse_config_arg(const char *arg, char **var)
+static int parse_config_arg(const char *arg, char **var, char **value)
 {
 	const char *last_dot;
-	char *key;
+	char *key, *val;
 
 	key = strdup(arg);
 	if (!key) {
@@ -107,7 +107,37 @@ static int parse_config_arg(const char *arg, char **var)
 		goto out_err;
 	}
 
-	*var = key;
+	val = strchr(key, '=');
+	if (val == NULL) {
+		*var = key;
+		return 0;
+	} else if (!strcmp(val, "=")) {
+		pr_err("The config variable does not contain a value: %s\n", arg);
+		goto out_err;
+	} else {
+		char *v, *ptr;
+
+		*value = strdup(val + 1); /* excluding a first character '=' */
+		if (*value == NULL) {
+			pr_err("%s: strdup failed\n", __func__);
+			goto out_err;
+		}
+
+		ptr = key;
+		v = strsep(&key, "=");
+		if (v[0] == '\0') {
+			pr_err("invalid config variable: %s\n", arg);
+			goto out_err;
+		}
+
+		*var = strdup(v);
+		free(ptr);
+		if (*var == NULL) {
+			pr_err("%s: strdup failed\n", __func__);
+			return -1;
+		}
+	}
+
 	return 0;
 out_err:
 	free(key);
@@ -169,11 +199,14 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)
 
 		for (i = 0; argv[i]; i++) {
 			char *var = NULL;
+			char *value = NULL;
 
-			if (parse_config_arg(argv[i], &var) < 0)
+			if (parse_config_arg(argv[i], &var, &value) < 0)
 				break;
-			ret = show_spec_config(set, var);
+			if (value == NULL)
+				ret = show_spec_config(set, var);
 			free(var);
+			free(value);
 		}
 	}
 
-- 
2.7.4

[PATCH v2 5/7] perf config: Add support for write functionality

[Taeung Song]

Add support for write functionality in config file
(i.e. user or system config file).
This functionality autogerates a config file with
existing config key-value pairs.

For the syntax examples,

    perf config [<file-option>] [section.name[=value] ...]

e.g. You can set the ui.show-headers to false with

    # perf config ui.show-headers=false

If you want to add or modify several config items, you can do like

    # perf config annotate.show_nr_jumps=false kmem.default=slab

Sure, you can also use file-option --user or --system.

Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Wang Nan <wangnan0@huawei.com>
Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
---
 tools/perf/builtin-config.c | 40 +++++++++++++++++++++++++++++++++++++++-
 tools/perf/util/config.c    |  6 ++++++
 tools/perf/util/config.h    |  2 ++
 3 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c
index 098d2df..2e7e295 100644
--- a/tools/perf/builtin-config.c
+++ b/tools/perf/builtin-config.c
@@ -17,7 +17,7 @@
 static bool use_system_config, use_user_config;
 
 static const char * const config_usage[] = {
-	"perf config [<file-option>] [options] [section.name ...]",
+	"perf config [<file-option>] [options] [section.name[=value] ...]",
 	NULL
 };
 
@@ -33,6 +33,37 @@ static struct option config_options[] = {
 	OPT_END()
 };
 
+static int set_config(struct perf_config_set *set, const char *file_name,
+		      const char *var, const char *value)
+{
+	struct perf_config_section *section = NULL;
+	struct perf_config_item *item = NULL;
+	const char *first_line = "# this file is auto-generated.";
+	FILE *fp = fopen(file_name, "w");
+
+	if (!fp)
+		return -1;
+	if (set == NULL)
+		return -1;
+
+	perf_config_set__collect(set, var, value);
+	fprintf(fp, "%s\n", first_line);
+
+	/* overwrite configvariables */
+	perf_config_items__for_each_entry(&set->sections, section) {
+		fprintf(fp, "[%s]\n", section->name);
+
+		perf_config_items__for_each_entry(&section->items, item) {
+			if (item->value)
+				fprintf(fp, "\t%s = %s\n",
+					item->name, item->value);
+		}
+	}
+	fclose(fp);
+
+	return 0;
+}
+
 static int show_spec_config(struct perf_config_set *set, const char *var)
 {
 	struct perf_config_section *section;
@@ -205,6 +236,13 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)
 				break;
 			if (value == NULL)
 				ret = show_spec_config(set, var);
+			else {
+				const char *config_filename = config_exclusive_filename;
+
+				if (!config_exclusive_filename)
+					config_filename = user_config;
+				ret = set_config(set, config_filename, var, value);
+			}
 			free(var);
 			free(value);
 		}
diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
index 18dae74..c8fb65d 100644
--- a/tools/perf/util/config.c
+++ b/tools/perf/util/config.c
@@ -602,6 +602,12 @@ static int collect_config(const char *var, const char *value,
 	return -1;
 }
 
+int perf_config_set__collect(struct perf_config_set *set,
+			     const char *var, const char *value)
+{
+	return collect_config(var, value, set);
+}
+
 static int perf_config_set__init(struct perf_config_set *set)
 {
 	int ret = -1;
diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h
index 6f813d4..0fcdb8c 100644
--- a/tools/perf/util/config.h
+++ b/tools/perf/util/config.h
@@ -33,6 +33,8 @@ const char *perf_etc_perfconfig(void);
 
 struct perf_config_set *perf_config_set__new(void);
 void perf_config_set__delete(struct perf_config_set *set);
+int perf_config_set__collect(struct perf_config_set *set,
+			     const char *var, const char *value);
 void perf_config__init(void);
 void perf_config__exit(void);
 void perf_config__refresh(void);
-- 
2.7.4

[PATCH v2 6/7] perf config: Document config setting examples in man page

[Taeung Song]

Explain how to add or modify particular config items in config file
and how to set several config items from user or system config file
using '--user' or '--system' options.

Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Wang Nan <wangnan0@huawei.com>
Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
---
 tools/perf/Documentation/perf-config.txt | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/tools/perf/Documentation/perf-config.txt b/tools/perf/Documentation/perf-config.txt
index 1714b0c..4b69c34 100644
--- a/tools/perf/Documentation/perf-config.txt
+++ b/tools/perf/Documentation/perf-config.txt
@@ -8,7 +8,7 @@ perf-config - Get and set variables in a configuration file.
 SYNOPSIS
 --------
 [verse]
-'perf config' [<file-option>] [section.name ...]
+'perf config' [<file-option>] [section.name[=value] ...]
 or
 'perf config' [<file-option>] -l | --list
 
@@ -120,6 +120,23 @@ Given a $HOME/.perfconfig like this:
 		children = true
 		group = true
 
+You can hide source code of annotate feature setting the config to false with
+
+	% perf config annotate.hide_src_code=true
+
+If you want to add or modify several config items, you can do like
+
+	% perf config ui.show-headers=false kmem.default=slab
+
+To modify the sort order of report functionality in user config file(i.e. `~/.perfconfig`), do
+
+	% perf config --user report.sort-order=srcline
+
+To change colors of selected line to other foreground and background colors
+in system config file (i.e. `$(sysconf)/perfconfig`), do
+
+	% perf config --system colors.selected=yellow,green
+
 To query the record mode of call graph, do
 
 	% perf config call-graph.record-mode
-- 
2.7.4

[PATCH v2 7/7] perf config: Mark where are config items from (user or system)

[Taeung Song]

To write config items to a particular config file,
we should know where are the config items from.

Config setting functionality autogenerate config file
with existing config items.
So, When collecting config items from user and system config
files (i.e. ~/.perfconfig and $(sysconf)/perfconfig),
perf_config_set can contain both user and system config items.
To avoid merging user and system config items on user config file,
we should know where each value is from.

Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Wang Nan <wangnan0@huawei.com>
Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
---
 tools/perf/builtin-config.c |  6 +++++-
 tools/perf/util/config.c    | 16 +++++++++++++++-
 tools/perf/util/config.h    |  4 +++-
 3 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c
index 2e7e295..09c0c71 100644
--- a/tools/perf/builtin-config.c
+++ b/tools/perf/builtin-config.c
@@ -46,14 +46,18 @@ static int set_config(struct perf_config_set *set, const char *file_name,
 	if (set == NULL)
 		return -1;
 
-	perf_config_set__collect(set, var, value);
+	perf_config_set__collect(set, file_name, var, value);
 	fprintf(fp, "%s\n", first_line);
 
 	/* overwrite configvariables */
 	perf_config_items__for_each_entry(&set->sections, section) {
+		if (!use_system_config && section->from_system_config)
+			continue;
 		fprintf(fp, "[%s]\n", section->name);
 
 		perf_config_items__for_each_entry(&section->items, item) {
+			if (!use_system_config && section->from_system_config)
+				continue;
 			if (item->value)
 				fprintf(fp, "\t%s = %s\n",
 					item->name, item->value);
diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
index c8fb65d..3d906db 100644
--- a/tools/perf/util/config.c
+++ b/tools/perf/util/config.c
@@ -594,6 +594,19 @@ static int collect_config(const char *var, const char *value,
 			goto out_free;
 	}
 
+	/* perf_config_set can contain both user and system config items.
+	 * So we should know where each value is from.
+	 * The classification would be needed when a particular config file
+	 * is overwrited by setting feature i.e. set_config().
+	 */
+	if (strcmp(config_file_name, perf_etc_perfconfig()) == 0) {
+		section->from_system_config = true;
+		item->from_system_config = true;
+	} else {
+		section->from_system_config = false;
+		item->from_system_config = false;
+	}
+
 	ret = set_value(item, value);
 	return ret;
 
@@ -602,9 +615,10 @@ static int collect_config(const char *var, const char *value,
 	return -1;
 }
 
-int perf_config_set__collect(struct perf_config_set *set,
+int perf_config_set__collect(struct perf_config_set *set, const char *file_name,
 			     const char *var, const char *value)
 {
+	config_file_name = file_name;
 	return collect_config(var, value, set);
 }
 
diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h
index 0fcdb8c..1a59a6b 100644
--- a/tools/perf/util/config.h
+++ b/tools/perf/util/config.h
@@ -7,12 +7,14 @@
 struct perf_config_item {
 	char *name;
 	char *value;
+	bool from_system_config;
 	struct list_head node;
 };
 
 struct perf_config_section {
 	char *name;
 	struct list_head items;
+	bool from_system_config;
 	struct list_head node;
 };
 
@@ -33,7 +35,7 @@ const char *perf_etc_perfconfig(void);
 
 struct perf_config_set *perf_config_set__new(void);
 void perf_config_set__delete(struct perf_config_set *set);
-int perf_config_set__collect(struct perf_config_set *set,
+int perf_config_set__collect(struct perf_config_set *set, const char *file_name,
 			     const char *var, const char *value);
 void perf_config__init(void);
 void perf_config__exit(void);
-- 
2.7.4

Re: [PATCH v2 0/7] perf config: Add support for config read/write

[Taeung Song]

Hi, Arnaldo :)

I guess you're so busy,
but would you mind If I ask a question ?

I sent a patchset adding support for config read/write.
(There are also Namhyung's demand lately 
https://lkml.org/lkml/2016/10/24/572)

For a while, I stoped the patchset about default config array
https://lkml.org/lkml/2016/9/5/17 .

Is it better to handle support for config reade/write, first ?


Thanks,
Taeung

On 11/14/2016 05:21 PM, Taeung Song wrote:
> Hello, :)
>
> Add simple config read/write functionalities.
>
> I had worked at the related patchset https://lkml.org/lkml/2016/2/22/38
> But I remake new this patchset for only support for read/write config file.
> And There're Namhyung's requests https://lkml.org/lkml/2016/10/24/572
>
> In particular, I agonized implement way for write functionality.
> I especially wonder other opinions about new boolean variable
> 'from_system_config' I use.
>
> If someone review this patchset and give me some feedback,
> I'd appreciated it. :)
>
> Thanks,
> Taeung
>
> v2:
> - a bit change commit messages
> - refactor parts of parse_config_arg
>
> Taeung Song (7):
>   perf config: Add support for getting functionality
>   perf config: Document examples getting config info in man page
>   perf config: Parse arguments before getting functionality
>   perf config: Parse arguments before setting functionality
>   perf config: Add support for write functionality
>   perf config: Document config setting examples in man page
>   perf config: Mark where are config items from (user or system)
>
>  tools/perf/Documentation/perf-config.txt |  35 +++++++
>  tools/perf/builtin-config.c              | 155 ++++++++++++++++++++++++++++++-
>  tools/perf/util/config.c                 |  20 ++++
>  tools/perf/util/config.h                 |   4 +
>  4 files changed, 211 insertions(+), 3 deletions(-)
>