linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v10 0/3] perf config: Reimplement perf_config()
@ 2016-06-23  8:55 Taeung Song
  2016-06-23  8:55 ` [PATCH v10 1/3] perf config: Bring declarations about config from util/cache.h to util/config.h Taeung Song
                   ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Taeung Song @ 2016-06-23  8:55 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Jiri Olsa, Namhyung Kim, Ingo Molnar,
	Peter Zijlstra, Alexander Shishkin, Masami Hiramatsu, Wang Nan,
	Taeung Song

Hello, :)

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

Many sub-commands use perf_config() but
everytime perf_config() is called, perf_config() always read config files.
(i.e. user config '~/.perfconfig' and system config '$(sysconfdir)/perfconfig')

But it is better to use the config set that already contains all config
key-value pairs to avoid this repetitive work reading the config files
in perf_config().

In summary, in order to use features about configuration,
we can call the functions at perf.c and other source files as below.

    # initialize a config set
    perf_config_init()

    # configure actual variables from a config set
    perf_config()

    # eliminate allocated config set
    perf_config_finish()

    # destroy existing config set and initialize a new config set.
    perf_config_refresh()

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

v10:
- rebased onto current acme/perf/core

v9:
- add config_set__for_each macro (Arnaldo)
- the source files that only need config.h don't include cache.h (Arnaldo)
- use 'config_set' as a static variable instead of a global variable. (Namhyung)
- add perf_config_init(), perf_config_finish() and perf_config_refresh() (Namhyung)
- remove [PATCH v8 4/5] perf config: Use zfree() instead of free() at perf_config_set__delete()
- rebased onto current acme/perf/core
- applied ([BUGFIX][PATCH v8 1/5] 826424)

v8:
- handle the error about NULL at perf_config_set__delete()
- bring declarations about config from util/config.h to util/config.h
- reimplement show_config() using perf_config_set__iter() instead of perf_config()
- rebased onto perf-core-for-mingo-20160607
- applied ([PATCH v7 1/7] 25d8f48, [PATCH v7 2/7] 8beeb00)

v7:
- fill a missing crumb that assign NULL to 'set' variable in perf_config_set__new()
  (Arnaldo)
- two patches applied ([PATCH v6 1/9] 78f71c9, [PATCH v6 3/9] 7db91f2)

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 (3):
  perf config: Bring declarations about config from util/cache.h to
    util/config.h
  perf config: Reimplement perf_config() introducing new
    perf_config_init() and perf_config_finish()
  perf config: Reimplement show_config() using config_set__for_each

 tools/perf/builtin-config.c        | 21 ++++-----
 tools/perf/builtin-help.c          |  2 +-
 tools/perf/builtin-kmem.c          |  2 +-
 tools/perf/builtin-record.c        |  1 +
 tools/perf/builtin-report.c        |  2 +-
 tools/perf/builtin-top.c           |  2 +-
 tools/perf/perf.c                  |  4 +-
 tools/perf/ui/browser.c            |  2 +-
 tools/perf/ui/browsers/annotate.c  |  1 +
 tools/perf/util/alias.c            |  1 +
 tools/perf/util/cache.h            | 11 -----
 tools/perf/util/color.c            |  1 +
 tools/perf/util/config.c           | 92 +++++++++++++++++++-------------------
 tools/perf/util/config.h           | 40 +++++++++++++++++
 tools/perf/util/help-unknown-cmd.c |  1 +
 tools/perf/util/intel-pt.c         |  1 +
 tools/perf/util/llvm-utils.c       |  1 +
 17 files changed, 111 insertions(+), 74 deletions(-)

-- 
2.5.0

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v10 1/3] perf config: Bring declarations about config from util/cache.h to util/config.h
  2016-06-23  8:55 [PATCH v10 0/3] perf config: Reimplement perf_config() Taeung Song
@ 2016-06-23  8:55 ` Taeung Song
  2016-06-23 11:57   ` Arnaldo Carvalho de Melo
  2016-06-26 10:55   ` [tip:perf/core] perf config: Move config declarations " tip-bot for Taeung Song
  2016-06-23  8:55 ` [PATCH v10 2/3] perf config: Reimplement perf_config() introducing new perf_config_init() and perf_config_finish() Taeung Song
  2016-06-23  8:55 ` [PATCH v10 3/3] perf config: Reimplement show_config() using config_set__for_each Taeung Song
  2 siblings, 2 replies; 9+ messages in thread
From: Taeung Song @ 2016-06-23  8:55 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Jiri Olsa, Namhyung Kim, Ingo Molnar,
	Peter Zijlstra, Alexander Shishkin, Masami Hiramatsu, Wang Nan,
	Taeung Song, Jiri Olsa

Lately util/config.h has been added
but util/cache.h has declarations of functions
and a global variable for config features.

To manage codes about configuration at one spot,
move them to util/config.h and let source files
that need config features include config.h
And if the source files that included previous cache.h
need only config.h, remove including cache.h.

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: Wang Nan <wangnan0@huawei.com>
Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
---
 tools/perf/builtin-help.c          |  2 +-
 tools/perf/builtin-kmem.c          |  2 +-
 tools/perf/builtin-record.c        |  1 +
 tools/perf/builtin-report.c        |  2 +-
 tools/perf/builtin-top.c           |  2 +-
 tools/perf/perf.c                  |  2 +-
 tools/perf/ui/browser.c            |  2 +-
 tools/perf/ui/browsers/annotate.c  |  1 +
 tools/perf/util/alias.c            |  1 +
 tools/perf/util/cache.h            | 11 -----------
 tools/perf/util/color.c            |  1 +
 tools/perf/util/config.h           | 11 +++++++++++
 tools/perf/util/help-unknown-cmd.c |  1 +
 tools/perf/util/intel-pt.c         |  1 +
 tools/perf/util/llvm-utils.c       |  1 +
 15 files changed, 24 insertions(+), 17 deletions(-)

diff --git a/tools/perf/builtin-help.c b/tools/perf/builtin-help.c
index f9830c9..268ab73 100644
--- a/tools/perf/builtin-help.c
+++ b/tools/perf/builtin-help.c
@@ -4,7 +4,7 @@
  * Builtin help command
  */
 #include "perf.h"
