Infrastructure code for perf-config
v5 cover Infrastructure code for perf-config
Hi,
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 contains basic code 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 think this infrastructure code is needed
to add new funcationalities for config management of perf-config.
If anyone reviews this, I'd appreciate it.
Thanks,
Taeung
v5:
- departmentalize perf_config_set__delete() (Arnaldo)
- remove confusing find_config() (Arnaldo)
- use pr_debug() instead of pr_err() (Arnaldo)
- use zfree() instead of free() (Arnaldo)
- more compact in perf_config_set__new() (Arnaldo)
- rename variables 'perf_configs', 'config_items', etc. (Arnaldo)
v4:
- fill perf_config_set__delete() in collect_config() for state of error
- fill the code setting is_custom value in add_config_item() (Namhyung)
v3:
- use the section list that contains configs each section instead of the single config list (Namhyung)
- exclude a patch for '--list-all' option from this patchset
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 (4):
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
tools/perf/builtin-config.c | 38 +++++- tools/perf/util/config.c | 309 ++++++++++++++++++++++++++++++++++++++++++++ tools/perf/util/config.h | 91 +++++++++++++ 3 files changed, 431 insertions(+), 7 deletions(-) create mode 100644 tools/perf/util/config.h
Signature · 1 line
2.5.0
PATCH v5 1/4 perf config: Introduce perf_config_set class
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.
--- tools/perf/util/config.c | 176 +++++++++++++++++++++++++++++++++++++++++++++++ tools/perf/util/config.h | 26 +++++++ 2 files changed, 202 insertions(+) create mode 100644 tools/perf/util/config.h
tools/perf/util/config.c +176 -0
@@ -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)
@@ -524,6 +525,181 @@ out:return ret; } +static struct perf_config_section *find_section (struct list_head *sections, +const char *section_name) +{ +struct perf_config_section *section; + +list_for_each_entry (section, sections, node) +if (!strcmp (section->name, section_name)) +return section; + +return NULL ; +} + +static struct perf_config_item *find_config_item (const char *name, +struct perf_config_section *section) +{ +struct perf_config_item *item; + +list_for_each_entry (item, §ion->items, node) +if (!strcmp (item->name, name)) +return item; + +return NULL ; +} + +static struct perf_config_section *add_section (struct list_head *sections, +const char *section_name) +{ +struct perf_config_section *section =zalloc (sizeof (*section)); + +if (!section) +return NULL ; + +INIT_LIST_HEAD (§ion->items); + section->name =strdup (section_name); +if (!section->name) { +pr_debug ("%s: strdup failed\n" , __func__); +free (section); +return NULL ; + } + +list_add_tail (§ion->node, sections); +return section; +} + +static struct perf_config_item *add_config_item (struct perf_config_section *section, +const char *name) +{ +struct perf_config_item *item =zalloc (sizeof (*item)); + +if (!item) +return NULL ; + + item->name =strdup (name); +if (!item->name) { +pr_debug ("%s: strdup failed\n" , __func__); + goto out_err; + } + +list_add_tail (&item->node, §ion->items); +return item; + +out_err: +free (item); +return NULL ; +} + +static int set_value (struct perf_config_item *item,const char *value) +{ +char *val =strdup (value); + +if (!val) +return -1 ; + +free (item->value); + item->value = val; +return 0 ; +} + +static int collect_config (const char *var,const char *value, +void *perf_config_set) +{ +int ret =-1 ; +char *ptr, *key; +char *section_name, *name; +struct perf_config_section *section =NULL ; +struct perf_config_item *item =NULL ; +struct perf_config_set *set = perf_config_set; +struct list_head *sections = &set->sections; + + key = ptr =strdup (var); +if (!key) { +pr_debug ("%s: strdup failed\n" , __func__); +return -1 ; + } + + section_name =strsep (&ptr,"." ); + name = ptr; +if (name ==NULL || value ==NULL ) + goto out_free; + + section =find_section (sections, section_name); +if (!section) { + section =add_section (sections, section_name); +if (!section) + goto out_free; + } + + item =find_config_item (name, section); +if (!item) { + item =add_config_item (section, name); +if (!item) + goto out_free; + } + + ret =set_value (item, value); +return ret; + +out_free: +free (key); +perf_config_set__delete (set); +return -1 ; +} + +struct perf_config_set *perf_config_set__new (void ) +{ +struct perf_config_set *set =zalloc (sizeof (*set)); + +if (set) { +INIT_LIST_HEAD (&set->sections); +perf_config (collect_config, set); + } + +return set; +} + +static void perf_config_item__delete (struct perf_config_item *item) +{ +zfree (&item->name); +zfree (&item->value); +free (item); +} + +static void perf_config_section__purge (struct perf_config_section *section) +{ +struct perf_config_item *item, *tmp; + +list_for_each_entry_safe (item, tmp, §ion->items, node) { +list_del_init (&item->node); +perf_config_item__delete (item); + } +} + +static void perf_config_section__delete (struct perf_config_section *section) +{ +perf_config_section__purge (section); +zfree (§ion->name); +free (section); +} + +static void perf_config_set__purge (struct perf_config_set *set) +{ +struct perf_config_section *section, *tmp; + +list_for_each_entry_safe (section, tmp, &set->sections, node) { +list_del_init (§ion->node); +perf_config_section__delete (section); + } +} + +void perf_config_set__delete (struct perf_config_set *set) +{ +perf_config_set__purge (set); +free (set); +} +/* * Call this to report error for your variable that should not * get a boolean value (i.e. "[my] var" means "true").
tools/perf/util/config.h new file +26 -0
@@ -0,0 +1,26 @@ +#ifndef __PERF_CONFIG_H +#define __PERF_CONFIG_H + +#include <stdbool.h> +#include <linux/list.h> + +struct perf_config_item { +char *name; +char *value; +struct list_head node; +}; + +struct perf_config_section { +char *name; +struct list_head items; +struct list_head node; +}; + +struct perf_config_set { +struct list_head sections; +}; + +struct perf_config_set *perf_config_set__new (void ); +void perf_config_set__delete (struct perf_config_set *set); + +#endif /* __PERF_CONFIG_H */
Signature · 1 line
2.5.0
Re: [PATCH v5 1/4] perf config: Introduce perf_config_set class
Hi,
2016-04-03 2:08 GMT+09:00 Taeung Song <[email protected]>:
[...]
Quoted 22 lines
tools/perf/util/config.c +22
-0+static struct perf_config_section *add_section (struct list_head *sections, +const char *section_name) +{ +struct perf_config_section *section =zalloc (sizeof (*section)); + +if (!section) +return NULL ; + +INIT_LIST_HEAD (§ion->items); + section->name =strdup (section_name); +if (!section->name) { +pr_debug ("%s: strdup failed\n" , __func__); +free (section); +return NULL ; + } + +list_add_tail (§ion->node, sections); +return section; +} + +static struct perf_config_item *add_config_item (struct perf_config_section *section, +const char *name)
tools/perf/util/config.c out: · line 583
Quoted 10 lines
tools/perf/util/config.c +10
-0+{ +struct perf_config_item *item =zalloc (sizeof (*item)); + +if (!item) +return NULL ; + + item->name =strdup (name); +if (!item->name) { +pr_debug ("%s: strdup failed\n" , __func__); + goto out_err;
As you did in above add_section(), you can do free(item) and return NULL here instead of using goto.
Other parts looks OK for me.
Thank you,
Quoted 9 lines
tools/perf/util/config.c +9
-0+ } + +list_add_tail (&item->node, §ion->items); +return item; + +out_err: +free (item); +return NULL ; +}
Re: [PATCH v5 1/4] perf config: Introduce perf_config_set class
Hi, Masami Hiramatsu
Quoted 40 lines · Masami Hiramatsu
Hi,
2016-04-03 2:08 GMT+09:00 Taeung Song <[email protected]>:
[...]Quoted 22 lines
(quoted diff) +22
-0+static struct perf_config_section *add_section(struct list_head *sections, + const char *section_name) +{ + struct perf_config_section *section = zalloc(sizeof(*section)); + + if (!section) + return NULL; + + INIT_LIST_HEAD(§ion->items); + section->name = strdup(section_name); + if (!section->name) { + pr_debug("%s: strdup failed\n", __func__); + free(section); + return NULL; + } + + list_add_tail(§ion->node, sections); + return section; +} + +static struct perf_config_item *add_config_item(struct perf_config_section *section, + const char *name)Review
Quoted 10 lines
(quoted diff) +10
-0+{ + struct perf_config_item *item = zalloc(sizeof(*item)); + + if (!item) + return NULL; + + item->name = strdup(name); + if (!item->name) { + pr_debug("%s: strdup failed\n", __func__); + goto out_err;As you did in above
add_section(), you can do free(item) and return NULL here instead of using goto.
Quoted 2 lines
Other parts looks OK for me.
I got it.
What you mean is like below ?
tools/perf/util/config.c +2 -5
@@ -580,15 +580,12 @@ static struct perf_config_item *add_config_item(struct perf_config_section *sect item->name =strdup (name);if (!item->name) {pr_debug ("%s: strdup failed\n" , __func__); - goto out_err; +free (item); +return NULL ; }list_add_tail (&item->node, §ion->items);return item; - -out_err: -free (item); -return NULL ; }static int set_value (struct perf_config_item *item,const char *value)
I'll resend v6 with this.
Thank you for your review. :-)
Taeung
PATCH v5 2/4 perf config: Let show_config() work with perf_config_set
In reply to Taeung SongCurrent 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=falseBecause 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--- tools/perf/builtin-config.c | 35 ++++++++++++++++++++++++++++------- 1 file changed, 28 insertions(+), 7 deletions(-)
tools/perf/builtin-config.c +28 -7
@@ -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;
@@ -32,13 +33,24 @@ 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 *set) { -if (value) -printf ("%s=%s\n" , key, value); -else -printf ("%s\n" , key); +struct perf_config_section *section; +struct perf_config_item *item; +struct list_head *sections = &set->sections; + +if (list_empty (sections)) +return -1 ; + +list_for_each_entry (section, sections, node) { +list_for_each_entry (item, §ion->items, node) { +char *value = item->value; + +if (value) +printf ("%s.%s=%s\n" , section->name, + item->name, value); + } + }return 0 ; }
@@ -46,6 +58,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 *set;char *user_config =mkpath ("%s/.perfconfig" ,getenv ("HOME" )); argc = parse_options(argc, argv, config_options, config_usage,
@@ -63,13 +76,19 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused)else if (use_user_config) config_exclusive_filename = user_config; + set =perf_config_set__new (); +if (!set) { + 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 (set);if (ret <0 ) {const char * config_filename = config_exclusive_filename;if (!config_exclusive_filename)
@@ -83,5 +102,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 (set); +out_err:return ret; }
Signature · 1 line
2.5.0
PATCH v5 3/4 perf config: Prepare all default configs
In reply to Taeung SongTo 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.
--- tools/perf/util/config.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++- tools/perf/util/config.h | 62 +++++++++++++++++++++++++- 2 files changed, 168 insertions(+), 4 deletions(-)
tools/perf/util/config.c +108 -2
@@ -15,6 +15,7 @@#include "util/llvm-utils.h" /* perf_llvm_config */ #include "config.h" +#define MAX_CONFIGS 64#define MAXNAME (256)#define DEBUG_CACHE_DIR ".debug"
@@ -29,6 +30,111 @@ static int config_file_eof;const char *config_exclusive_filename; +struct perf_config_section default_sections[] = { + { .name ="colors" }, + { .name ="tui" }, + { .name ="buildid" }, + { .name ="annotate" }, + { .name ="gtk" }, + { .name ="pager" }, + { .name ="help" }, + { .name ="hist" }, + { .name ="ui" }, + { .name ="call-graph" }, + { .name ="report" }, + { .name ="top" }, + { .name ="man" }, + { .name ="kmem" } +}; + +struct perf_config_item default_config_items[][MAX_CONFIGS ] = { + [CONFIG_COLORS ] = { +CONF_STR_VAR ("top" ,"red, default" ), +CONF_STR_VAR ("medium" ,"green, default" ), +CONF_STR_VAR ("normal" ,"lightgray, default" ), +CONF_STR_VAR ("selected" ,"white, lightgray" ), +CONF_STR_VAR ("jump_arrows" ,"blue, default" ), +CONF_STR_VAR ("addr" ,"magenta, default" ), +CONF_STR_VAR ("root" ,"white, blue" ), +CONF_END () + }, + [CONFIG_TUI ] = { +CONF_BOOL_VAR ("report" , true), +CONF_BOOL_VAR ("annotate" , true), +CONF_BOOL_VAR ("top" , true), +CONF_END () + }, + [CONFIG_BUILDID ] = { +CONF_STR_VAR ("dir" ,"~/.debug" ), +CONF_END () + }, + [CONFIG_ANNOTATE ] = { +CONF_BOOL_VAR ("hide_src_code" , false), +CONF_BOOL_VAR ("use_offset" , true), +CONF_BOOL_VAR ("jump_arrows" , true), +CONF_BOOL_VAR ("show_nr_jumps" , false), +CONF_BOOL_VAR ("show_linenr" , false), +CONF_BOOL_VAR ("show_total_period" , false), +CONF_END () + }, + [CONFIG_GTK ] = { +CONF_BOOL_VAR ("annotate" , false), +CONF_BOOL_VAR ("report" , false), +CONF_BOOL_VAR ("top" , false), +CONF_END () + }, + [CONFIG_PAGER ] = { +CONF_BOOL_VAR ("cmd" , true), +CONF_BOOL_VAR ("report" , true), +CONF_BOOL_VAR ("annotate" , true), +CONF_BOOL_VAR ("top" , true), +CONF_BOOL_VAR ("diff" , true), +CONF_END () + }, + [CONFIG_HELP ] = { +CONF_STR_VAR ("format" ,"man" ), +CONF_INT_VAR ("autocorrect" ,0 ), +CONF_END () + }, + [CONFIG_HIST ] = { +CONF_STR_VAR ("percentage" ,"absolute" ), +CONF_END () + }, + [CONFIG_UI ] = { +CONF_BOOL_VAR ("show-headers" , true), +CONF_END () + }, + [CONFIG_CALL_GRAPH ] = { +CONF_STR_VAR ("record-mode" ,"fp" ), +CONF_LONG_VAR ("dump-size" ,8192 ), +CONF_STR_VAR ("print-type" ,"graph" ), +CONF_STR_VAR ("order" ,"callee" ), +CONF_STR_VAR ("sort-key" ,"function" ), +CONF_DOUBLE_VAR ("threshold" ,0.5 ), +CONF_LONG_VAR ("print-limit" ,0 ), +CONF_END () + }, + [CONFIG_REPORT ] = { +CONF_BOOL_VAR ("group" , true), +CONF_BOOL_VAR ("children" , true), +CONF_FLOAT_VAR ("percent-limit" ,0 ), +CONF_U64_VAR ("queue-size" ,0 ), +CONF_END () + }, + [CONFIG_TOP ] = { +CONF_BOOL_VAR ("children" , true), +CONF_END () + }, + [CONFIG_MAN ] = { +CONF_STR_VAR ("viewer" ,"man" ), +CONF_END () + }, + [CONFIG_KMEM ] = { +CONF_STR_VAR ("default" ,"slab" ), +CONF_END () + } +}; +static int get_next_char (void ) {int c;
@@ -662,7 +768,7 @@ struct perf_config_set *perf_config_set__new(void)static void perf_config_item__delete (struct perf_config_item *item) { -zfree (&item->name); +zfree ((char **)&item->name);zfree (&item->value);free (item); }
@@ -680,7 +786,7 @@ static void perf_config_section__purge(struct perf_config_section *section)static void perf_config_section__delete (struct perf_config_section *section) {perf_config_section__purge (section); -zfree (§ion->name); +zfree ((char **)§ion->name);free (section); }
tools/perf/util/config.h +60 -2
80 lines not on this page · raw message
Signature · 1 line
2.5.0
PATCH v5 4/4 perf config: Initialize perf_config_set with all default configs
In reply to Taeung SongTo 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.
--- tools/perf/builtin-config.c | 11 +++++++---- tools/perf/util/config.c | 33 ++++++++++++++++++++++++++++++--- tools/perf/util/config.h | 7 +++++++ 3 files changed, 44 insertions(+), 7 deletions(-)
tools/perf/builtin-config.c +7 -4
@@ -35,23 +35,26 @@ static struct option config_options[] = {static int show_config (struct perf_config_set *set) { +bool has_value = false;struct perf_config_section *section;struct perf_config_item *item;struct list_head *sections = &set->sections; -if (list_empty (sections)) -return -1 ; -list_for_each_entry (section, sections, node) {list_for_each_entry (item, §ion->items, node) {char *value = item->value; -if (value) +if (value) {printf ("%s.%s=%s\n" , section->name, item->name, value); + has_value = true; + } } } +if (!has_value) +return -1 ; +return 0 ; }
tools/perf/util/config.c +30 -3
@@ -663,6 +663,7 @@ static struct perf_config_section *add_section(struct list_head *sections,if (!section)return NULL ; + section->is_custom = true;INIT_LIST_HEAD (§ion->items); section->name =strdup (section_name);if (!section->name) {
@@ -683,6 +684,7 @@ static struct perf_config_item *add_config_item(struct perf_config_section *sectif (!item)return NULL ; + item->is_custom = true; item->name =strdup (name);if (!item->name) {pr_debug ("%s: strdup failed\n" , __func__);
@@ -754,12 +756,35 @@ out_free:return -1 ; } +static struct perf_config_set *perf_config_set__init (struct perf_config_set *set) +{ +int i, j; +struct perf_config_section *section; +struct perf_config_item *items; +struct list_head *sections = &set->sections; + +INIT_LIST_HEAD (&set->sections); + +for (i =0 ; i !=CONFIG_END ; i++) { + section = &default_sections[i]; +INIT_LIST_HEAD (§ion->items); + + items = default_config_items[i]; +for (j =0 ; items[j].name !=NULL ; j++) +list_add_tail (&items[j].node, §ion->items); + +list_add_tail (§ion->node, sections); + } + +return set; +} +struct perf_config_set *perf_config_set__new (void ) {struct perf_config_set *set =zalloc (sizeof (*set));if (set) { -INIT_LIST_HEAD (&set->sections); +perf_config_set__init (set);perf_config (collect_config, set); }
@@ -779,7 +804,8 @@ static void perf_config_section__purge(struct perf_config_section *section)list_for_each_entry_safe (item, tmp, §ion->items, node) {list_del_init (&item->node); -perf_config_item__delete (item); +if (item->is_custom) +perf_config_item__delete (item); } }
@@ -796,7 +822,8 @@ static void perf_config_set__purge(struct perf_config_set *set)list_for_each_entry_safe (section, tmp, &set->sections, node) {list_del_init (§ion->node); -perf_config_section__delete (section); +if (section->is_custom) +perf_config_section__delete (section); } }
tools/perf/util/config.h +7 -0
@@ -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 *name;char *value;
@@ -27,11 +32,13 @@ struct perf_config_item {const char *s; } default_value;enum perf_config_type type; +bool is_custom;struct list_head node; };struct perf_config_section {const char *name; +bool is_custom;struct list_head items;struct list_head node; };
Signature · 1 line
2.5.0