Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1414895 > unrolled thread

[RFC][PATCH v6 0/9] perf config: Reimplement perf_config() using perf_config_set__inter()

Back to article view | Back to linux.kernel

Contents

  [RFC][PATCH v6 0/9] perf config: Reimplement perf_config() using perf_config_set__inter() Taeung Song <treeze.taeung@gmail.com> - 2016-06-06 13:00 +0200
    [PATCH v6 4/9] perf config: Use new perf_config_set__init() to initialize config set Taeung Song <treeze.taeung@gmail.com> - 2016-06-06 13:00 +0200
    [PATCH v6 9/9] perf config: Reimplement show_config() using perf_config() Taeung Song <treeze.taeung@gmail.com> - 2016-06-06 13:00 +0200
    [BUGFIX][PATCH v6 3/9] perf config: Handle the error when config set is NULL at collect_config() Taeung Song <treeze.taeung@gmail.com> - 2016-06-06 13:00 +0200
      Re: [BUGFIX][PATCH v6 3/9] perf config: Handle the error when config  set is NULL at collect_config() Arnaldo Carvalho de Melo <acme@kernel.org> - 2016-06-06 22:30 +0200
      [tip:perf/core] perf config: Handle the error when config set is  NULL at collect_config() tip-bot for Taeung Song <tipbot@zytor.com> - 2016-06-08 10:50 +0200

#1414895 — [RFC][PATCH v6 0/9] perf config: Reimplement perf_config() using perf_config_set__inter()

Hello, :)

This patchset is to reimplement perf_config() for efficient config management.

Everytime perf_config() is called, perf_config() always read config files.
(i.e. user config '~/.perfconfig' and system config '$(sysconfdir)/perfconfig')

But we need to use 'struct perf_config_set config_set' variable
that already contains all config key-value pairs
to avoid this repetitive work in perf_config().

In other words, if new perf_config() is called,
only first time 'config_set' is initialized
collecting all configs from config files and it work with perf_config_set__iter().

If we do, 'config_set' can be reused wherever using perf_config()
and a feature of old perf_config() is the same as new perf_config()
work without the repetitive work that read the config files.

IMHO, I think this patchset is needed because not only the repetitive work
should be avoided but also in near future, it would be smooth to manage perf configs.

Most important patch of this patchset is "[PATCH v5 7/9] perf config: Reimplement
perf_config() using perf_config_set__iter()" and PATCH 1/9 ~ 6/9 are preparation for it.

If you give me any feedback, I'd apprecicated it. :)

Thanks,
Taeung

v6:
- add printing error message when perf_config_set__iter() is failed
- modify commit messages for bugfix 1~3 (PATCH 1/9 ~ 3/9)
  to help reviewers easily understand why them is needed

v5:
- solve the leak when perf_config_set__init() failed (Arnaldo)
  (to clear the problem it is needed to apply the bottom bugfix 1~3 patches)
- bugfix 1) fix the problem of abnormal terminaltion at perf_parse_file() called by perf_config()
- bugfix 2) if failed at collect_config(), finally free a config set
            after it is done instead of freeing the config set in the function
- bugfix 3) handle NULL pointer exception of 'set' at collect_config()

v4:
- Keep perf_config_set__delete() as it is (Arnaldo)
- Remove perf_config_set__check() (Arnaldo)
- Keep the existing code about the config set at cmd_config() (Arnaldo)

v3:
- add freeing config set after sub-command work at run_builtin() (Namhyung)
- remove needless code about the config set at cmd_config()
- add a patch about a global variable 'config_set'

v2:
- split a patch into several patches
- reimplement show_config() using new perf_config()
- modify perf_config_set__delete using global variable 'config_set'
- reset config set when only 'config' sub-commaned work
  because of options for config file location

Taeung Song (9):
  perf config: Fix the problem of abnormal termination at
    perf_parse_file()
  perf config: If collect_config() is failed, finally free a config set
    after it is done
  perf config: Handle the error when config set is NULL at
    collect_config()
  perf config: Use new perf_config_set__init() to initialize config set
  perf config: Add global variable 'config_set'
  perf config: Use zfree() instead of free() at
    perf_config_set__delete()
  perf config: Reimplement perf_config() using perf_config_set__iter()
  perf config: Reset the config set at only 'config' sub-command
  perf config: Reimplement show_config() using perf_config()

 tools/perf/builtin-config.c |  41 +++++-------
 tools/perf/perf.c           |   1 +
 tools/perf/util/cache.h     |   1 +
 tools/perf/util/config.c    | 159 +++++++++++++++++++++++++++++---------------
 tools/perf/util/config.h    |   4 +-
 5 files changed, 124 insertions(+), 82 deletions(-)