-#include "util/cache.h"
+#include "util/config.h"
 #include "builtin.h"
 #include <subcmd/exec-cmd.h>
 #include "common-cmds.h"
diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c
index 58adfee..4defe44 100644
--- a/tools/perf/builtin-kmem.c
+++ b/tools/perf/builtin-kmem.c
@@ -4,7 +4,7 @@
 #include "util/evlist.h"
 #include "util/evsel.h"
 #include "util/util.h"
-#include "util/cache.h"
+#include "util/config.h"
 #include "util/symbol.h"
 #include "util/thread.h"
 #include "util/header.h"
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index b1304eb..c97b2b69 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -13,6 +13,7 @@
 #include "util/util.h"
 #include <subcmd/parse-options.h>
 #include "util/parse-events.h"
+#include "util/config.h"
 
 #include "util/callchain.h"
 #include "util/cgroup.h"
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 9f36b23..bcb49ff 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -8,7 +8,7 @@
 #include "builtin.h"
 
 #include "util/util.h"
-#include "util/cache.h"
+#include "util/config.h"
 
 #include "util/annotate.h"
 #include "util/color.h"
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 81dba80..ec4cba6 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -22,7 +22,7 @@
 #include "perf.h"
 
 #include "util/annotate.h"
-#include "util/cache.h"
+#include "util/config.h"
 #include "util/color.h"
 #include "util/evlist.h"
 #include "util/evsel.h"
diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index 634bf7c..66772da 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -10,7 +10,7 @@
 
 #include "util/env.h"
 #include <subcmd/exec-cmd.h>
-#include "util/cache.h"
+#include "util/config.h"
 #include "util/quote.h"
 #include <subcmd/run-command.h>
 #include "util/parse-events.h"
diff --git a/tools/perf/ui/browser.c b/tools/perf/ui/browser.c
index af68a9d..3eb3edb 100644
--- a/tools/perf/ui/browser.c
+++ b/tools/perf/ui/browser.c
@@ -1,5 +1,5 @@
 #include "../util.h"
-#include "../cache.h"
+#include "../config.h"
 #include "../../perf.h"
 #include "libslang.h"
 #include "ui.h"
diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index 4fc208e..0e106bb 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -8,6 +8,7 @@
 #include "../../util/sort.h"
 #include "../../util/symbol.h"
 #include "../../util/evsel.h"
+#include "../../util/config.h"
 #include <pthread.h>
 
 struct disasm_line_samples {
diff --git a/tools/perf/util/alias.c b/tools/perf/util/alias.c
index c0b43ee..6c80f83 100644
--- a/tools/perf/util/alias.c
+++ b/tools/perf/util/alias.c
@@ -1,4 +1,5 @@
 #include "cache.h"
+#include "config.h"
 
 static const char *alias_key;
 static char *alias_val;
diff --git a/tools/perf/util/cache.h b/tools/perf/util/cache.h
index 369f382..9f90e36 100644
--- a/tools/perf/util/cache.h
+++ b/tools/perf/util/cache.h
@@ -18,17 +18,6 @@
 #define PERF_TRACEFS_ENVIRONMENT "PERF_TRACEFS_DIR"
 #define PERF_PAGER_ENVIRONMENT "PERF_PAGER"
 
-extern const char *config_exclusive_filename;
-
-typedef int (*config_fn_t)(const char *, const char *, void *);
-int perf_default_config(const char *, const char *, void *);
-int perf_config(config_fn_t fn, void *);
-int perf_config_int(const char *, const char *);
-u64 perf_config_u64(const char *, const char *);
-int perf_config_bool(const char *, const char *);
-int config_error_nonbool(const char *);
-const char *perf_etc_perfconfig(void);
-
 char *alias_lookup(const char *alias);
 int split_cmdline(char *cmdline, const char ***argv);
 
diff --git a/tools/perf/util/color.c b/tools/perf/util/color.c
index 43e84aa..1210ba5 100644
--- a/tools/perf/util/color.c
+++ b/tools/perf/util/color.c
@@ -1,5 +1,6 @@
 #include <linux/kernel.h>
 #include "cache.h"
+#include "config.h"
 #include "color.h"
 #include <math.h>
 
diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h
index 22ec626..155a441 100644
--- a/tools/perf/util/config.h
+++ b/tools/perf/util/config.h
@@ -20,6 +20,17 @@ struct perf_config_set {
 	struct list_head sections;
 };
 
+extern const char *config_exclusive_filename;
+
+typedef int (*config_fn_t)(const char *, const char *, void *);
+int perf_default_config(const char *, const char *, void *);
+int perf_config(config_fn_t fn, void *);
+int perf_config_int(const char *, const char *);
+u64 perf_config_u64(const char *, const char *);
+int perf_config_bool(const char *, const char *);
+int config_error_nonbool(const char *);
+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);
 
diff --git a/tools/perf/util/help-unknown-cmd.c b/tools/perf/util/help-unknown-cmd.c
index d62ccae..776e285 100644
--- a/tools/perf/util/help-unknown-cmd.c
+++ b/tools/perf/util/help-unknown-cmd.c
@@ -1,4 +1,5 @@
 #include "cache.h"
+#include "config.h"
 #include <subcmd/help.h>
 #include "../builtin.h"
 #include "levenshtein.h"
diff --git a/tools/perf/util/intel-pt.c b/tools/perf/util/intel-pt.c
index 1371969..a2fe3a2 100644
--- a/tools/perf/util/intel-pt.c
+++ b/tools/perf/util/intel-pt.c
@@ -39,6 +39,7 @@
 #include "auxtrace.h"
 #include "tsc.h"
 #include "intel-pt.h"
+#include "config.h"
 
 #include "intel-pt-decoder/intel-pt-log.h"
 #include "intel-pt-decoder/intel-pt-decoder.h"
diff --git a/tools/perf/util/llvm-utils.c b/tools/perf/util/llvm-utils.c
index 878a566..40b6f72 100644
--- a/tools/perf/util/llvm-utils.c
+++ b/tools/perf/util/llvm-utils.c
@@ -8,6 +8,7 @@
 #include <stdlib.h>
 #include "debug.h"
 #include "llvm-utils.h"
