* [RFC][PATCH v2 0/5] perf config: Introduce perf_config_set class @ 2016-03-14 12:16 Taeung Song 2016-03-14 12:16 ` [PATCH v2 1/5] " Taeung Song ` (4 more replies) 0 siblings, 5 replies; 10+ messages in thread From: Taeung Song @ 2016-03-14 12:16 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: linux-kernel, Jiri Olsa, Namhyung Kim, Ingo Molnar, Peter Zijlstra, Taeung Song Hi, all :-) We can use the config files (i.e user wide ~/.perfconfig and system wide $(sysconfdir)/perfconfig) to configure perf tools. perf-config help user manage the config files, not manually look into or edit them. Introduce new infrastructure code for config management features of perf-config subcommand. This pathset is for various purposes of configuration management showing current configs, in the near future, showing all configs with default value, getting current configs from the config files or writing configs that user type on the config files, etc. IMHO, I want to add new effective funcationalities for config management of perf-config based on this infrastructure code. Thanks, Taeung v2: - remove perf_config_kind (user, system or both config files) and needless at this time, etc. (Namhyung) - separate this patch as several patches (Namhyung) - fix typing errors, etc. Taeung Song (5): perf config: Introduce perf_config_set class perf config: Let show_config() work with perf_config_set perf config: Prepare all default configs perf config: Initialize perf_config_set with all default configs perf config: Add 'list-all' option to show all perf's configs tools/perf/Documentation/perf-config.txt | 6 + tools/perf/builtin-config.c | 102 +++++++++++++++-- tools/perf/util/config.c | 186 +++++++++++++++++++++++++++++++ tools/perf/util/config.h | 112 +++++++++++++++++++ 4 files changed, 399 insertions(+), 7 deletions(-) create mode 100644 tools/perf/util/config.h -- 2.5.0 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 1/5] perf config: Introduce perf_config_set class 2016-03-14 12:16 [RFC][PATCH v2 0/5] perf config: Introduce perf_config_set class Taeung Song @ 2016-03-14 12:16 ` Taeung Song 2016-03-17 12:31 ` Namhyung Kim 2016-03-14 12:16 ` [PATCH v2 2/5] perf config: Let show_config() work with perf_config_set Taeung Song ` (3 subsequent siblings) 4 siblings, 1 reply; 10+ messages in thread From: Taeung Song @ 2016-03-14 12:16 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: linux-kernel, Jiri Olsa, Namhyung Kim, Ingo Molnar, Peter Zijlstra, Taeung Song This infrastructure code was designed for upcoming features of perf-config. That collect config key-value pairs from user and system config files (i.e. user wide ~/.perfconfig and system wide $(sysconfdir)/perfconfig) to manage perf's configs. Cc: Namhyung Kim <namhyung@kernel.org> Cc: Jiri Olsa <jolsa@kernel.org> Signed-off-by: Taeung Song <treeze.taeung@gmail.com> --- tools/perf/builtin-config.c | 1 + tools/perf/util/config.c | 123 ++++++++++++++++++++++++++++++++++++++++++++ tools/perf/util/config.h | 21 ++++++++ 3 files changed, 145 insertions(+) create mode 100644 tools/perf/util/config.h diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c index c42448e..412c725 100644 --- a/tools/perf/builtin-config.c +++ b/tools/perf/builtin-config.c @@ -12,6 +12,7 @@ #include <subcmd/parse-options.h> #include "util/util.h" #include "util/debug.h" +#include "util/config.h" static bool use_system_config, use_user_config; diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c index 4e72763..b9660e4 100644 --- a/tools/perf/util/config.c +++ b/tools/perf/util/config.c @@ -13,6 +13,7 @@ #include <subcmd/exec-cmd.h> #include "util/hist.h" /* perf_hist_config */ #include "util/llvm-utils.h" /* perf_llvm_config */ +#include "config.h" #define MAXNAME (256) @@ -506,6 +507,128 @@ out: return ret; } +static struct perf_config_item *find_config(struct list_head *config_list, + const char *section, + const char *name) +{ + struct perf_config_item *config; + + list_for_each_entry(config, config_list, list) { + if (!strcmp(config->section, section) && + !strcmp(config->name, name)) + return config; + } + + return NULL; +} + +static struct perf_config_item *add_config(struct list_head *config_list, + const char *section, + const char *name) +{ + struct perf_config_item *config = zalloc(sizeof(*config)); + + if (!config) + return NULL; + + config->section = strdup(section); + if (!section) + goto out_err; + + config->name = strdup(name); + if (!name) { + free((char *)config->section); + goto out_err; + } + + list_add_tail(&config->list, config_list); + return config; + +out_err: + free(config); + pr_err("%s: strdup failed\n", __func__); + return NULL; +} + +static int set_value(struct perf_config_item *config, const char *value) +{ + char *val = strdup(value); + + if (!val) + return -1; + config->value = val; + + return 0; +} + +static int collect_config(const char *var, const char *value, + void *configs) +{ + int ret = 0; + char *ptr, *key; + char *section, *name; + struct perf_config_item *config; + struct list_head *config_list = configs; + + key = ptr = strdup(var); + if (!key) { + pr_err("%s: strdup failed\n", __func__); + return -1; + } + + section = strsep(&ptr, "."); + name = ptr; + if (name == NULL || value == NULL) { + ret = -1; + goto out_free; + } + + config = find_config(config_list, section, name); + if (!config) { + config = add_config(config_list, section, name); + if (!config) { + free(config->section); + free(config->name); + ret = -1; + goto out_free; + } + } + + ret = set_value(config, value); + +out_free: + free(key); + return ret; +} + +struct perf_config_set *perf_config_set__new(void) +{ + struct perf_config_set *perf_configs = zalloc(sizeof(*perf_configs)); + + if (!perf_configs) + return NULL; + + INIT_LIST_HEAD(&perf_configs->config_list); + perf_config(collect_config, &perf_configs->config_list); + + return perf_configs; +} + +void perf_config_set__delete(struct perf_config_set *perf_configs) +{ + struct perf_config_item *pos, *item; + + list_for_each_entry_safe(pos, item, &perf_configs->config_list, list) { + list_del(&pos->list); + free(pos->section); + free(pos->name); + free(pos->value); + free(pos); + } + + free(perf_configs); +} + /* * Call this to report error for your variable that should not * get a boolean value (i.e. "[my] var" means "true"). diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h new file mode 100644 index 0000000..44c226f --- /dev/null +++ b/tools/perf/util/config.h @@ -0,0 +1,21 @@ +#ifndef __PERF_CONFIG_H +#define __PERF_CONFIG_H + +#include <stdbool.h> +#include <linux/list.h> + +struct perf_config_item { + char *section; + char *name; + char *value; + struct list_head list; +}; + +struct perf_config_set { + struct list_head config_list; +}; + +struct perf_config_set *perf_config_set__new(void); +void perf_config_set__delete(struct perf_config_set *perf_configs); + +#endif /* __PERF_CONFIG_H */ -- 2.5.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/5] perf config: Introduce perf_config_set class 2016-03-14 12:16 ` [PATCH v2 1/5] " Taeung Song @ 2016-03-17 12:31 ` Namhyung Kim 2016-03-17 14:10 ` Taeung Song 0 siblings, 1 reply; 10+ messages in thread From: Namhyung Kim @ 2016-03-17 12:31 UTC (permalink / raw) To: Taeung Song Cc: Arnaldo Carvalho de Melo, linux-kernel, Jiri Olsa, Ingo Molnar, Peter Zijlstra Hi Taeung, On Mon, Mar 14, 2016 at 09:16:05PM +0900, Taeung Song wrote: > This infrastructure code was designed for > upcoming features of perf-config. > > That collect config key-value pairs from user and > system config files (i.e. user wide ~/.perfconfig > and system wide $(sysconfdir)/perfconfig) > to manage perf's configs. > > Cc: Namhyung Kim <namhyung@kernel.org> > Cc: Jiri Olsa <jolsa@kernel.org> > Signed-off-by: Taeung Song <treeze.taeung@gmail.com> > --- > tools/perf/builtin-config.c | 1 + > tools/perf/util/config.c | 123 ++++++++++++++++++++++++++++++++++++++++++++ > tools/perf/util/config.h | 21 ++++++++ > 3 files changed, 145 insertions(+) > create mode 100644 tools/perf/util/config.h > > diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c > index c42448e..412c725 100644 > --- a/tools/perf/builtin-config.c > +++ b/tools/perf/builtin-config.c > @@ -12,6 +12,7 @@ > #include <subcmd/parse-options.h> > #include "util/util.h" > #include "util/debug.h" > +#include "util/config.h" > > static bool use_system_config, use_user_config; > > diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c > index 4e72763..b9660e4 100644 > --- a/tools/perf/util/config.c > +++ b/tools/perf/util/config.c > @@ -13,6 +13,7 @@ > #include <subcmd/exec-cmd.h> > #include "util/hist.h" /* perf_hist_config */ > #include "util/llvm-utils.h" /* perf_llvm_config */ > +#include "config.h" > > #define MAXNAME (256) > > @@ -506,6 +507,128 @@ out: > return ret; > } > > +static struct perf_config_item *find_config(struct list_head *config_list, > + const char *section, > + const char *name) > +{ > + struct perf_config_item *config; > + > + list_for_each_entry(config, config_list, list) { > + if (!strcmp(config->section, section) && > + !strcmp(config->name, name)) > + return config; > + } Hmm.. why do you remove the section list? > + > + return NULL; > +} > + > +static struct perf_config_item *add_config(struct list_head *config_list, > + const char *section, > + const char *name) > +{ > + struct perf_config_item *config = zalloc(sizeof(*config)); > + > + if (!config) > + return NULL; > + > + config->section = strdup(section); > + if (!section) > + goto out_err; > + > + config->name = strdup(name); > + if (!name) { > + free((char *)config->section); > + goto out_err; > + } > + > + list_add_tail(&config->list, config_list); > + return config; > + > +out_err: > + free(config); > + pr_err("%s: strdup failed\n", __func__); > + return NULL; > +} > + > +static int set_value(struct perf_config_item *config, const char *value) > +{ > + char *val = strdup(value); > + > + if (!val) > + return -1; > + config->value = val; It seems to overwrite old value.. Thanks, Namhyung > + > + return 0; > +} > + > +static int collect_config(const char *var, const char *value, > + void *configs) > +{ > + int ret = 0; > + char *ptr, *key; > + char *section, *name; > + struct perf_config_item *config; > + struct list_head *config_list = configs; > + > + key = ptr = strdup(var); > + if (!key) { > + pr_err("%s: strdup failed\n", __func__); > + return -1; > + } > + > + section = strsep(&ptr, "."); > + name = ptr; > + if (name == NULL || value == NULL) { > + ret = -1; > + goto out_free; > + } > + > + config = find_config(config_list, section, name); > + if (!config) { > + config = add_config(config_list, section, name); > + if (!config) { > + free(config->section); > + free(config->name); > + ret = -1; > + goto out_free; > + } > + } > + > + ret = set_value(config, value); > + > +out_free: > + free(key); > + return ret; > +} > + > +struct perf_config_set *perf_config_set__new(void) > +{ > + struct perf_config_set *perf_configs = zalloc(sizeof(*perf_configs)); > + > + if (!perf_configs) > + return NULL; > + > + INIT_LIST_HEAD(&perf_configs->config_list); > + perf_config(collect_config, &perf_configs->config_list); > + > + return perf_configs; > +} > + > +void perf_config_set__delete(struct perf_config_set *perf_configs) > +{ > + struct perf_config_item *pos, *item; > + > + list_for_each_entry_safe(pos, item, &perf_configs->config_list, list) { > + list_del(&pos->list); > + free(pos->section); > + free(pos->name); > + free(pos->value); > + free(pos); > + } > + > + free(perf_configs); > +} > + > /* > * Call this to report error for your variable that should not > * get a boolean value (i.e. "[my] var" means "true"). > diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h > new file mode 100644 > index 0000000..44c226f > --- /dev/null > +++ b/tools/perf/util/config.h > @@ -0,0 +1,21 @@ > +#ifndef __PERF_CONFIG_H > +#define __PERF_CONFIG_H > + > +#include <stdbool.h> > +#include <linux/list.h> > + > +struct perf_config_item { > + char *section; > + char *name; > + char *value; > + struct list_head list; > +}; > + > +struct perf_config_set { > + struct list_head config_list; > +}; > + > +struct perf_config_set *perf_config_set__new(void); > +void perf_config_set__delete(struct perf_config_set *perf_configs); > + > +#endif /* __PERF_CONFIG_H */ > -- > 2.5.0 > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/5] perf config: Introduce perf_config_set class 2016-03-17 12:31 ` Namhyung Kim @ 2016-03-17 14:10 ` Taeung Song 2016-03-17 23:27 ` Namhyung Kim 0 siblings, 1 reply; 10+ messages in thread From: Taeung Song @ 2016-03-17 14:10 UTC (permalink / raw) To: Namhyung Kim Cc: Arnaldo Carvalho de Melo, linux-kernel, Jiri Olsa, Ingo Molnar, Peter Zijlstra Hi, Namhyung On 03/17/2016 09:31 PM, Namhyung Kim wrote: > Hi Taeung, > > On Mon, Mar 14, 2016 at 09:16:05PM +0900, Taeung Song wrote: >> This infrastructure code was designed for >> upcoming features of perf-config. >> >> That collect config key-value pairs from user and >> system config files (i.e. user wide ~/.perfconfig >> and system wide $(sysconfdir)/perfconfig) >> to manage perf's configs. >> >> Cc: Namhyung Kim <namhyung@kernel.org> >> Cc: Jiri Olsa <jolsa@kernel.org> >> Signed-off-by: Taeung Song <treeze.taeung@gmail.com> >> --- >> tools/perf/builtin-config.c | 1 + >> tools/perf/util/config.c | 123 ++++++++++++++++++++++++++++++++++++++++++++ >> tools/perf/util/config.h | 21 ++++++++ >> 3 files changed, 145 insertions(+) >> create mode 100644 tools/perf/util/config.h >> >> diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c >> index c42448e..412c725 100644 >> --- a/tools/perf/builtin-config.c >> +++ b/tools/perf/builtin-config.c >> @@ -12,6 +12,7 @@ >> #include <subcmd/parse-options.h> >> #include "util/util.h" >> #include "util/debug.h" >> +#include "util/config.h" >> >> static bool use_system_config, use_user_config; >> >> diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c >> index 4e72763..b9660e4 100644 >> --- a/tools/perf/util/config.c >> +++ b/tools/perf/util/config.c >> @@ -13,6 +13,7 @@ >> #include <subcmd/exec-cmd.h> >> #include "util/hist.h" /* perf_hist_config */ >> #include "util/llvm-utils.h" /* perf_llvm_config */ >> +#include "config.h" >> >> #define MAXNAME (256) >> >> @@ -506,6 +507,128 @@ out: >> return ret; >> } >> >> +static struct perf_config_item *find_config(struct list_head *config_list, >> + const char *section, >> + const char *name) >> +{ >> + struct perf_config_item *config; >> + >> + list_for_each_entry(config, config_list, list) { >> + if (!strcmp(config->section, section) && >> + !strcmp(config->name, name)) >> + return config; >> + } > > Hmm.. why do you remove the section list? > IMHO, there are several reasons 1) To use only one list (default config, custom config(user/system)) 1-1) I used two list that were 'list_head sections' and 'config_item default_configs[]'. So if checking type of config variable, two for-loop must be needed for each list. Because two structure was different i.e. 'sections' list mean config_section list that each section contain config_element list. (there wasn't telling about correct type of 'value' instead of string(char *)) struct config_element { char *name; char *value; struct list_head list; }; struct config_section { char *name; struct list_head element_head; struct list_head list; }; 'struct config_item default_configs[]' mean all default configs. struct config_item { const char *section; const char *name; union { bool b; int i; u32 l; u64 ll; float f; double d; const char *s; } value; enum config_type type; const char *desc; }; IMHO, I think this is a bit complex and I want to simplify the perf's config list on perf-config. 2) A small number of perf's configs I think perf's configs aren't too many so I think two structure for section and element aren't needed. 3) A object for a config variable need to have enough info for itself This is a bit similar to 1) reason. If using only 'struct config_item' for the config list, it can contain section name, name, values(default, user config, system config, both config), correct type, etc. If we do, we needn't to find detail for a config variable at other objects e.g. When we find correct type of a config variable, we needn't to do for-loop for default_configs[] in order to know the type. I think this is better than old two structure. >> + >> + return NULL; >> +} >> + >> +static struct perf_config_item *add_config(struct list_head *config_list, >> + const char *section, >> + const char *name) >> +{ >> + struct perf_config_item *config = zalloc(sizeof(*config)); >> + >> + if (!config) >> + return NULL; >> + >> + config->section = strdup(section); >> + if (!section) >> + goto out_err; >> + >> + config->name = strdup(name); >> + if (!name) { >> + free((char *)config->section); >> + goto out_err; >> + } >> + >> + list_add_tail(&config->list, config_list); >> + return config; >> + >> +out_err: >> + free(config); >> + pr_err("%s: strdup failed\n", __func__); >> + return NULL; >> +} >> + >> +static int set_value(struct perf_config_item *config, const char *value) >> +{ >> + char *val = strdup(value); >> + >> + if (!val) >> + return -1; >> + config->value = val; > > It seems to overwrite old value.. > Yes, I know it. If don't using '--user' or '--system', there isn't exclusive config file path then have to read both config files. But because user config file has a high order of priority, if two config file has same variable, old value(for system config) must be overwrote by new value(for user config). Thanks, Taeung > > >> + >> + return 0; >> +} >> + >> +static int collect_config(const char *var, const char *value, >> + void *configs) >> +{ >> + int ret = 0; >> + char *ptr, *key; >> + char *section, *name; >> + struct perf_config_item *config; >> + struct list_head *config_list = configs; >> + >> + key = ptr = strdup(var); >> + if (!key) { >> + pr_err("%s: strdup failed\n", __func__); >> + return -1; >> + } >> + >> + section = strsep(&ptr, "."); >> + name = ptr; >> + if (name == NULL || value == NULL) { >> + ret = -1; >> + goto out_free; >> + } >> + >> + config = find_config(config_list, section, name); >> + if (!config) { >> + config = add_config(config_list, section, name); >> + if (!config) { >> + free(config->section); >> + free(config->name); >> + ret = -1; >> + goto out_free; >> + } >> + } >> + >> + ret = set_value(config, value); >> + >> +out_free: >> + free(key); >> + return ret; >> +} >> + >> +struct perf_config_set *perf_config_set__new(void) >> +{ >> + struct perf_config_set *perf_configs = zalloc(sizeof(*perf_configs)); >> + >> + if (!perf_configs) >> + return NULL; >> + >> + INIT_LIST_HEAD(&perf_configs->config_list); >> + perf_config(collect_config, &perf_configs->config_list); >> + >> + return perf_configs; >> +} >> + >> +void perf_config_set__delete(struct perf_config_set *perf_configs) >> +{ >> + struct perf_config_item *pos, *item; >> + >> + list_for_each_entry_safe(pos, item, &perf_configs->config_list, list) { >> + list_del(&pos->list); >> + free(pos->section); >> + free(pos->name); >> + free(pos->value); >> + free(pos); >> + } >> + >> + free(perf_configs); >> +} >> + >> /* >> * Call this to report error for your variable that should not >> * get a boolean value (i.e. "[my] var" means "true"). >> diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h >> new file mode 100644 >> index 0000000..44c226f >> --- /dev/null >> +++ b/tools/perf/util/config.h >> @@ -0,0 +1,21 @@ >> +#ifndef __PERF_CONFIG_H >> +#define __PERF_CONFIG_H >> + >> +#include <stdbool.h> >> +#include <linux/list.h> >> + >> +struct perf_config_item { >> + char *section; >> + char *name; >> + char *value; >> + struct list_head list; >> +}; >> + >> +struct perf_config_set { >> + struct list_head config_list; >> +}; >> + >> +struct perf_config_set *perf_config_set__new(void); >> +void perf_config_set__delete(struct perf_config_set *perf_configs); >> + >> +#endif /* __PERF_CONFIG_H */ >> -- >> 2.5.0 >> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/5] perf config: Introduce perf_config_set class 2016-03-17 14:10 ` Taeung Song @ 2016-03-17 23:27 ` Namhyung Kim 2016-03-18 0:52 ` Taeung Song 0 siblings, 1 reply; 10+ messages in thread From: Namhyung Kim @ 2016-03-17 23:27 UTC (permalink / raw) To: Taeung Song Cc: Arnaldo Carvalho de Melo, linux-kernel, Jiri Olsa, Ingo Molnar, Peter Zijlstra On Thu, Mar 17, 2016 at 11:10:12PM +0900, Taeung Song wrote: > Hi, Namhyung > > On 03/17/2016 09:31 PM, Namhyung Kim wrote: > >Hi Taeung, > > > >On Mon, Mar 14, 2016 at 09:16:05PM +0900, Taeung Song wrote: > >>This infrastructure code was designed for > >>upcoming features of perf-config. > >> > >>That collect config key-value pairs from user and > >>system config files (i.e. user wide ~/.perfconfig > >>and system wide $(sysconfdir)/perfconfig) > >>to manage perf's configs. > >> > >>Cc: Namhyung Kim <namhyung@kernel.org> > >>Cc: Jiri Olsa <jolsa@kernel.org> > >>Signed-off-by: Taeung Song <treeze.taeung@gmail.com> > >>--- > >> tools/perf/builtin-config.c | 1 + > >> tools/perf/util/config.c | 123 ++++++++++++++++++++++++++++++++++++++++++++ > >> tools/perf/util/config.h | 21 ++++++++ > >> 3 files changed, 145 insertions(+) > >> create mode 100644 tools/perf/util/config.h > >> > >>diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c > >>index c42448e..412c725 100644 > >>--- a/tools/perf/builtin-config.c > >>+++ b/tools/perf/builtin-config.c > >>@@ -12,6 +12,7 @@ > >> #include <subcmd/parse-options.h> > >> #include "util/util.h" > >> #include "util/debug.h" > >>+#include "util/config.h" > >> > >> static bool use_system_config, use_user_config; > >> > >>diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c > >>index 4e72763..b9660e4 100644 > >>--- a/tools/perf/util/config.c > >>+++ b/tools/perf/util/config.c > >>@@ -13,6 +13,7 @@ > >> #include <subcmd/exec-cmd.h> > >> #include "util/hist.h" /* perf_hist_config */ > >> #include "util/llvm-utils.h" /* perf_llvm_config */ > >>+#include "config.h" > >> > >> #define MAXNAME (256) > >> > >>@@ -506,6 +507,128 @@ out: > >> return ret; > >> } > >> > >>+static struct perf_config_item *find_config(struct list_head *config_list, > >>+ const char *section, > >>+ const char *name) > >>+{ > >>+ struct perf_config_item *config; > >>+ > >>+ list_for_each_entry(config, config_list, list) { > >>+ if (!strcmp(config->section, section) && > >>+ !strcmp(config->name, name)) > >>+ return config; > >>+ } > > > >Hmm.. why do you remove the section list? > > > > IMHO, there are several reasons > > 1) To use only one list (default config, custom config(user/system)) > > 1-1) I used two list that were 'list_head sections' > and 'config_item default_configs[]'. So if checking > type of config variable, two for-loop must be needed > for each list. Because two structure was different i.e. > > 'sections' list mean config_section list > that each section contain config_element list. > (there wasn't telling about correct type of 'value' instead of string(char > *)) > > struct config_element { > char *name; > char *value; > struct list_head list; > }; > > struct config_section { > char *name; > struct list_head element_head; > struct list_head list; > }; > > 'struct config_item default_configs[]' mean all default configs. > > struct config_item { > const char *section; > const char *name; > union { > bool b; > int i; > u32 l; > u64 ll; > float f; > double d; > const char *s; > } value; > enum config_type type; > const char *desc; > }; > > > IMHO, I think this is a bit complex > and I want to simplify the perf's config list on perf-config. > > 2) A small number of perf's configs > > I think perf's configs aren't too many so I think > two structure for section and element aren't needed. OK. > > 3) A object for a config variable need to have enough info for itself > > This is a bit similar to 1) reason. > If using only 'struct config_item' for the config list, > it can contain section name, name, values(default, user config, > system config, both config), correct type, etc. > > If we do, we needn't to find detail for a config variable at other objects > e.g. > When we find correct type of a config variable, > we needn't to do for-loop for default_configs[] in order to know the > type. I'm not sure I understand you correctly, but I think this is not related to the two-level structure. > > > I think this is better than old two structure. > > >>+ > >>+ return NULL; > >>+} > >>+ > >>+static struct perf_config_item *add_config(struct list_head *config_list, > >>+ const char *section, > >>+ const char *name) > >>+{ > >>+ struct perf_config_item *config = zalloc(sizeof(*config)); > >>+ > >>+ if (!config) > >>+ return NULL; > >>+ > >>+ config->section = strdup(section); > >>+ if (!section) > >>+ goto out_err; > >>+ > >>+ config->name = strdup(name); > >>+ if (!name) { > >>+ free((char *)config->section); > >>+ goto out_err; > >>+ } > >>+ > >>+ list_add_tail(&config->list, config_list); > >>+ return config; > >>+ > >>+out_err: > >>+ free(config); > >>+ pr_err("%s: strdup failed\n", __func__); > >>+ return NULL; > >>+} > >>+ > >>+static int set_value(struct perf_config_item *config, const char *value) > >>+{ > >>+ char *val = strdup(value); > >>+ > >>+ if (!val) > >>+ return -1; > >>+ config->value = val; > > > >It seems to overwrite old value.. > > > > Yes, I know it. > If don't using '--user' or '--system', > there isn't exclusive config file path > then have to read both config files. > > But because user config file has a high order of priority, > if two config file has same variable, old value(for system config) > must be overwrote by new value(for user config). But shouldn't it free the old value before overwriting? Thanks, Namhyung > > > > > > >>+ > >>+ return 0; > >>+} ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2 1/5] perf config: Introduce perf_config_set class 2016-03-17 23:27 ` Namhyung Kim @ 2016-03-18 0:52 ` Taeung Song 0 siblings, 0 replies; 10+ messages in thread From: Taeung Song @ 2016-03-18 0:52 UTC (permalink / raw) To: Namhyung Kim Cc: Arnaldo Carvalho de Melo, linux-kernel, Jiri Olsa, Ingo Molnar, Peter Zijlstra On 03/18/2016 08:27 AM, Namhyung Kim wrote: > On Thu, Mar 17, 2016 at 11:10:12PM +0900, Taeung Song wrote: >> Hi, Namhyung >> >> On 03/17/2016 09:31 PM, Namhyung Kim wrote: >>> Hi Taeung, >>> >>> On Mon, Mar 14, 2016 at 09:16:05PM +0900, Taeung Song wrote: >>>> This infrastructure code was designed for >>>> upcoming features of perf-config. >>>> >>>> That collect config key-value pairs from user and >>>> system config files (i.e. user wide ~/.perfconfig >>>> and system wide $(sysconfdir)/perfconfig) >>>> to manage perf's configs. >>>> >>>> Cc: Namhyung Kim <namhyung@kernel.org> >>>> Cc: Jiri Olsa <jolsa@kernel.org> >>>> Signed-off-by: Taeung Song <treeze.taeung@gmail.com> >>>> --- >>>> tools/perf/builtin-config.c | 1 + >>>> tools/perf/util/config.c | 123 ++++++++++++++++++++++++++++++++++++++++++++ >>>> tools/perf/util/config.h | 21 ++++++++ >>>> 3 files changed, 145 insertions(+) >>>> create mode 100644 tools/perf/util/config.h >>>> >>>> diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c >>>> index c42448e..412c725 100644 >>>> --- a/tools/perf/builtin-config.c >>>> +++ b/tools/perf/builtin-config.c >>>> @@ -12,6 +12,7 @@ >>>> #include <subcmd/parse-options.h> >>>> #include "util/util.h" >>>> #include "util/debug.h" >>>> +#include "util/config.h" >>>> >>>> static bool use_system_config, use_user_config; >>>> >>>> diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c >>>> index 4e72763..b9660e4 100644 >>>> --- a/tools/perf/util/config.c >>>> +++ b/tools/perf/util/config.c >>>> @@ -13,6 +13,7 @@ >>>> #include <subcmd/exec-cmd.h> >>>> #include "util/hist.h" /* perf_hist_config */ >>>> #include "util/llvm-utils.h" /* perf_llvm_config */ >>>> +#include "config.h" >>>> >>>> #define MAXNAME (256) >>>> >>>> @@ -506,6 +507,128 @@ out: >>>> return ret; >>>> } >>>> >>>> +static struct perf_config_item *find_config(struct list_head *config_list, >>>> + const char *section, >>>> + const char *name) >>>> +{ >>>> + struct perf_config_item *config; >>>> + >>>> + list_for_each_entry(config, config_list, list) { >>>> + if (!strcmp(config->section, section) && >>>> + !strcmp(config->name, name)) >>>> + return config; >>>> + } >>> >>> Hmm.. why do you remove the section list? >>> >> >> IMHO, there are several reasons >> >> 1) To use only one list (default config, custom config(user/system)) >> >> 1-1) I used two list that were 'list_head sections' >> and 'config_item default_configs[]'. So if checking >> type of config variable, two for-loop must be needed >> for each list. Because two structure was different i.e. >> >> 'sections' list mean config_section list >> that each section contain config_element list. >> (there wasn't telling about correct type of 'value' instead of string(char >> *)) >> >> struct config_element { >> char *name; >> char *value; >> struct list_head list; >> }; >> >> struct config_section { >> char *name; >> struct list_head element_head; >> struct list_head list; >> }; >> >> 'struct config_item default_configs[]' mean all default configs. >> >> struct config_item { >> const char *section; >> const char *name; >> union { >> bool b; >> int i; >> u32 l; >> u64 ll; >> float f; >> double d; >> const char *s; >> } value; >> enum config_type type; >> const char *desc; >> }; >> >> >> IMHO, I think this is a bit complex >> and I want to simplify the perf's config list on perf-config. >> >> 2) A small number of perf's configs >> >> I think perf's configs aren't too many so I think >> two structure for section and element aren't needed. > > OK. > > >> >> 3) A object for a config variable need to have enough info for itself >> >> This is a bit similar to 1) reason. >> If using only 'struct config_item' for the config list, >> it can contain section name, name, values(default, user config, >> system config, both config), correct type, etc. >> >> If we do, we needn't to find detail for a config variable at other objects >> e.g. >> When we find correct type of a config variable, >> we needn't to do for-loop for default_configs[] in order to know the >> type. > > I'm not sure I understand you correctly, but I think this is not > related to the two-level structure. What you said is right. I just thought using two lists (list_head sections, default_configs[]) was complex.. At this patchset, I only focused on simplifying the config list on perf-config.. As you said, it hasn't problems to use the two-level structure i.e. struct config_element struct config_section (that has multiple elements) So, what about this structures ? (you may already think about it, but..) struct config_element { char *name; char *value; /*from the two config file (user/system)*/ union { bool b; int i; u32 l; u64 ll; float f; double d; const char *s; } default_value; /* perf's default config value */ enum config_type type; struct list_head list; }; struct config_section { char *name; struct list_head element_head; struct list_head list; }; Because I want to use only one list (default_configs + sections) for more concise code than old code. > >> >> >> I think this is better than old two structure. >> >>>> + >>>> + return NULL; >>>> +} >>>> + >>>> +static struct perf_config_item *add_config(struct list_head *config_list, >>>> + const char *section, >>>> + const char *name) >>>> +{ >>>> + struct perf_config_item *config = zalloc(sizeof(*config)); >>>> + >>>> + if (!config) >>>> + return NULL; >>>> + >>>> + config->section = strdup(section); >>>> + if (!section) >>>> + goto out_err; >>>> + >>>> + config->name = strdup(name); >>>> + if (!name) { >>>> + free((char *)config->section); >>>> + goto out_err; >>>> + } >>>> + >>>> + list_add_tail(&config->list, config_list); >>>> + return config; >>>> + >>>> +out_err: >>>> + free(config); >>>> + pr_err("%s: strdup failed\n", __func__); >>>> + return NULL; >>>> +} >>>> + >>>> +static int set_value(struct perf_config_item *config, const char *value) >>>> +{ >>>> + char *val = strdup(value); >>>> + >>>> + if (!val) >>>> + return -1; >>>> + config->value = val; >>> >>> It seems to overwrite old value.. >>> >> >> Yes, I know it. >> If don't using '--user' or '--system', >> there isn't exclusive config file path >> then have to read both config files. >> >> But because user config file has a high order of priority, >> if two config file has same variable, old value(for system config) >> must be overwrote by new value(for user config). > > But shouldn't it free the old value before overwriting? > Sorry, I missed free() out. I'll fix it. Thanks, Taeung > >> >>> >>> >>>> + >>>> + return 0; >>>> +} ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v2 2/5] perf config: Let show_config() work with perf_config_set 2016-03-14 12:16 [RFC][PATCH v2 0/5] perf config: Introduce perf_config_set class Taeung Song 2016-03-14 12:16 ` [PATCH v2 1/5] " Taeung Song @ 2016-03-14 12:16 ` Taeung Song 2016-03-14 12:16 ` [PATCH v2 3/5] perf config: Prepare all default configs Taeung Song ` (2 subsequent siblings) 4 siblings, 0 replies; 10+ messages in thread From: Taeung Song @ 2016-03-14 12:16 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: linux-kernel, Jiri Olsa, Namhyung Kim, Ingo Molnar, Peter Zijlstra, Taeung Song Current show_config() has a problem when user or system config files have same config variables i.e. # cat ~/.perfconfig [top] children = false when $(sysconfdir) is /usr/local/etc # cat /usr/local/etc/perfconfig [top] children = true Before: # perf config --user --list top.children=false # perf config --system --list top.children=true # perf config --list top.children=true top.children=false Because perf_config() can call show_config() each the config file (user and system). So fix it. After: # perf config --user --list top.children=false # perf config --system --list top.children=true # perf config --list top.children=false Cc: Namhyung Kim <namhyung@kernel.org> Cc: Jiri Olsa <jolsa@kernel.org> Signed-off-by: Taeung Song <treeze.taeung@gmail.com> --- tools/perf/builtin-config.c | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c index 412c725..2217772 100644 --- a/tools/perf/builtin-config.c +++ b/tools/perf/builtin-config.c @@ -33,13 +33,21 @@ static struct option config_options[] = { OPT_END() }; -static int show_config(const char *key, const char *value, - void *cb __maybe_unused) +static int show_config(struct perf_config_set *perf_configs) { - if (value) - printf("%s=%s\n", key, value); - else - printf("%s\n", key); + struct perf_config_item *config; + struct list_head *config_list = &perf_configs->config_list; + + if (list_empty(config_list)) + return -1; + + list_for_each_entry(config, config_list, list) { + char *value = config->value; + + if (value) + printf("%s.%s=%s\n", config->section, + config->name, value); + } return 0; } @@ -47,6 +55,7 @@ static int show_config(const char *key, const char *value, int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused) { int ret = 0; + struct perf_config_set *perf_configs; char *user_config = mkpath("%s/.perfconfig", getenv("HOME")); argc = parse_options(argc, argv, config_options, config_usage, @@ -64,13 +73,19 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused) else if (use_user_config) config_exclusive_filename = user_config; + perf_configs = perf_config_set__new(); + if (!perf_configs) { + ret = -1; + goto out_err; + } + switch (actions) { case ACTION_LIST: if (argc) { pr_err("Error: takes no arguments\n"); parse_options_usage(config_usage, config_options, "l", 1); } else { - ret = perf_config(show_config, NULL); + ret = show_config(perf_configs); if (ret < 0) { const char * config_filename = config_exclusive_filename; if (!config_exclusive_filename) @@ -84,5 +99,7 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused) usage_with_options(config_usage, config_options); } + perf_config_set__delete(perf_configs); +out_err: return ret; } -- 2.5.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 3/5] perf config: Prepare all default configs 2016-03-14 12:16 [RFC][PATCH v2 0/5] perf config: Introduce perf_config_set class Taeung Song 2016-03-14 12:16 ` [PATCH v2 1/5] " Taeung Song 2016-03-14 12:16 ` [PATCH v2 2/5] perf config: Let show_config() work with perf_config_set Taeung Song @ 2016-03-14 12:16 ` Taeung Song 2016-03-14 12:16 ` [PATCH v2 4/5] perf config: Initialize perf_config_set with " Taeung Song 2016-03-14 12:16 ` [PATCH v2 5/5] perf config: Add 'list-all' option to show all perf's configs Taeung Song 4 siblings, 0 replies; 10+ messages in thread From: Taeung Song @ 2016-03-14 12:16 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: linux-kernel, Jiri Olsa, Namhyung Kim, Ingo Molnar, Peter Zijlstra, Taeung Song To precisely manage configs, prepare all default perf's configs that contain default section name, variable name, value and correct type, not string type. In the near future, this will be used when checking type of config variable or showing all configs with default values, etc. Cc: Namhyung Kim <namhyung@kernel.org> Cc: Jiri Olsa <jolsa@kernel.org> Signed-off-by: Taeung Song <treeze.taeung@gmail.com> --- tools/perf/util/config.c | 54 ++++++++++++++++++++++++++--- tools/perf/util/config.h | 89 ++++++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 137 insertions(+), 6 deletions(-) diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c index b9660e4..0b9ba51 100644 --- a/tools/perf/util/config.c +++ b/tools/perf/util/config.c @@ -29,6 +29,52 @@ static int config_file_eof; const char *config_exclusive_filename; +struct perf_config_item default_configs[] = { + CONF_STR_VAR(COLORS_TOP, "colors", "top", "red, default"), + CONF_STR_VAR(COLORS_MEDIUM, "colors", "medium", "green, default"), + CONF_STR_VAR(COLORS_NORMAL, "colors", "normal", "lightgray, default"), + CONF_STR_VAR(COLORS_SELECTED, "colors", "selected", "white, lightgray"), + CONF_STR_VAR(COLORS_JUMP_ARROWS, "colors", "jump_arrows", "blue, default"), + CONF_STR_VAR(COLORS_ADDR, "colors", "addr", "magenta, default"), + CONF_STR_VAR(COLORS_ROOT, "colors", "root", "white, blue"), + CONF_BOOL_VAR(TUI_REPORT, "tui", "report", true), + CONF_BOOL_VAR(TUI_ANNOTATE, "tui", "annotate", true), + CONF_BOOL_VAR(TUI_TOP, "tui", "top", true), + CONF_STR_VAR(BUILDID_DIR, "buildid", "dir", "~/.debug"), + CONF_BOOL_VAR(ANNOTATE_HIDE_SRC_CODE, "annotate", "hide_src_code", false), + CONF_BOOL_VAR(ANNOTATE_USE_OFFSET, "annotate", "use_offset", true), + CONF_BOOL_VAR(ANNOTATE_JUMP_ARROWS, "annotate", "jump_arrows", true), + CONF_BOOL_VAR(ANNOTATE_SHOW_NR_JUMPS, "annotate", "show_nr_jumps", false), + CONF_BOOL_VAR(ANNOTATE_SHOW_LINENR, "annotate", "show_linenr", false), + CONF_BOOL_VAR(ANNOTATE_SHOW_TOTAL_PERIOD, "annotate", "show_total_period", false), + CONF_BOOL_VAR(GTK_ANNOTATE, "gtk", "annotate", false), + CONF_BOOL_VAR(GTK_REPORT, "gtk", "report", false), + CONF_BOOL_VAR(GTK_TOP, "gtk", "top", false), + CONF_BOOL_VAR(PAGER_CMD, "pager", "cmd", true), + CONF_BOOL_VAR(PAGER_REPORT, "pager", "report", true), + CONF_BOOL_VAR(PAGER_ANNOTATE, "pager", "annotate", true), + CONF_BOOL_VAR(PAGER_TOP, "pager", "top", true), + CONF_BOOL_VAR(PAGER_DIFF, "pager", "diff", true), + CONF_STR_VAR(HELP_FORMAT, "help", "format", "man"), + CONF_INT_VAR(HELP_AUTOCORRECT, "help", "autocorrect", 0), + CONF_STR_VAR(HIST_PERCENTAGE, "hist", "percentage", "absolute"), + CONF_BOOL_VAR(UI_SHOW_HEADERS, "ui", "show-headers", true), + CONF_STR_VAR(CALL_GRAPH_RECORD_MODE, "call-graph", "record-mode", "fp"), + CONF_LONG_VAR(CALL_GRAPH_DUMP_SIZE, "call-graph", "dump-size", 8192), + CONF_STR_VAR(CALL_GRAPH_PRINT_TYPE, "call-graph", "print-type", "graph"), + CONF_STR_VAR(CALL_GRAPH_ORDER, "call-graph", "order", "callee"), + CONF_STR_VAR(CALL_GRAPH_SORT_KEY, "call-graph", "sort-key", "function"), + CONF_DOUBLE_VAR(CALL_GRAPH_THRESHOLD, "call-graph", "threshold", 0.5), + CONF_LONG_VAR(CALL_GRAPH_PRINT_LIMIT, "call-graph", "print-limit", 0), + CONF_BOOL_VAR(REPORT_GROUP, "report", "group", true), + CONF_BOOL_VAR(REPORT_CHILDREN, "report", "children", true), + CONF_FLOAT_VAR(REPORT_PERCENT_LIMIT, "report", "percent-limit", 0), + CONF_U64_VAR(REPORT_QUEUE_SIZE, "report", "queue-size", 0), + CONF_BOOL_VAR(TOP_CHILDREN, "top", "children", true), + CONF_STR_VAR(MAN_VIEWER, "man", "viewer", "man"), + CONF_STR_VAR(KMEM_DEFAULT, "kmem", "default", "slab"), +}; + static int get_next_char(void) { int c; @@ -587,8 +633,8 @@ static int collect_config(const char *var, const char *value, if (!config) { config = add_config(config_list, section, name); if (!config) { - free(config->section); - free(config->name); + free((char *)config->section); + free((char *)config->name); ret = -1; goto out_free; } @@ -620,8 +666,8 @@ void perf_config_set__delete(struct perf_config_set *perf_configs) list_for_each_entry_safe(pos, item, &perf_configs->config_list, list) { list_del(&pos->list); - free(pos->section); - free(pos->name); + free((char *)pos->section); + free((char *)pos->name); free(pos->value); free(pos); } diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h index 44c226f..3c30576 100644 --- a/tools/perf/util/config.h +++ b/tools/perf/util/config.h @@ -4,10 +4,30 @@ #include <stdbool.h> #include <linux/list.h> +enum perf_config_type { + CONFIG_TYPE_BOOL, + CONFIG_TYPE_INT, + CONFIG_TYPE_LONG, + CONFIG_TYPE_U64, + CONFIG_TYPE_FLOAT, + CONFIG_TYPE_DOUBLE, + CONFIG_TYPE_STRING +}; + struct perf_config_item { - char *section; - char *name; + const char *section; + const char *name; char *value; + union { + bool b; + int i; + u32 l; + u64 ll; + float f; + double d; + const char *s; + } default_value; + enum perf_config_type type; struct list_head list; }; @@ -15,6 +35,71 @@ struct perf_config_set { struct list_head config_list; }; +enum perf_config_idx { + CONFIG_COLORS_TOP, + CONFIG_COLORS_MEDIUM, + CONFIG_COLORS_NORMAL, + CONFIG_COLORS_SELECTED, + CONFIG_COLORS_JUMP_ARROWS, + CONFIG_COLORS_ADDR, + CONFIG_COLORS_ROOT, + CONFIG_TUI_REPORT, + CONFIG_TUI_ANNOTATE, + CONFIG_TUI_TOP, + CONFIG_BUILDID_DIR, + CONFIG_ANNOTATE_HIDE_SRC_CODE, + CONFIG_ANNOTATE_USE_OFFSET, + CONFIG_ANNOTATE_JUMP_ARROWS, + CONFIG_ANNOTATE_SHOW_NR_JUMPS, + CONFIG_ANNOTATE_SHOW_LINENR, + CONFIG_ANNOTATE_SHOW_TOTAL_PERIOD, + CONFIG_GTK_ANNOTATE, + CONFIG_GTK_REPORT, + CONFIG_GTK_TOP, + CONFIG_PAGER_CMD, + CONFIG_PAGER_REPORT, + CONFIG_PAGER_ANNOTATE, + CONFIG_PAGER_TOP, + CONFIG_PAGER_DIFF, + CONFIG_HELP_FORMAT, + CONFIG_HELP_AUTOCORRECT, + CONFIG_HIST_PERCENTAGE, + CONFIG_UI_SHOW_HEADERS, + CONFIG_CALL_GRAPH_RECORD_MODE, + CONFIG_CALL_GRAPH_DUMP_SIZE, + CONFIG_CALL_GRAPH_PRINT_TYPE, + CONFIG_CALL_GRAPH_ORDER, + CONFIG_CALL_GRAPH_SORT_KEY, + CONFIG_CALL_GRAPH_THRESHOLD, + CONFIG_CALL_GRAPH_PRINT_LIMIT, + CONFIG_REPORT_GROUP, + CONFIG_REPORT_CHILDREN, + CONFIG_REPORT_PERCENT_LIMIT, + CONFIG_REPORT_QUEUE_SIZE, + CONFIG_TOP_CHILDREN, + CONFIG_MAN_VIEWER, + CONFIG_KMEM_DEFAULT, + CONFIG_END +}; + +#define CONF_VAR(_sec, _name, _field, _val, _type) \ + { .section = _sec, .name = _name, .default_value._field = _val, .type = _type } + +#define CONF_BOOL_VAR(_idx, _sec, _name, _val) \ + [CONFIG_##_idx] = CONF_VAR(_sec, _name, b, _val, CONFIG_TYPE_BOOL) +#define CONF_INT_VAR(_idx, _sec, _name, _val) \ + [CONFIG_##_idx] = CONF_VAR(_sec, _name, i, _val, CONFIG_TYPE_INT) +#define CONF_LONG_VAR(_idx, _sec, _name, _val) \ + [CONFIG_##_idx] = CONF_VAR(_sec, _name, l, _val, CONFIG_TYPE_LONG) +#define CONF_U64_VAR(_idx, _sec, _name, _val) \ + [CONFIG_##_idx] = CONF_VAR(_sec, _name, ll, _val, CONFIG_TYPE_U64) +#define CONF_FLOAT_VAR(_idx, _sec, _name, _val) \ + [CONFIG_##_idx] = CONF_VAR(_sec, _name, f, _val, CONFIG_TYPE_FLOAT) +#define CONF_DOUBLE_VAR(_idx, _sec, _name, _val) \ + [CONFIG_##_idx] = CONF_VAR(_sec, _name, d, _val, CONFIG_TYPE_DOUBLE) +#define CONF_STR_VAR(_idx, _sec, _name, _val) \ + [CONFIG_##_idx] = CONF_VAR(_sec, _name, s, _val, CONFIG_TYPE_STRING) + struct perf_config_set *perf_config_set__new(void); void perf_config_set__delete(struct perf_config_set *perf_configs); -- 2.5.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 4/5] perf config: Initialize perf_config_set with all default configs 2016-03-14 12:16 [RFC][PATCH v2 0/5] perf config: Introduce perf_config_set class Taeung Song ` (2 preceding siblings ...) 2016-03-14 12:16 ` [PATCH v2 3/5] perf config: Prepare all default configs Taeung Song @ 2016-03-14 12:16 ` Taeung Song 2016-03-14 12:16 ` [PATCH v2 5/5] perf config: Add 'list-all' option to show all perf's configs Taeung Song 4 siblings, 0 replies; 10+ messages in thread From: Taeung Song @ 2016-03-14 12:16 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: linux-kernel, Jiri Olsa, Namhyung Kim, Ingo Molnar, Peter Zijlstra, Taeung Song To avoid duplicated config variables and use perf_config_set classifying between standard perf config variables and unknown or new config variables other than them, initialize perf_config_set with all default configs. And this will be needed when showing all configs with default value or checking correct type of a config variable in the near future. Cc: Namhyung Kim <namhyung@kernel.org> Cc: Jiri Olsa <jolsa@kernel.org> Signed-off-by: Taeung Song <treeze.taeung@gmail.com> --- tools/perf/builtin-config.c | 11 +++++++---- tools/perf/util/config.c | 27 ++++++++++++++++++++++----- tools/perf/util/config.h | 6 ++++++ 3 files changed, 35 insertions(+), 9 deletions(-) diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c index 2217772..1bc1121 100644 --- a/tools/perf/builtin-config.c +++ b/tools/perf/builtin-config.c @@ -35,20 +35,23 @@ static struct option config_options[] = { static int show_config(struct perf_config_set *perf_configs) { + bool has_value = false; struct perf_config_item *config; struct list_head *config_list = &perf_configs->config_list; - if (list_empty(config_list)) - return -1; - list_for_each_entry(config, config_list, list) { char *value = config->value; - if (value) + if (value) { printf("%s.%s=%s\n", config->section, config->name, value); + has_value = true; + } } + if (!has_value) + return -1; + return 0; } diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c index 0b9ba51..5289063 100644 --- a/tools/perf/util/config.c +++ b/tools/perf/util/config.c @@ -647,6 +647,21 @@ out_free: return ret; } +static struct perf_config_set *perf_config_set__init(struct perf_config_set *perf_configs) +{ + int i; + struct list_head *head = &perf_configs->config_list; + + INIT_LIST_HEAD(&perf_configs->config_list); + + for (i = 0; i != CONFIG_END; i++) { + struct perf_config_item *config = &default_configs[i]; + + list_add_tail(&config->list, head); + } + return perf_configs; +} + struct perf_config_set *perf_config_set__new(void) { struct perf_config_set *perf_configs = zalloc(sizeof(*perf_configs)); @@ -654,7 +669,7 @@ struct perf_config_set *perf_config_set__new(void) if (!perf_configs) return NULL; - INIT_LIST_HEAD(&perf_configs->config_list); + perf_config_set__init(perf_configs); perf_config(collect_config, &perf_configs->config_list); return perf_configs; @@ -666,10 +681,12 @@ void perf_config_set__delete(struct perf_config_set *perf_configs) list_for_each_entry_safe(pos, item, &perf_configs->config_list, list) { list_del(&pos->list); - free((char *)pos->section); - free((char *)pos->name); - free(pos->value); - free(pos); + if (pos->is_custom) { + free((char *)pos->section); + free((char *)pos->name); + free(pos->value); + free(pos); + } } free(perf_configs); diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h index 3c30576..df47420 100644 --- a/tools/perf/util/config.h +++ b/tools/perf/util/config.h @@ -14,6 +14,11 @@ enum perf_config_type { CONFIG_TYPE_STRING }; +/** + * struct perf_config_item - element of perf's configs + * + * @is_custom: unknown or new config other than default config + */ struct perf_config_item { const char *section; const char *name; @@ -28,6 +33,7 @@ struct perf_config_item { const char *s; } default_value; enum perf_config_type type; + bool is_custom; struct list_head list; }; -- 2.5.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2 5/5] perf config: Add 'list-all' option to show all perf's configs 2016-03-14 12:16 [RFC][PATCH v2 0/5] perf config: Introduce perf_config_set class Taeung Song ` (3 preceding siblings ...) 2016-03-14 12:16 ` [PATCH v2 4/5] perf config: Initialize perf_config_set with " Taeung Song @ 2016-03-14 12:16 ` Taeung Song 4 siblings, 0 replies; 10+ messages in thread From: Taeung Song @ 2016-03-14 12:16 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: linux-kernel, Jiri Olsa, Namhyung Kim, Ingo Molnar, Peter Zijlstra, Taeung Song That show all possible config variables with default values. The syntax examples are like below. perf config [<file-option>] [options] display all perf config with default values. # perf config -a | --list-all But configs from user or system config file have a high priority e.g. Default of report.children is true # cat ~/.perfconfig [report] children=false # perf config --list-all ..(omitted).. call-graph.threshold=0.500000 call-graph.print-limit=0 report.group=true report.children=false report.percent-limit=0.000000 report.queue-size=0 ..(omitted).. Cc: Namhyung Kim <namhyung@kernel.org> Cc: Jiri Olsa <jolsa@kernel.org> Signed-off-by: Taeung Song <treeze.taeung@gmail.com> --- tools/perf/Documentation/perf-config.txt | 6 +++ tools/perf/builtin-config.c | 69 +++++++++++++++++++++++++++++++- 2 files changed, 74 insertions(+), 1 deletion(-) diff --git a/tools/perf/Documentation/perf-config.txt b/tools/perf/Documentation/perf-config.txt index 15949e2..d9fb8c3 100644 --- a/tools/perf/Documentation/perf-config.txt +++ b/tools/perf/Documentation/perf-config.txt @@ -9,6 +9,8 @@ SYNOPSIS -------- [verse] 'perf config' [<file-option>] -l | --list +or +'perf config' [<file-option>] -a | --list-all DESCRIPTION ----------- @@ -29,6 +31,10 @@ OPTIONS For writing and reading options: write to system-wide '$(sysconfdir)/perfconfig' or read it. +-a:: +--list-all:: + Show current and all possible config variables with default values. + CONFIGURATION FILE ------------------ diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c index 1bc1121..7ceae55 100644 --- a/tools/perf/builtin-config.c +++ b/tools/perf/builtin-config.c @@ -22,17 +22,75 @@ static const char * const config_usage[] = { }; enum actions { - ACTION_LIST = 1 + ACTION_LIST = 1, + ACTION_LIST_ALL } actions; static struct option config_options[] = { OPT_SET_UINT('l', "list", &actions, "show current config variables", ACTION_LIST), + OPT_SET_UINT('a', "list-all", &actions, + "show current and all possible config" + " variables with default values", ACTION_LIST_ALL), OPT_BOOLEAN(0, "system", &use_system_config, "use system config file"), OPT_BOOLEAN(0, "user", &use_user_config, "use user config file"), OPT_END() }; +#define DEFAULT_VAL(_field) config->default_value._field + +static char *get_default_value(struct perf_config_item *config) +{ + int ret = 0; + char *value; + + if (config->is_custom) + return NULL; + + if (config->type == CONFIG_TYPE_BOOL) + ret = asprintf(&value, "%s", + DEFAULT_VAL(b) ? "true" : "false"); + else if (config->type == CONFIG_TYPE_INT) + ret = asprintf(&value, "%d", DEFAULT_VAL(i)); + else if (config->type == CONFIG_TYPE_LONG) + ret = asprintf(&value, "%u", DEFAULT_VAL(l)); + else if (config->type == CONFIG_TYPE_U64) + ret = asprintf(&value, "%"PRId64, DEFAULT_VAL(ll)); + else if (config->type == CONFIG_TYPE_FLOAT) + ret = asprintf(&value, "%f", DEFAULT_VAL(f)); + else if (config->type == CONFIG_TYPE_DOUBLE) + ret = asprintf(&value, "%f", DEFAULT_VAL(d)); + else + ret = asprintf(&value, "%s", DEFAULT_VAL(s)); + + if (ret < 0) + return NULL; + + return value; +} + +static int show_all_config(struct perf_config_set *perf_configs) +{ + char *value, *default_value; + struct perf_config_item *config; + struct list_head *config_list = &perf_configs->config_list; + + list_for_each_entry(config, config_list, list) { + value = config->value; + if (!value) + value = default_value = get_default_value(config); + + if (value) + printf("%s.%s=%s\n", config->section, + config->name, value); + + free(default_value); + default_value = NULL; + } + + return 0; +} + static int show_config(struct perf_config_set *perf_configs) { bool has_value = false; @@ -61,6 +119,9 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused) struct perf_config_set *perf_configs; char *user_config = mkpath("%s/.perfconfig", getenv("HOME")); + set_option_flag(config_options, 'l', "list", PARSE_OPT_EXCLUSIVE); + set_option_flag(config_options, 'a', "list-all", PARSE_OPT_EXCLUSIVE); + argc = parse_options(argc, argv, config_options, config_usage, PARSE_OPT_STOP_AT_NON_OPTION); @@ -83,6 +144,12 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused) } switch (actions) { + case ACTION_LIST_ALL: + if (argc) + parse_options_usage(config_usage, config_options, "a", 1); + else + ret = show_all_config(perf_configs); + break; case ACTION_LIST: if (argc) { pr_err("Error: takes no arguments\n"); -- 2.5.0 ^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-03-18 0:52 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-03-14 12:16 [RFC][PATCH v2 0/5] perf config: Introduce perf_config_set class Taeung Song 2016-03-14 12:16 ` [PATCH v2 1/5] " Taeung Song 2016-03-17 12:31 ` Namhyung Kim 2016-03-17 14:10 ` Taeung Song 2016-03-17 23:27 ` Namhyung Kim 2016-03-18 0:52 ` Taeung Song 2016-03-14 12:16 ` [PATCH v2 2/5] perf config: Let show_config() work with perf_config_set Taeung Song 2016-03-14 12:16 ` [PATCH v2 3/5] perf config: Prepare all default configs Taeung Song 2016-03-14 12:16 ` [PATCH v2 4/5] perf config: Initialize perf_config_set with " Taeung Song 2016-03-14 12:16 ` [PATCH v2 5/5] perf config: Add 'list-all' option to show all perf's configs 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).