-- 
2.5.0

[toc] | [next] | [standalone]

#1414896 — [PATCH v6 4/9] perf config: Use new perf_config_set__init() to initialize config set

Instead of perf_config(), This function initialize config set
collecting all configs from config files (i.e. user config
~/.perfconfig and system config $(sysconfdir)/perfconfig).

If there are the same config variable both user and system
config file, user config has higher priority than system config.

Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
---
 tools/perf/util/config.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 48 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
index 062eeb8..6bfa112 100644
--- a/tools/perf/util/config.c
+++ b/tools/perf/util/config.c
@@ -646,14 +646,61 @@ out_free:
 	return -1;
 }
 
+static int perf_config_set__init(struct perf_config_set *set)
+{
+	int ret = -1;
+	const char *home = NULL;
+
+	/* Setting $PERF_CONFIG makes perf read _only_ the given config file. */
+	if (config_exclusive_filename)
+		return perf_config_from_file(collect_config, config_exclusive_filename, set);
+	if (perf_config_system() && !access(perf_etc_perfconfig(), R_OK)) {
+		if (perf_config_from_file(collect_config, perf_etc_perfconfig(), set) < 0)
+			goto out;
+	}
+
+	home = getenv("HOME");
+	if (perf_config_global() && home) {
+		char *user_config = strdup(mkpath("%s/.perfconfig", home));
+		struct stat st;
+
+		if (user_config == NULL) {
+			warning("Not enough memory to process %s/.perfconfig, "
+				"ignoring it.", home);
+			goto out;
+		}
+
+		if (stat(user_config, &st) < 0)
+			goto out_free;
+
+		if (st.st_uid && (st.st_uid != geteuid())) {
+			warning("File %s not owned by current user or root, "
+				"ignoring it.", user_config);
+			goto out_free;
+		}
+
+		if (!st.st_size)
+			goto out_free;
+
+		ret = perf_config_from_file(collect_config, user_config, set);
+
+out_free:
+		free(user_config);
+	}
+out:
+	return ret;
+}
+
 struct perf_config_set *perf_config_set__new(void)
 {
 	struct perf_config_set *set = zalloc(sizeof(*set));
 
 	if (set) {
 		INIT_LIST_HEAD(&set->sections);
-		if (perf_config(collect_config, set) < 0)
+		if (perf_config_set__init(set) < 0) {
 			perf_config_set__delete(set);
+			return NULL;
+		}
 	}
 
 	return set;
-- 
2.5.0

[toc] | [prev] | [next] | [standalone]

#1414897 — [PATCH v6 9/9] perf config: Reimplement show_config() using perf_config()

Old show_config() directly use config set so
there are many duplicated code with perf_config_set__iter().

So reimplement show_config() using perf_config() that use
perf_config_set__iter() with config set that already
contains all configs.

Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
---
 tools/perf/builtin-config.c | 29 +++++++----------------------
 1 file changed, 7 insertions(+), 22 deletions(-)

diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c
index 4dab41e..b6ae8ea 100644
--- a/tools/perf/builtin-config.c
+++ b/tools/perf/builtin-config.c
@@ -33,28 +33,13 @@ static struct option config_options[] = {
 	OPT_END()
 };
 
-static int show_config(struct perf_config_set *set)
+static int show_config(const char *key, const char *value,
+		       void *cb __maybe_unused)
 {
-	struct perf_config_section *section;
-	struct perf_config_item *item;
-	struct list_head *sections;
-
-	if (set == NULL)
-		return -1;
-
-	sections = &set->sections;
-	if (list_empty(sections))
-		return -1;
-
-	list_for_each_entry(section, sections, node) {
-		list_for_each_entry(item, &section->items, node) {
-			char *value = item->value;
-
-			if (value)
-				printf("%s.%s=%s\n", section->name,
-				       item->name, value);
-		}
-	}
+	if (value)
+		printf("%s=%s\n", key, value);
+	else
+		printf("%s\n", key);
 
 	return 0;
 }
@@ -96,7 +81,7 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)
 			pr_err("Error: takes no arguments\n");
 			parse_options_usage(config_usage, config_options, "l", 1);
 		} else {
-			ret = show_config(config_set);
+			ret = perf_config(show_config, NULL);
 			if (ret < 0) {
 				const char * config_filename = config_exclusive_filename;
 				if (!config_exclusive_filename)
-- 
2.5.0

[toc] | [prev] | [next] | [standalone]

#1414899 — [BUGFIX][PATCH v6 3/9] perf config: Handle the error when config set is NULL at collect_config()

collect_config() collect all config key-value pairs
from config files and put each config info in config set.
But if config set (i.e. 'set' variable at collect_config())
is NULL, this is wrong so handle it.

Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
---
 tools/perf/util/config.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
index d013f90..062eeb8 100644
--- a/tools/perf/util/config.c
+++ b/tools/perf/util/config.c
@@ -607,8 +607,12 @@ static int collect_config(const char *var, const char *value,
 	struct perf_config_section *section = NULL;
 	struct perf_config_item *item = NULL;
 	struct perf_config_set *set = perf_config_set;
-	struct list_head *sections = &set->sections;
+	struct list_head *sections;
 
+	if (set == NULL)
+		return -1;
+
+	sections = &set->sections;
 	key = ptr = strdup(var);
 	if (!key) {
 		pr_debug("%s: strdup failed\n", __func__);
-- 
2.5.0

[toc] | [prev] | [next] | [standalone]

#1415420 — Re: [BUGFIX][PATCH v6 3/9] perf config: Handle the error when config set is NULL at collect_config()

Em Mon, Jun 06, 2016 at 07:52:54PM +0900, Taeung Song escreveu:
> collect_config() collect all config key-value pairs
> from config files and put each config info in config set.
> But if config set (i.e. 'set' variable at collect_config())
> is NULL, this is wrong so handle it.

Looks ok, applied.

- Arnaldo
 
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Jiri Olsa <jolsa@redhat.com>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
> ---
>  tools/perf/util/config.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
> index d013f90..062eeb8 100644
> --- a/tools/perf/util/config.c
> +++ b/tools/perf/util/config.c
> @@ -607,8 +607,12 @@ static int collect_config(const char *var, const char *value,
>  	struct perf_config_section *section = NULL;
>  	struct perf_config_item *item = NULL;
>  	struct perf_config_set *set = perf_config_set;
> -	struct list_head *sections = &set->sections;
> +	struct list_head *sections;
>  
> +	if (set == NULL)
> +		return -1;
> +
> +	sections = &set->sections;
>  	key = ptr = strdup(var);
>  	if (!key) {
>  		pr_debug("%s: strdup failed\n", __func__);
> -- 
> 2.5.0

[toc] | [prev] | [next] | [standalone]

#1417077 — [tip:perf/core] perf config: Handle the error when config set is NULL at collect_config()

Commit-ID:  7db91f251056f90fec4121f028680ab3153a0f3c
Gitweb:     http://git.kernel.org/tip/7db91f251056f90fec4121f028680ab3153a0f3c
Author:     Taeung Song <treeze.taeung@gmail.com>
AuthorDate: Mon, 6 Jun 2016 19:52:54 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Mon, 6 Jun 2016 17:43:19 -0300

perf config: Handle the error when config set is NULL at collect_config()

collect_config() collect all config key-value pairs from config files
and put each config info in config set.  But if config set (i.e. 'set'
variable at collect_config()) is NULL, this is wrong so handle it.

Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Link: http://lkml.kernel.org/r/1465210380-26749-4-git-send-email-treeze.taeung@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/config.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
index b500737..c73f1c4 100644
--- a/tools/perf/util/config.c
+++ b/tools/perf/util/config.c
@@ -607,8 +607,12 @@ static int collect_config(const char *var, const char *value,
 	struct perf_config_section *section = NULL;
 	struct perf_config_item *item = NULL;
 	struct perf_config_set *set = perf_config_set;
-	struct list_head *sections = &set->sections;
+	struct list_head *sections;
 
+	if (set == NULL)
+		return -1;
+
+	sections = &set->sections;
 	key = ptr = strdup(var);
 	if (!key) {
 		pr_debug("%s: strdup failed\n", __func__);

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web