+#include "config.h"
 
 #define CLANG_BPF_CMD_DEFAULT_TEMPLATE				\
 		"$CLANG_EXEC -D__KERNEL__ -D__NR_CPUS__=$NR_CPUS "\
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v10 2/3] perf config: Reimplement perf_config() introducing new perf_config_init() and perf_config_finish()
  2016-06-23  8:55 [PATCH v10 0/3] perf config: Reimplement perf_config() Taeung Song
  2016-06-23  8:55 ` [PATCH v10 1/3] perf config: Bring declarations about config from util/cache.h to util/config.h Taeung Song
@ 2016-06-23  8:55 ` Taeung Song
  2016-06-23 11:56   ` Arnaldo Carvalho de Melo
  2016-06-23  8:55 ` [PATCH v10 3/3] perf config: Reimplement show_config() using config_set__for_each Taeung Song
  2 siblings, 1 reply; 9+ messages in thread
From: Taeung Song @ 2016-06-23  8:55 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Jiri Olsa, Namhyung Kim, Ingo Molnar,
	Peter Zijlstra, Alexander Shishkin, Masami Hiramatsu, Wang Nan,
	Taeung Song, Jiri Olsa, Ingo Molnar

Many sub-commands use perf_config() but
everytime perf_config() is called, perf_config() always read config files.
(i.e. user config '~/.perfconfig' and system config '$(sysconfdir)/perfconfig')

But it is better to use the config set that already contains all config
key-value pairs to avoid this repetitive work reading the config files
in perf_config(). (the config set mean a static variable 'config_set')

In other words, if new perf_config_init() is called,
only first time 'config_set' is initialized collecting all configs from the config files.
And then we could use new perf_config() like old perf_config().
When a sub-command finished, free the config set by perf_config_finish() at run_builtin().

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

In summary, in order to use features about configuration,
we can call the functions at perf.c and other source files as below.

    # initialize a config set
    perf_config_init()

    # configure actual variables from a config set
    perf_config()

    # eliminate allocated config set
    perf_config_finish()

    # destroy existing config set and initialize a new config set.
    perf_config_refresh()

Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Jiri Olsa <jolsa@redhat.com>
Cc: Wang Nan <wangnan0@huawei.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@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 |  4 ++
 tools/perf/perf.c           |  2 +
 tools/perf/util/config.c    | 92 +++++++++++++++++++++++----------------------
 tools/perf/util/config.h    | 29 ++++++++++++++
 4 files changed, 82 insertions(+), 45 deletions(-)

diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c
index fe1b77f..cfd1036 100644
--- a/tools/perf/builtin-config.c
+++ b/tools/perf/builtin-config.c
@@ -80,6 +80,10 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)
 	else if (use_user_config)
 		config_exclusive_filename = user_config;
 
+	/*
+	 * At only 'config' sub-command, individually use the config set
+	 * because of reinitializing with options config file location.
+	 */
 	set = perf_config_set__new();
 	if (!set) {
 		ret = -1;
diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index 66772da..280967e 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -355,6 +355,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
 
 	perf_env__set_cmdline(&perf_env, argc, argv);
 	status = p->fn(argc, argv, prefix);
+	perf_config_finish();
 	exit_browser(status);
 	perf_env__exit(&perf_env);
 	bpf__clear();
@@ -522,6 +523,7 @@ int main(int argc, const char **argv)
 
 	srandom(time(NULL));
 
+	perf_config_init();
 	perf_config(perf_default_config, NULL);
 	set_buildid_dir(NULL);
 
diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
index d15c592..a16f95d 100644
--- a/tools/perf/util/config.c
+++ b/tools/perf/util/config.c
@@ -26,6 +26,7 @@ static FILE *config_file;
 static const char *config_file_name;
 static int config_linenr;
 static int config_file_eof;
+static struct perf_config_set *config_set;
 
 const char *config_exclusive_filename;
 
@@ -478,51 +479,6 @@ static int perf_config_global(void)
 	return !perf_env_bool("PERF_CONFIG_NOGLOBAL", 0);
 }
 
-int perf_config(config_fn_t fn, void *data)
-{
-	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(fn, config_exclusive_filename, data);
-	if (perf_config_system() && !access(perf_etc_perfconfig(), R_OK)) {
-		if (perf_config_from_file(fn, perf_etc_perfconfig(), data) < 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(fn, user_config, data);
-
-out_free:
-		free(user_config);
-	}
-out:
-	return ret;
-}
-
 static struct perf_config_section *find_section(struct list_head *sections,
 						const char *section_name)
 {
@@ -706,6 +662,52 @@ struct perf_config_set *perf_config_set__new(void)
 	return set;
 }
 
+int perf_config(config_fn_t fn, void *data)
+{
+	int ret = 0;
+	char key[BUFSIZ];
+	struct perf_config_section *section;
+	struct perf_config_item *item;
+
+	if (config_set == NULL)
+		return -1;
+
+	config_set__for_each(config_set, section, item) {
+		char *value = item->value;
+
+		if (value) {
+			scnprintf(key, sizeof(key), "%s.%s",
+				  section->name, item->name);
+			ret = fn(key, value, data);
+			if (ret < 0) {
+				pr_err("Error: wrong config key-value pair %s=%s\n",
+				       key, value);
+				break;
+			}
+		}
+	}
+
+	return ret;
+}
+
+void perf_config_init(void)
+{
+	if (config_set == NULL)
+		config_set = perf_config_set__new();
+}
+
+void perf_config_finish(void)
+{
+	perf_config_set__delete(config_set);
+	config_set = NULL;
+}
+
+void perf_config_refresh(void)
+{
+	perf_config_finish();
+	perf_config_init();
+}
+
 static void perf_config_item__delete(struct perf_config_item *item)
 {
 	zfree(&item->name);
diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h
index 155a441..746c619 100644
--- a/tools/perf/util/config.h
+++ b/tools/perf/util/config.h
@@ -33,5 +33,34 @@ 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);
+void perf_config_init(void);
+void perf_config_finish(void);
+void perf_config_refresh(void);
+
+/**
+ * config_sections__for_each - iterate thru all the sections
+ * @list: list_head instance to iterate
+ * @section: struct perf_config_section iterator
+ */
+#define config_sections__for_each(list, section)	\
+        list_for_each_entry(section, list, node)
+
+/**
+ * config_items__for_each - iterate thru all the items
+ * @list: list_head instance to iterate
+ * @item: struct perf_config_item iterator
+ */
+#define config_items__for_each(list, item)	\
+        list_for_each_entry(item, list, node)
+
+/**
+ * config_set__for_each - iterate thru all the config section-item pairs
+ * @set: evlist instance to iterate
+ * @section: struct perf_config_section iterator
+ * @item: struct perf_config_item iterator
+ */
+#define config_set__for_each(set, section, item)			\
+	config_sections__for_each(&set->sections, section)		\
+	config_items__for_each(&section->items, item)
 
 #endif /* __PERF_CONFIG_H */
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v10 3/3] perf config: Reimplement show_config() using config_set__for_each
  2016-06-23  8:55 [PATCH v10 0/3] perf config: Reimplement perf_config() Taeung Song
  2016-06-23  8:55 ` [PATCH v10 1/3] perf config: Bring declarations about config from util/cache.h to util/config.h Taeung Song
  2016-06-23  8:55 ` [PATCH v10 2/3] perf config: Reimplement perf_config() introducing new perf_config_init() and perf_config_finish() Taeung Song
