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 <namhy...@kernel.org> > Cc: Jiri Olsa <jo...@redhat.com> > Cc: Wang Nan <wangn...@huawei.com> > Cc: Peter Zijlstra <pet...@infradead.org> > Cc: Ingo Molnar <mi...@redhat.com> > Cc: Masami Hiramatsu <mhira...@kernel.org> > Cc: Alexander Shishkin <alexander.shish...@linux.intel.com> > Signed-off-by: Taeung Song <treeze.tae...@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(§ion->items, item) Ditto. > #endif /* __PERF_CONFIG_H */ > -- > 2.5.0