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


Groups > linux.kernel > #1415931 > unrolled thread

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

Started byTaeung Song <treeze.taeung@gmail.com>
First post2016-06-07 11:30 +0200
Last post2016-06-08 10:50 +0200
Articles 3 — 2 participants

Back to article view | Back to linux.kernel


Contents

  [RFC][PATCH v7 0/9] perf config: Reimplement perf_config() using perf_config_set__inter() Taeung Song <treeze.taeung@gmail.com> - 2016-06-07 11:30 +0200
    [PATCH v7 2/7] perf config: Use new perf_config_set__init() to initialize config set Taeung Song <treeze.taeung@gmail.com> - 2016-06-07 11:40 +0200
      [tip:perf/core] perf config: Use new perf_config_set__init() to  initialize config set tip-bot for Taeung Song <tipbot@zytor.com> - 2016-06-08 10:50 +0200

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

FromTaeung Song <treeze.taeung@gmail.com>
Date2016-06-07 11:30 +0200
Subject[RFC][PATCH v7 0/9] perf config: Reimplement perf_config() using perf_config_set__inter()
Message-ID<rHluF-Te-3@gated-at.bofh.it>
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.

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

Thanks,
Taeung

v7:
- fill a missing crumb that assign NULL to 'set' variable in perf_config_set__new()
  (Arnaldo)

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 (7):
  perf config: If collect_config() is failed, finally free a config set
    after it is done
  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    | 147 +++++++++++++++++++++++++++++---------------
 tools/perf/util/config.h    |   4 +-
 5 files changed, 117 insertions(+), 77 deletions(-)

-- 
2.5.0

[toc] | [next] | [standalone]


#1415940 — [PATCH v7 2/7] perf config: Use new perf_config_set__init() to initialize config set

FromTaeung Song <treeze.taeung@gmail.com>
Date2016-06-07 11:40 +0200
Subject[PATCH v7 2/7] perf config: Use new perf_config_set__init() to initialize config set
Message-ID<rHlEm-Wz-21@gated-at.bofh.it>
In reply to#1415931
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 | 47 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 46 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
index e086f59..8749eca 100644
--- a/tools/perf/util/config.c
+++ b/tools/perf/util/config.c
@@ -646,13 +646,58 @@ 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);
 			set = NULL;
 		}
-- 
2.5.0

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


#1417083 — [tip:perf/core] perf config: Use new perf_config_set__init() to initialize config set

Fromtip-bot for Taeung Song <tipbot@zytor.com>
Date2016-06-08 10:50 +0200
Subject[tip:perf/core] perf config: Use new perf_config_set__init() to initialize config set
Message-ID<rHHlw-6ig-37@gated-at.bofh.it>
In reply to#1415940
Commit-ID:  8beeb00f2c8498686eee02b8edcd1488b903c90b
Gitweb:     http://git.kernel.org/tip/8beeb00f2c8498686eee02b8edcd1488b903c90b
Author:     Taeung Song <treeze.taeung@gmail.com>
AuthorDate: Tue, 7 Jun 2016 18:26:12 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Tue, 7 Jun 2016 11:01:25 -0300

perf config: Use new perf_config_set__init() to initialize config set

Instead of perf_config(), this function initializes config set by
reading various files: user config ~/.perfconfig and system config
$(sysconfdir)/perfconfig).

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

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/1465291577-20973-3-git-send-email-treeze.taeung@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/config.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 46 insertions(+), 1 deletion(-)

diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
index e086f59..8749eca 100644
--- a/tools/perf/util/config.c
+++ b/tools/perf/util/config.c
@@ -646,13 +646,58 @@ 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);
 			set = NULL;
 		}

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web