@ 2016-06-23  8:55 ` Taeung Song
  2 siblings, 0 replies; 9+ messages in thread
From: Taeung Song @ 2016-06-23  8:55 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Jiri Olsa, Namhyung Kim, Ingo Molnar,
	Peter Zijlstra, Alexander Shishkin, Masami Hiramatsu, Wang Nan,
	Taeung Song, Jiri Olsa

Lately config_set__for_each is added.
In order to let show_config() be short and clear,
remake this function using config_set__for_each macro

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>
Cc: Wang Nan <wangnan0@huawei.com>
Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
---
 tools/perf/builtin-config.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c
index cfd1036..c144643 100644
--- a/tools/perf/builtin-config.c
+++ b/tools/perf/builtin-config.c
@@ -37,23 +37,16 @@ static int show_config(struct perf_config_set *set)
 {
 	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;
+	config_set__for_each(set, section, item) {
+		char *value = item->value;
 
-			if (value)
-				printf("%s.%s=%s\n", section->name,
-				       item->name, value);
-		}
+		if (value)
+			printf("%s.%s=%s\n", section->name,
+			       item->name, value);
 	}
 
 	return 0;
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v10 2/3] perf config: Reimplement perf_config() introducing new perf_config_init() and perf_config_finish()
  2016-06-23  8:55 ` [PATCH v10 2/3] perf config: Reimplement perf_config() introducing new perf_config_init() and perf_config_finish() Taeung Song
@ 2016-06-23 11:56   ` Arnaldo Carvalho de Melo
  2016-06-23 12:41     ` Taeung Song
  0 siblings, 1 reply; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-06-23 11:56 UTC (permalink / raw)
  To: Taeung Song
  Cc: linux-kernel, Jiri Olsa, Namhyung Kim, Ingo Molnar,
	Peter Zijlstra, Alexander Shishkin, Masami Hiramatsu, Wang Nan,
	Jiri Olsa, Ingo Molnar

Em Thu, Jun 23, 2016 at 05:55:18PM +0900, Taeung Song escreveu:
> Many sub-commands use perf_config() but
> everytime perf_config() is called, perf_config() always read config files.
> (i.e. user config '~/.perfconfig' and system config '$(sysconfdir)/perfconfig')
> 
> But it is better to use the config set that already contains all config
> key-value pairs to avoid this repetitive work reading the config files
> in perf_config(). (the config set mean a static variable 'config_set')
> 
> In other words, if new perf_config_init() is called,
> only first time 'config_set' is initialized collecting all configs from the config files.
> And then we could use new perf_config() like old perf_config().
> When a sub-command finished, free the config set by perf_config_finish() at run_builtin().
> 
> If we do, 'config_set' can be reused wherever perf_config() is called
> and a feature of old perf_config() is the same as new perf_config() work
> without the repetitive work that read the config files.
> 
> In summary, in order to use features about configuration,
> we can call the functions at perf.c and other source files as below.
> 
>     # initialize a config set
>     perf_config_init()
> 
>     # configure actual variables from a config set
>     perf_config()
> 
>     # eliminate allocated config set
>     perf_config_finish()
> 
>     # destroy existing config set and initialize a new config set.
>     perf_config_refresh()
> 
> Cc: Namhyung Kim <namhyung@kernel.org>
> Cc: Jiri Olsa <jolsa@redhat.com>
> Cc: Wang Nan <wangnan0@huawei.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Ingo Molnar <mingo@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 |  4 ++
>  tools/perf/perf.c           |  2 +
>  tools/perf/util/config.c    | 92 +++++++++++++++++++++++----------------------
>  tools/perf/util/config.h    | 29 ++++++++++++++
>  4 files changed, 82 insertions(+), 45 deletions(-)
> 
> diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c
> index fe1b77f..cfd1036 100644
> --- a/tools/perf/builtin-config.c
> +++ b/tools/perf/builtin-config.c
> @@ -80,6 +80,10 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)
>  	else if (use_user_config)
>  		config_exclusive_filename = user_config;
>  
> +	/*
> +	 * At only 'config' sub-command, individually use the config set
> +	 * because of reinitializing with options config file location.
> +	 */
>  	set = perf_config_set__new();
>  	if (!set) {
>  		ret = -1;
> diff --git a/tools/perf/perf.c b/tools/perf/perf.c
> index 66772da..280967e 100644
> --- a/tools/perf/perf.c
> +++ b/tools/perf/perf.c
> @@ -355,6 +355,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
>  
>  	perf_env__set_cmdline(&perf_env, argc, argv);
>  	status = p->fn(argc, argv, prefix);
> +	perf_config_finish();
>  	exit_browser(status);
>  	perf_env__exit(&perf_env);
>  	bpf__clear();
> @@ -522,6 +523,7 @@ int main(int argc, const char **argv)
>  
>  	srandom(time(NULL));
>  
> +	perf_config_init();
>  	perf_config(perf_default_config, NULL);
>  	set_buildid_dir(NULL);
>  
> diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
> index d15c592..a16f95d 100644
> --- a/tools/perf/util/config.c
> +++ b/tools/perf/util/config.c
> @@ -26,6 +26,7 @@ static FILE *config_file;
>  static const char *config_file_name;
>  static int config_linenr;
>  static int config_file_eof;
> +static struct perf_config_set *config_set;
>  
>  const char *config_exclusive_filename;
>  
> @@ -478,51 +479,6 @@ static int perf_config_global(void)
>  	return !perf_env_bool("PERF_CONFIG_NOGLOBAL", 0);
>  }
>  
> -int perf_config(config_fn_t fn, void *data)
> -{
> -	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(fn, config_exclusive_filename, data);
> -	if (perf_config_system() && !access(perf_etc_perfconfig(), R_OK)) {
> -		if (perf_config_from_file(fn, perf_etc_perfconfig(), data) < 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(fn, user_config, data);
> -
> -out_free:
> -		free(user_config);
> -	}
> -out:
> -	return ret;
> -}
> -
>  static struct perf_config_section *find_section(struct list_head *sections,
>  						const char *section_name)
>  {
> @@ -706,6 +662,52 @@ struct perf_config_set *perf_config_set__new(void)
>  	return set;
>  }
>  
> +int perf_config(config_fn_t fn, void *data)
> +{
> +	int ret = 0;
> +	char key[BUFSIZ];
> +	struct perf_config_section *section;
> +	struct perf_config_item *item;
> +
> +	if (config_set == NULL)
> +		return -1;
> +
> +	config_set__for_each(config_set, section, item) {
> +		char *value = item->value;
> +
> +		if (value) {
> +			scnprintf(key, sizeof(key), "%s.%s",
> +				  section->name, item->name);
> +			ret = fn(key, value, data);
> +			if (ret < 0) {
> +				pr_err("Error: wrong config key-value pair %s=%s\n",
> +				       key, value);
> +				break;
> +			}
> +		}
> +	}
> +
> +	return ret;
> +}
> +
> +void perf_config_init(void)
> +{
> +	if (config_set == NULL)
> +		config_set = perf_config_set__new();
> +}
> +
> +void perf_config_finish(void)
> +{
> +	perf_config_set__delete(config_set);
> +	config_set = NULL;
> +}
> +
> +void perf_config_refresh(void)
> +{
> +	perf_config_finish();
> +	perf_config_init();
> +}
> +
>  static void perf_config_item__delete(struct perf_config_item *item)
>  {
>  	zfree(&item->name);
> diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h
> index 155a441..746c619 100644
> --- a/tools/perf/util/config.h
> +++ b/tools/perf/util/config.h
> @@ -33,5 +33,34 @@ 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);
> +void perf_config_init(void);
> +void perf_config_finish(void);
> +void perf_config_refresh(void);

Please use double _ to separate the subsystem/class from its methods,
i.e.:

void perf_config__init(void);
void perf_config__finish(void);
void perf_config__refresh(void);

> +
> +/**
> + * config_sections__for_each - iterate thru all the sections
> + * @list: list_head instance to iterate
> + * @section: struct perf_config_section iterator
> + */
> +#define config_sections__for_each(list, section)	\
> +        list_for_each_entry(section, list, node)

These macros operate on perf_config_sections, so please name it
accordingly, i.e.:
	perf_config_sections__for_each()

> +
> +/**
> + * config_items__for_each - iterate thru all the items
> + * @list: list_head instance to iterate
> + * @item: struct perf_config_item iterator
> + */
> +#define config_items__for_each(list, item)	\
> +        list_for_each_entry(item, list, node)
> +

Ditto

> +/**
> + * config_set__for_each - iterate thru all the config section-item pairs
> + * @set: evlist instance to iterate
> + * @section: struct perf_config_section iterator
> + * @item: struct perf_config_item iterator
> + */
> +#define config_set__for_each(set, section, item)			\
> +	config_sections__for_each(&set->sections, section)		\
> +	config_items__for_each(&section->items, item)

Ditto.
  
>  #endif /* __PERF_CONFIG_H */
> -- 
> 2.5.0

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v10 1/3] perf config: Bring declarations about config from util/cache.h to util/config.h
  2016-06-23  8:55 ` [PATCH v10 1/3] perf config: Bring declarations about config from util/cache.h to util/config.h Taeung Song
@ 2016-06-23 11:57   ` Arnaldo Carvalho de Melo
  2016-06-23 12:42     ` Taeung Song
  2016-06-26 10:55   ` [tip:perf/core] perf config: Move config declarations " tip-bot for Taeung Song
  1 sibling, 1 reply; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2016-06-23 11:57 UTC (permalink / raw)
  To: Taeung Song
  Cc: linux-kernel, Jiri Olsa, Namhyung Kim, Ingo Molnar,
	Peter Zijlstra, Alexander Shishkin, Masami Hiramatsu, Wang Nan,
	Jiri Olsa

Em Thu, Jun 23, 2016 at 05:55:17PM +0900, Taeung Song escreveu:
> Lately util/config.h has been added
> but util/cache.h has declarations of functions
> and a global variable for config features.
> 
> To manage codes about configuration at one spot,
> move them to util/config.h and let source files
> that need config features include config.h
> And if the source files that included previous cache.h
> need only config.h, remove including cache.h.

Applied and build tested.

- Arnaldo

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v10 2/3] perf config: Reimplement perf_config() introducing new perf_config_init() and perf_config_finish()
  2016-06-23 11:56   ` Arnaldo Carvalho de Melo
@ 2016-06-23 12:41     ` Taeung Song
  0 siblings, 0 replies; 9+ messages in thread
From: Taeung Song @ 2016-06-23 12:41 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Jiri Olsa, Namhyung Kim, Ingo Molnar,
	Peter Zijlstra, Alexander Shishkin, Masami Hiramatsu, Wang Nan,
	Jiri Olsa, Ingo Molnar

Hi, Arnaldo :)

On 06/23/2016 08:56 PM, Arnaldo Carvalho de Melo wrote:
> Em Thu, Jun 23, 2016 at 05:55:18PM +0900, Taeung Song escreveu:
>> Many sub-commands use perf_config() but
>> everytime perf_config() is called, perf_config() always read config files.
>> (i.e. user config '~/.perfconfig' and system config '$(sysconfdir)/perfconfig')
>>
>> But it is better to use the config set that already contains all config
>> key-value pairs to avoid this repetitive work reading the config files
>> in perf_config(). (the config set mean a static variable 'config_set')
>>
>> In other words, if new perf_config_init() is called,
>> only first time 'config_set' is initialized collecting all configs from the config files.
>> And then we could use new perf_config() like old perf_config().
>> When a sub-command finished, free the config set by perf_config_finish() at run_builtin().
>>
>> If we do, 'config_set' can be reused wherever perf_config() is called
>> and a feature of old perf_config() is the same as new perf_config() work
>> without the repetitive work that read the config files.
>>
>> In summary, in order to use features about configuration,
>> we can call the functions at perf.c and other source files as below.
>>
>>      # initialize a config set
>>      perf_config_init()
>>
>>      # configure actual variables from a config set
>>      perf_config()
>>
>>      # eliminate allocated config set
>>      perf_config_finish()
>>
>>      # destroy existing config set and initialize a new config set.
>>      perf_config_refresh()
>>
>> Cc: Namhyung Kim <namhyung@kernel.org>
>> Cc: Jiri Olsa <jolsa@redhat.com>
>> Cc: Wang Nan <wangnan0@huawei.com>
>> Cc: Peter Zijlstra <peterz@infradead.org>
>> Cc: Ingo Molnar <mingo@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 |  4 ++
>>   tools/perf/perf.c           |  2 +
>>   tools/perf/util/config.c    | 92 +++++++++++++++++++++++----------------------
>>   tools/perf/util/config.h    | 29 ++++++++++++++
>>   4 files changed, 82 insertions(+), 45 deletions(-)
>>
>> diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c
>> index fe1b77f..cfd1036 100644
>> --- a/tools/perf/builtin-config.c
>> +++ b/tools/perf/builtin-config.c
>> @@ -80,6 +80,10 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)
>>   	else if (use_user_config)
>>   		config_exclusive_filename = user_config;
>>
>> +	/*
>> +	 * At only 'config' sub-command, individually use the config set
>> +	 * because of reinitializing with options config file location.
>> +	 */
>>   	set = perf_config_set__new();
>>   	if (!set) {
>>   		ret = -1;
>> diff --git a/tools/perf/perf.c b/tools/perf/perf.c
>> index 66772da..280967e 100644
>> --- a/tools/perf/perf.c
>> +++ b/tools/perf/perf.c
>> @@ -355,6 +355,7 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
>>
>>   	perf_env__set_cmdline(&perf_env, argc, argv);
>>   	status = p->fn(argc, argv, prefix);
>> +	perf_config_finish();
>>   	exit_browser(status);
>>   	perf_env__exit(&perf_env);
>>   	bpf__clear();
>> @@ -522,6 +523,7 @@ int main(int argc, const char **argv)
>>
>>   	srandom(time(NULL));
>>
>> +	perf_config_init();
>>   	perf_config(perf_default_config, NULL);
>>   	set_buildid_dir(NULL);
>>
>> diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c
>> index d15c592..a16f95d 100644
>> --- a/tools/perf/util/config.c
>> +++ b/tools/perf/util/config.c
>> @@ -26,6 +26,7 @@ static FILE *config_file;
>>   static const char *config_file_name;
>>   static int config_linenr;
>>   static int config_file_eof;
>> +static struct perf_config_set *config_set;
>>
>>   const char *config_exclusive_filename;
>>
>> @@ -478,51 +479,6 @@ static int perf_config_global(void)
>>   	return !perf_env_bool("PERF_CONFIG_NOGLOBAL", 0);
>>   }
>>
>> -int perf_config(config_fn_t fn, void *data)
>> -{
>> -	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(fn, config_exclusive_filename, data);
>> -	if (perf_config_system() && !access(perf_etc_perfconfig(), R_OK)) {
>> -		if (perf_config_from_file(fn, perf_etc_perfconfig(), data) < 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(fn, user_config, data);
>> -
>> -out_free:
>> -		free(user_config);
>> -	}
>> -out:
>> -	return ret;
>> -}
>> -
>>   static struct perf_config_section *find_section(struct list_head *sections,
>>   						const char *section_name)
>>   {
>> @@ -706,6 +662,52 @@ struct perf_config_set *perf_config_set__new(void)
>>   	return set;
>>   }
>>
>> +int perf_config(config_fn_t fn, void *data)
>> +{
>> +	int ret = 0;
>> +	char key[BUFSIZ];
>> +	struct perf_config_section *section;
>> +	struct perf_config_item *item;
>> +
>> +	if (config_set == NULL)
>> +		return -1;
>> +
>> +	config_set__for_each(config_set, section, item) {
>> +		char *value = item->value;
>> +
>> +		if (value) {
>> +			scnprintf(key, sizeof(key), "%s.%s",
>> +				  section->name, item->name);
>> +			ret = fn(key, value, data);
>> +			if (ret < 0) {
>> +				pr_err("Error: wrong config key-value pair %s=%s\n",
>> +				       key, value);
>> +				break;
>> +			}
>> +		}
>> +	}
>> +
>> +	return ret;
>> +}
>> +
>> +void perf_config_init(void)
>> +{
>> +	if (config_set == NULL)
>> +		config_set = perf_config_set__new();
>> +}
>> +
>> +void perf_config_finish(void)
>> +{
>> +	perf_config_set__delete(config_set);
>> +	config_set = NULL;
>> +}
>> +
>> +void perf_config_refresh(void)
>> +{
>> +	perf_config_finish();
>> +	perf_config_init();
>> +}
>> +
>>   static void perf_config_item__delete(struct perf_config_item *item)
>>   {
>>   	zfree(&item->name);
>> diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h
>> index 155a441..746c619 100644
>> --- a/tools/perf/util/config.h
>> +++ b/tools/perf/util/config.h
>> @@ -33,5 +33,34 @@ 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);
>> +void perf_config_init(void);
>> +void perf_config_finish(void);
>> +void perf_config_refresh(void);
>
> Please use double _ to separate the subsystem/class from its methods,
> i.e.:
>
> void perf_config__init(void);
> void perf_config__finish(void);
> void perf_config__refresh(void);

I got it :)

>> +
>> +/**
>> + * config_sections__for_each - iterate thru all the sections
>> + * @list: list_head instance to iterate
>> + * @section: struct perf_config_section iterator
>> + */
>> +#define config_sections__for_each(list, section)	\
>> +        list_for_each_entry(section, list, node)
>
> These macros operate on perf_config_sections, so please name it
> accordingly, i.e.:
> 	perf_config_sections__for_each()
>
>> +
>> +/**
>> + * config_items__for_each - iterate thru all the items
>> + * @list: list_head instance to iterate
>> + * @item: struct perf_config_item iterator
>> + */
>> +#define config_items__for_each(list, item)	\
>> +        list_for_each_entry(item, list, node)
>> +
>
> Ditto
>
>> +/**
>> + * config_set__for_each - iterate thru all the config section-item pairs
>> + * @set: evlist instance to iterate
>> + * @section: struct perf_config_section iterator
>> + * @item: struct perf_config_item iterator
>> + */
>> +#define config_set__for_each(set, section, item)			\
>> +	config_sections__for_each(&set->sections, section)		\
>> +	config_items__for_each(&section->items, item)
>
> Ditto.
>

Granted.

I'll send v11 soon with this change!

Thanks,
Taeung

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v10 1/3] perf config: Bring declarations about config from util/cache.h to util/config.h
  2016-06-23 11:57   ` Arnaldo Carvalho de Melo
@ 2016-06-23 12:42     ` Taeung Song
  0 siblings, 0 replies; 9+ messages in thread
From: Taeung Song @ 2016-06-23 12:42 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: linux-kernel, Jiri Olsa, Namhyung Kim, Ingo Molnar,
	Peter Zijlstra, Alexander Shishkin, Masami Hiramatsu, Wang Nan,
	Jiri Olsa

Thank you !! :-D

On 06/23/2016 08:57 PM, Arnaldo Carvalho de Melo wrote:
> Em Thu, Jun 23, 2016 at 05:55:17PM +0900, Taeung Song escreveu:
>> Lately util/config.h has been added
>> but util/cache.h has declarations of functions
>> and a global variable for config features.
>>
>> To manage codes about configuration at one spot,
>> move them to util/config.h and let source files
>> that need config features include config.h
>> And if the source files that included previous cache.h
>> need only config.h, remove including cache.h.
>
> Applied and build tested.
>
> - Arnaldo
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [tip:perf/core] perf config: Move config declarations from util/cache.h to util/config.h
  2016-06-23  8:55 ` [PATCH v10 1/3] perf config: Bring declarations about config from util/cache.h to util/config.h Taeung Song
  2016-06-23 11:57   ` Arnaldo Carvalho de Melo
@ 2016-06-26 10:55   ` tip-bot for Taeung Song
  1 sibling, 0 replies; 9+ messages in thread
From: tip-bot for Taeung Song @ 2016-06-26 10:55 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: acme, mhiramat, hpa, treeze.taeung, alexander.shishkin, peterz,
	jolsa, tglx, namhyung, linux-kernel, wangnan0, mingo

Commit-ID:  41840d211c518e6af6e327b03e09323824e563bf
Gitweb:     http://git.kernel.org/tip/41840d211c518e6af6e327b03e09323824e563bf
Author:     Taeung Song <treeze.taeung@gmail.com>
AuthorDate: Thu, 23 Jun 2016 17:55:17 +0900
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Thu, 23 Jun 2016 08:51:41 -0300

perf config: Move config declarations from util/cache.h to util/config.h

Lately util/config.h has been added but util/cache.h has declarations of
functions and a global variable for config features.

To manage codes about configuration at one spot, move them to
util/config.h and let source files that need config features include
config.h And if the source files that included previous cache.h need
only config.h, remove including cache.h.

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>
Cc: Wang Nan <wangnan0@huawei.com>
Link: http://lkml.kernel.org/r/1466672119-4852-2-git-send-email-treeze.taeung@gmail.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-help.c          |  2 +-
 tools/perf/builtin-kmem.c          |  2 +-
 tools/perf/builtin-record.c        |  1 +
 tools/perf/builtin-report.c        |  2 +-
 tools/perf/builtin-top.c           |  2 +-
 tools/perf/perf.c                  |  2 +-
 tools/perf/ui/browser.c            |  2 +-
 tools/perf/ui/browsers/annotate.c  |  1 +
 tools/perf/util/alias.c            |  1 +
 tools/perf/util/cache.h            | 11 -----------
 tools/perf/util/color.c            |  1 +
 tools/perf/util/config.h           | 11 +++++++++++
 tools/perf/util/help-unknown-cmd.c |  1 +
 tools/perf/util/intel-pt.c         |  1 +
 tools/perf/util/llvm-utils.c       |  1 +
 15 files changed, 24 insertions(+), 17 deletions(-)

diff --git a/tools/perf/builtin-help.c b/tools/perf/builtin-help.c
index f9830c9..268ab73 100644
--- a/tools/perf/builtin-help.c
+++ b/tools/perf/builtin-help.c
@@ -4,7 +4,7 @@
  * Builtin help command
  */
 #include "perf.h"
-#include "util/cache.h"
+#include "util/config.h"
 #include "builtin.h"
 #include <subcmd/exec-cmd.h>
 #include "common-cmds.h"
diff --git a/tools/perf/builtin-kmem.c b/tools/perf/builtin-kmem.c
index 58adfee..4defe44 100644
--- a/tools/perf/builtin-kmem.c
+++ b/tools/perf/builtin-kmem.c
@@ -4,7 +4,7 @@
 #include "util/evlist.h"
 #include "util/evsel.h"
 #include "util/util.h"
-#include "util/cache.h"
+#include "util/config.h"
 #include "util/symbol.h"
 #include "util/thread.h"
 #include "util/header.h"
diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index b1304eb..c97b2b69 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -13,6 +13,7 @@
 #include "util/util.h"
 #include <subcmd/parse-options.h>
 #include "util/parse-events.h"
+#include "util/config.h"
 
 #include "util/callchain.h"
 #include "util/cgroup.h"
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 9f36b23..bcb49ff 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -8,7 +8,7 @@
 #include "builtin.h"
 
 #include "util/util.h"
-#include "util/cache.h"
+#include "util/config.h"
 
 #include "util/annotate.h"
 #include "util/color.h"
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 81dba80..ec4cba6 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -22,7 +22,7 @@
 #include "perf.h"
 
 #include "util/annotate.h"
-#include "util/cache.h"
+#include "util/config.h"
 #include "util/color.h"
 #include "util/evlist.h"
 #include "util/evsel.h"
diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index 634bf7c..66772da 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -10,7 +10,7 @@
 
 #include "util/env.h"
 #include <subcmd/exec-cmd.h>
-#include "util/cache.h"
+#include "util/config.h"
 #include "util/quote.h"
 #include <subcmd/run-command.h>
 #include "util/parse-events.h"
diff --git a/tools/perf/ui/browser.c b/tools/perf/ui/browser.c
index af68a9d..3eb3edb 100644
--- a/tools/perf/ui/browser.c
+++ b/tools/perf/ui/browser.c
@@ -1,5 +1,5 @@
 #include "../util.h"
-#include "../cache.h"
+#include "../config.h"
 #include "../../perf.h"
 #include "libslang.h"
 #include "ui.h"
diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index 4fc208e..0e106bb 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -8,6 +8,7 @@
 #include "../../util/sort.h"
 #include "../../util/symbol.h"
 #include "../../util/evsel.h"
+#include "../../util/config.h"
 #include <pthread.h>
 
 struct disasm_line_samples {
diff --git a/tools/perf/util/alias.c b/tools/perf/util/alias.c
index c0b43ee..6c80f83 100644
--- a/tools/perf/util/alias.c
+++ b/tools/perf/util/alias.c
@@ -1,4 +1,5 @@
 #include "cache.h"
+#include "config.h"
 
 static const char *alias_key;
 static char *alias_val;
diff --git a/tools/perf/util/cache.h b/tools/perf/util/cache.h
index 369f382..9f90e36 100644
--- a/tools/perf/util/cache.h
+++ b/tools/perf/util/cache.h
@@ -18,17 +18,6 @@
 #define PERF_TRACEFS_ENVIRONMENT "PERF_TRACEFS_DIR"
 #define PERF_PAGER_ENVIRONMENT "PERF_PAGER"
 
-extern const char *config_exclusive_filename;
-
-typedef int (*config_fn_t)(const char *, const char *, void *);
-int perf_default_config(const char *, const char *, void *);
-int perf_config(config_fn_t fn, void *);
-int perf_config_int(const char *, const char *);
-u64 perf_config_u64(const char *, const char *);
-int perf_config_bool(const char *, const char *);
-int config_error_nonbool(const char *);
-const char *perf_etc_perfconfig(void);
-
 char *alias_lookup(const char *alias);
 int split_cmdline(char *cmdline, const char ***argv);
 
diff --git a/tools/perf/util/color.c b/tools/perf/util/color.c
index 43e84aa..1210ba5 100644
--- a/tools/perf/util/color.c
+++ b/tools/perf/util/color.c
@@ -1,5 +1,6 @@
 #include <linux/kernel.h>
 #include "cache.h"
+#include "config.h"
 #include "color.h"
 #include <math.h>
 
diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h
index 22ec626..155a441 100644
--- a/tools/perf/util/config.h
+++ b/tools/perf/util/config.h
@@ -20,6 +20,17 @@ struct perf_config_set {
 	struct list_head sections;
 };
 
+extern const char *config_exclusive_filename;
+
+typedef int (*config_fn_t)(const char *, const char *, void *);
+int perf_default_config(const char *, const char *, void *);
+int perf_config(config_fn_t fn, void *);
+int perf_config_int(const char *, const char *);
+u64 perf_config_u64(const char *, const char *);
+int perf_config_bool(const char *, const char *);
+int config_error_nonbool(const char *);
+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);
 
diff --git a/tools/perf/util/help-unknown-cmd.c b/tools/perf/util/help-unknown-cmd.c
index d62ccae..776e285 100644
--- a/tools/perf/util/help-unknown-cmd.c
+++ b/tools/perf/util/help-unknown-cmd.c
@@ -1,4 +1,5 @@
 #include "cache.h"
+#include "config.h"
 #include <subcmd/help.h>
 #include "../builtin.h"
 #include "levenshtein.h"
diff --git a/tools/perf/util/intel-pt.c b/tools/perf/util/intel-pt.c
index 1371969..a2fe3a2 100644
--- a/tools/perf/util/intel-pt.c
+++ b/tools/perf/util/intel-pt.c
@@ -39,6 +39,7 @@
 #include "auxtrace.h"
 #include "tsc.h"
 #include "intel-pt.h"
+#include "config.h"
 
 #include "intel-pt-decoder/intel-pt-log.h"
 #include "intel-pt-decoder/intel-pt-decoder.h"
diff --git a/tools/perf/util/llvm-utils.c b/tools/perf/util/llvm-utils.c
index 878a566..40b6f72 100644
--- a/tools/perf/util/llvm-utils.c
+++ b/tools/perf/util/llvm-utils.c
@@ -8,6 +8,7 @@
 #include <stdlib.h>
 #include "debug.h"
 #include "llvm-utils.h"
+#include "config.h"
 
 #define CLANG_BPF_CMD_DEFAULT_TEMPLATE				\
 		"$CLANG_EXEC -D__KERNEL__ -D__NR_CPUS__=$NR_CPUS "\

^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2016-06-26 10:55 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-23  8:55 [PATCH v10 0/3] perf config: Reimplement perf_config() Taeung Song
2016-06-23  8:55 ` [PATCH v10 1/3] perf config: Bring declarations about config from util/cache.h to util/config.h Taeung Song
2016-06-23 11:57   ` Arnaldo Carvalho de Melo
2016-06-23 12:42     ` Taeung Song
2016-06-26 10:55   ` [tip:perf/core] perf config: Move config declarations " tip-bot for Taeung Song
2016-06-23  8:55 ` [PATCH v10 2/3] perf config: Reimplement perf_config() introducing new perf_config_init() and perf_config_finish() Taeung Song
2016-06-23 11:56   ` Arnaldo Carvalho de Melo
2016-06-23 12:41     ` Taeung Song
2016-06-23  8:55 ` [PATCH v10 3/3] perf config: Reimplement show_config() using config_set__for_each Taeung Song

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).