Skip to contentSkip to conversation

Infrastructure code for perf-config

RFC PATCH Superseded Taeung Song 4 participants 14 messages Apr 14, 2016 – Apr 20, 2016 perf events

v8 of 11 Superseded Latest is v11 →

lore
Share & tools
lore

Taeung Song

author
#0 lore raw

v8 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

v8:
- rebased onto the current acme/perf/core

v7:
- rename 'is_custom' to 'is_allocated' to be proper (Masami)
- fix the code about free() or zfree() in perf_config_*__delete() (Masami)
- check set == NULL or not in show_config() (Masami)

v6:
- don't use goto in add_config_item() (Masami)

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 |  42 +++++-
 tools/perf/util/config.c    | 308 ++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/config.h    |  91 +++++++++++++
 3 files changed, 434 insertions(+), 7 deletions(-)
 create mode 100644 tools/perf/util/config.h
Signature · 1 line
2.5.0
author
#1 lore raw

PATCH v8 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.

Reviewed-by Masami Hiramatsu <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Jiri Olsa <[email protected]>
Signed-off-by Taeung Song <[email protected]>
---
 tools/perf/util/config.c | 173 +++++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/config.h |  26 +++++++
 2 files changed, 199 insertions(+)
 create mode 100644 tools/perf/util/config.h
tools/perf/util/config.c +173 -0
index 664490b..dad7d82 100644
@@ -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,178 @@ 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, &section->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(&section->items);
+	section->name = strdup(section_name);
+	if (!section->name) {
+		pr_debug("%s: strdup failed\n", __func__);
+		free(section);
+		return NULL;
+	}
+
+	list_add_tail(&section->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__);
+		free(item);
+		return NULL;
+	}
+
+	list_add_tail(&item->node, &section->items);
+	return item;
+}
+
+static int set_value(struct perf_config_item *item, const char *value)
+{
+	char *val = strdup(value);
+
+	if (!val)
+		return -1;
+
+	zfree(&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, &section->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(&section->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(&section->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
new file mode 100644
index 0000000..22ec626
@@ -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
merge bot
#2 lore raw

[tip:perf/core] perf config: Introduce perf_config_set class

perf config: Introduce perf_config_set class
Commit-ID:  20105ca1240c3d3ac8cc79bf195022e5e5c1c3fb
Gitweb:     http://git.kernel.org/tip/20105ca1240c3d3ac8cc79bf195022e5e5c1c3fb
Author:     Taeung Song <[email protected]>
AuthorDate: Thu, 14 Apr 2016 16:53:18 +0900
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Thu, 14 Apr 2016 09:00:42 -0300

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.

Reviewed-by Masami Hiramatsu <[email protected]>
Signed-off-by Taeung Song <[email protected]>
Cc Jiri Olsa <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Peter Zijlstra <[email protected]>
Link http://lkml.kernel.org/r/[email protected] open here
Signed-off-by Arnaldo Carvalho de Melo <[email protected]>
---
 tools/perf/util/config.c | 173 +++++++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/config.h |  26 +++++++
 2 files changed, 199 insertions(+)
tools/perf/util/config.c +173 -0
index 664490b..dad7d82 100644
@@ -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,178 @@ 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, &section->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(&section->items);
+	section->name = strdup(section_name);
+	if (!section->name) {
+		pr_debug("%s: strdup failed\n", __func__);
+		free(section);
+		return NULL;
+	}
+
+	list_add_tail(&section->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__);
+		free(item);
+		return NULL;
+	}
+
+	list_add_tail(&item->node, &section->items);
+	return item;
+}
+
+static int set_value(struct perf_config_item *item, const char *value)
+{
+	char *val = strdup(value);
+
+	if (!val)
+		return -1;
+
+	zfree(&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, &section->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(&section->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(&section->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
new file mode 100644
index 0000000..22ec626
@@ -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 */
author
#3 lore raw

PATCH v8 2/4 perf config: Let show_config() work with perf_config_set

In reply to 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 Masami Hiramatsu <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Jiri Olsa <[email protected]>
Signed-off-by Taeung Song <[email protected]>
---
 tools/perf/builtin-config.c | 39 ++++++++++++++++++++++++++++++++-------
 1 file changed, 32 insertions(+), 7 deletions(-)
tools/perf/builtin-config.c +32 -7
index c42448e..fe1b77f 100644
@@ -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,28 @@ 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;
+
+	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;
+
+			if (value)
+				printf("%s.%s=%s\n", section->name,
+				       item->name, value);
+		}
+	}
 
 	return 0;
 }

@@ -46,6 +62,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 +80,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 +106,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
merge bot
#4 lore raw

[tip:perf/core] perf config: Make show_config() use perf_config_set

perf config: Make show_config() use perf_config_set
Commit-ID:  860b8d4b3f893c97f905b978ecf62f48816dc5de
Gitweb:     http://git.kernel.org/tip/860b8d4b3f893c97f905b978ecf62f48816dc5de
Author:     Taeung Song <[email protected]>
AuthorDate: Thu, 14 Apr 2016 16:53:19 +0900
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Thu, 14 Apr 2016 09:15:47 -0300

perf config: Make show_config() use perf_config_set

Currently show_config() has a problem when user and system config files have the 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). Fix it.

After:

  # perf config --user --list
  top.children=false
  # perf config --system --list
  top.children=true
  # perf config --list
  top.children=false
Signed-off-by Taeung Song <[email protected]>
Tested-by Arnaldo Carvalho de Melo <[email protected]>
Cc Jiri Olsa <[email protected]>
Cc Masami Hiramatsu <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Peter Zijlstra <[email protected]>
Link http://lkml.kernel.org/r/[email protected] open here
Signed-off-by Arnaldo Carvalho de Melo <[email protected]>
---
 tools/perf/builtin-config.c | 39 ++++++++++++++++++++++++++++++++-------
 1 file changed, 32 insertions(+), 7 deletions(-)
tools/perf/builtin-config.c +32 -7
index c42448e..fe1b77f 100644
@@ -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,28 @@ 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;
+
+	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;
+
+			if (value)
+				printf("%s.%s=%s\n", section->name,
+				       item->name, value);
+		}
+	}
 
 	return 0;
 }

@@ -46,6 +62,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 +80,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 +106,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;
 }
author
#5 lore raw

PATCH v8 3/4 perf config: Prepare all default configs

In reply to 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.

Acked-by Namhyung Kim <[email protected]>
Cc Masami Hiramatsu <[email protected]>
Cc Jiri Olsa <[email protected]>
Signed-off-by Taeung Song <[email protected]>
---
 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
index dad7d82..5604392 100644
@@ -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;

@@ -659,7 +765,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);
 }

@@ -677,7 +783,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(&section->name);
+	zfree((char **)&section->name);
 	free(section);
 }
tools/perf/util/config.h +60 -2

80 lines not on this page · raw message

Signature · 1 line
2.5.0
maintainer
#6 lore raw

Re: [PATCH v8 3/4] perf config: Prepare all default configs

Em Thu, Apr 14, 2016 at 04:53:20PM +0900, Taeung Song escreveu:

Quoted 23 lines

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.

Acked-by Namhyung Kim <[email protected]>
Cc Masami Hiramatsu <[email protected]>
Cc Jiri Olsa <[email protected]>
Signed-off-by Taeung Song <[email protected]>
---
 tools/perf/util/config.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++-
 tools/perf/util/config.h |  62 +++++++++++++++++++++++++-
 2 files changed, 168 insertions(+), 4 deletions(-)
tools/perf/util/config.c +0 -0
index dad7d82..5604392 100644

tools/perf/util/config.c line 18

Quoted 4 lines
tools/perf/util/config.c +1 -0
@@ -15,6 +15,7 @@
 #include "util/llvm-utils.h"   /* perf_llvm_config */
 #include "config.h"
 
+#define MAX_CONFIGS 64

Do we have to add another arbitrary maximum? Where is it used?

Quoted 221 lines
tools/perf/util/config.c +107 -2
 #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;

@@ -659,7 +765,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);
 }

@@ -677,7 +783,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(&section->name);
+	zfree((char **)&section->name);
 	free(section);
 }
tools/perf/util/config.h +60 -2
index 22ec626..84dcc1d 100644
@@ -4,14 +4,34 @@
 #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 *name;
+	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 node;
 };
 
 struct perf_config_section {
-	char *name;
+	const char *name;
 	struct list_head items;
 	struct list_head node;
 };

@@ -20,6 +40,44 @@ struct perf_config_set {
 	struct list_head sections;
 };
 
+enum perf_config_secion_idx {
+	CONFIG_COLORS,
+	CONFIG_TUI,
+	CONFIG_BUILDID,
+	CONFIG_ANNOTATE,
+	CONFIG_GTK,
+	CONFIG_PAGER,
+	CONFIG_HELP,
+	CONFIG_HIST,
+	CONFIG_UI,
+	CONFIG_CALL_GRAPH,
+	CONFIG_REPORT,
+	CONFIG_TOP,
+	CONFIG_MAN,
+	CONFIG_KMEM,
+	CONFIG_END
+};
+
+#define CONF_VAR(_name, _field, _val, _type)				\
+	{ .name = _name, .default_value._field = _val, .type = _type }
+
+#define CONF_BOOL_VAR(_name, _val)			\
+	CONF_VAR(_name, b, _val, CONFIG_TYPE_BOOL)
+#define CONF_INT_VAR(_name, _val)			\
+	CONF_VAR(_name, i, _val, CONFIG_TYPE_INT)
+#define CONF_LONG_VAR(_name, _val)			\
+	CONF_VAR(_name, l, _val, CONFIG_TYPE_LONG)
+#define CONF_U64_VAR(_name, _val)			\
+	CONF_VAR(_name, ll, _val, CONFIG_TYPE_U64)
+#define CONF_FLOAT_VAR(_name, _val)			\
+	CONF_VAR(_name, f, _val, CONFIG_TYPE_FLOAT)
+#define CONF_DOUBLE_VAR(_name, _val)			\
+	CONF_VAR(_name, d, _val, CONFIG_TYPE_DOUBLE)
+#define CONF_STR_VAR(_name, _val)			\
+	CONF_VAR(_name, s, _val, CONFIG_TYPE_STRING)
+#define CONF_END()					\
+	{ .name = NULL }
+
 struct perf_config_set *perf_config_set__new(void);
 void perf_config_set__delete(struct perf_config_set *set);
Signature · 1 line
2.5.0
author
#7 lore raw

Re: [PATCH v8 3/4] perf config: Prepare all default configs

Hi, Arnaldo

Quoted 27 lines · Arnaldo Carvalho de Melo

Em Thu, Apr 14, 2016 at 04:53:20PM +0900, Taeung Song escreveu:

Quoted 26 lines

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.

Acked-by Namhyung Kim <[email protected]>
Cc Masami Hiramatsu <[email protected]>
Cc Jiri Olsa <[email protected]>
Signed-off-by Taeung Song <[email protected]>
---
  tools/perf/util/config.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++-
  tools/perf/util/config.h |  62 +++++++++++++++++++++++++-
  2 files changed, 168 insertions(+), 4 deletions(-)
tools/perf/util/config.c +0 -0
index dad7d82..5604392 100644
@@ -15,6 +15,7 @@
  #include "util/llvm-utils.h"   /* perf_llvm_config */
  #include "config.h"
Quoted 4 lines

tools/perf/util/config.c line 18

Quoted 1 line
tools/perf/util/config.c +1 -0
+#define MAX_CONFIGS 64

Do we have to add another arbitrary maximum? Where is it used?

IMHO, it is my idea. If you want to avoid using this arbitrary maxinum, I'd modify the code.

MAX_CONFIGS is used in order to declare two-dimensional arrays 'default_config_items' as below.

struct perf_config_item default_config_items[][MAX_CONFIGS] = {
	[CONFIG_COLORS] = {
		CONF_STR_VAR("top", "red, default"),
		CONF_STR_VAR("medium", "green, default"),

...(omitted)...

IMHO, it is because of perf_config_set__init() in [PATCH v8 4/4]. If we don't use two-dimensional arrays, I think the code of perf_config_set__init() could be complicated.

Thanks,
Taeung

Quoted 221 lines
Quoted 221 lines
tools/perf/util/config.c +107 -2
  #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;

@@ -659,7 +765,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);
  }

@@ -677,7 +783,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(&section->name);
+	zfree((char **)&section->name);
  	free(section);
  }
tools/perf/util/config.h +60 -3
index 22ec626..84dcc1d 100644
@@ -4,14 +4,34 @@
  #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 *name;
+	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 node;
  };
 
  struct perf_config_section {
-	char *name;
+	const char *name;
  	struct list_head items;
  	struct list_head node;
  };

@@ -20,6 +40,44 @@ struct perf_config_set {
  	struct list_head sections;
  };
 
+enum perf_config_secion_idx {
+	CONFIG_COLORS,
+	CONFIG_TUI,
+	CONFIG_BUILDID,
+	CONFIG_ANNOTATE,
+	CONFIG_GTK,
+	CONFIG_PAGER,
+	CONFIG_HELP,
+	CONFIG_HIST,
+	CONFIG_UI,
+	CONFIG_CALL_GRAPH,
+	CONFIG_REPORT,
+	CONFIG_TOP,
+	CONFIG_MAN,
+	CONFIG_KMEM,
+	CONFIG_END
+};
+
+#define CONF_VAR(_name, _field, _val, _type)				\
+	{ .name = _name, .default_value._field = _val, .type = _type }
+
+#define CONF_BOOL_VAR(_name, _val)			\
+	CONF_VAR(_name, b, _val, CONFIG_TYPE_BOOL)
+#define CONF_INT_VAR(_name, _val)			\
+	CONF_VAR(_name, i, _val, CONFIG_TYPE_INT)
+#define CONF_LONG_VAR(_name, _val)			\
+	CONF_VAR(_name, l, _val, CONFIG_TYPE_LONG)
+#define CONF_U64_VAR(_name, _val)			\
+	CONF_VAR(_name, ll, _val, CONFIG_TYPE_U64)
+#define CONF_FLOAT_VAR(_name, _val)			\
+	CONF_VAR(_name, f, _val, CONFIG_TYPE_FLOAT)
+#define CONF_DOUBLE_VAR(_name, _val)			\
+	CONF_VAR(_name, d, _val, CONFIG_TYPE_DOUBLE)
+#define CONF_STR_VAR(_name, _val)			\
+	CONF_VAR(_name, s, _val, CONFIG_TYPE_STRING)
+#define CONF_END()					\
+	{ .name = NULL }
+
  struct perf_config_set *perf_config_set__new(void);
  void perf_config_set__delete(struct perf_config_set *set);
 
--

2.5.0

author
#8 lore raw

Re: [PATCH v8 3/4] perf config: Prepare all default configs

Hi, Namhyung

Quoted 47 lines · Taeung Song

Hi, Arnaldo

Quoted 28 lines · Arnaldo Carvalho de Melo

Em Thu, Apr 14, 2016 at 04:53:20PM +0900, Taeung Song escreveu:

Quoted 27 lines

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.

Acked-by Namhyung Kim <[email protected]>
Cc Masami Hiramatsu <[email protected]>
Cc Jiri Olsa <[email protected]>
Signed-off-by Taeung Song <[email protected]>
---
  tools/perf/util/config.c | 110
++++++++++++++++++++++++++++++++++++++++++++++-
  tools/perf/util/config.h |  62 +++++++++++++++++++++++++-
  2 files changed, 168 insertions(+), 4 deletions(-)
tools/perf/util/config.c +0 -0
index dad7d82..5604392 100644
@@ -15,6 +15,7 @@
  #include "util/llvm-utils.h"   /* perf_llvm_config */
  #include "config.h"
Quoted 4 lines

tools/perf/util/config.c line 18

Quoted 1 line
tools/perf/util/config.c +1 -0
+#define MAX_CONFIGS 64

Do we have to add another arbitrary maximum? Where is it used?

IMHO, it is my idea. If you want to avoid using this arbitrary maxinum, I'd modify the code.

MAX_CONFIGS is used in order to declare two-dimensional arrays 'default_config_items'
as below.

struct perf_config_item default_config_items[][MAX_CONFIGS] = {
     [CONFIG_COLORS] = {
         CONF_STR_VAR("top", "red, default"),
         CONF_STR_VAR("medium", "green, default"),
Quoted 6 lines

...(omitted)...

IMHO, it is because of perf_config_set__init() in [PATCH v8 4/4]. If we don't use two-dimensional arrays, I think the code of perf_config_set__init() could be complicated.

As above, I used MAX_CONFIGS because of two-dimensinal arrays 'default_config_items'.

What do you think about it ?

We don't need to add this arbitrary maximum ?
or would you mind, if I look for other way about 'default_config_item' ?

Thanks,
Taeung

Quoted 223 lines
Quoted 223 lines
Quoted 223 lines
tools/perf/util/config.c +107 -2
  #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;

@@ -659,7 +765,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);
  }

@@ -677,7 +783,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(&section->name);
+    zfree((char **)&section->name);
      free(section);

  }
tools/perf/util/config.h +60 -3
index 22ec626..84dcc1d 100644
@@ -4,14 +4,34 @@
  #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 *name;
+    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 node;
  };
 
  struct perf_config_section {
-    char *name;
+    const char *name;
      struct list_head items;
      struct list_head node;
  };

@@ -20,6 +40,44 @@ struct perf_config_set {
      struct list_head sections;
  };
 
+enum perf_config_secion_idx {
+    CONFIG_COLORS,
+    CONFIG_TUI,
+    CONFIG_BUILDID,
+    CONFIG_ANNOTATE,
+    CONFIG_GTK,
+    CONFIG_PAGER,
+    CONFIG_HELP,
+    CONFIG_HIST,
+    CONFIG_UI,
+    CONFIG_CALL_GRAPH,
+    CONFIG_REPORT,
+    CONFIG_TOP,
+    CONFIG_MAN,
+    CONFIG_KMEM,
+    CONFIG_END
+};
+
+#define CONF_VAR(_name, _field, _val, _type)                \
+    { .name = _name, .default_value._field = _val, .type = _type }
+
+#define CONF_BOOL_VAR(_name, _val)            \
+    CONF_VAR(_name, b, _val, CONFIG_TYPE_BOOL)
+#define CONF_INT_VAR(_name, _val)            \
+    CONF_VAR(_name, i, _val, CONFIG_TYPE_INT)
+#define CONF_LONG_VAR(_name, _val)            \
+    CONF_VAR(_name, l, _val, CONFIG_TYPE_LONG)
+#define CONF_U64_VAR(_name, _val)            \
+    CONF_VAR(_name, ll, _val, CONFIG_TYPE_U64)
+#define CONF_FLOAT_VAR(_name, _val)            \
+    CONF_VAR(_name, f, _val, CONFIG_TYPE_FLOAT)
+#define CONF_DOUBLE_VAR(_name, _val)            \
+    CONF_VAR(_name, d, _val, CONFIG_TYPE_DOUBLE)
+#define CONF_STR_VAR(_name, _val)            \
+    CONF_VAR(_name, s, _val, CONFIG_TYPE_STRING)
+#define CONF_END()                    \
+    { .name = NULL }
+
  struct perf_config_set *perf_config_set__new(void);
  void perf_config_set__delete(struct perf_config_set *set);
 
--

2.5.0

maintainer
#9 lore raw

Re: [PATCH v8 3/4] perf config: Prepare all default configs

Hi Taeung,

Quoted 57 lines · Taeung Song

Hi, Namhyung

Quoted 47 lines · Taeung Song

Hi, Arnaldo

Quoted 28 lines · Arnaldo Carvalho de Melo

Em Thu, Apr 14, 2016 at 04:53:20PM +0900, Taeung Song escreveu:

Quoted 27 lines

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.

Acked-by Namhyung Kim <[email protected]>
Cc Masami Hiramatsu <[email protected]>
Cc Jiri Olsa <[email protected]>
Signed-off-by Taeung Song <[email protected]>
---
  tools/perf/util/config.c | 110
++++++++++++++++++++++++++++++++++++++++++++++-
  tools/perf/util/config.h |  62 +++++++++++++++++++++++++-
  2 files changed, 168 insertions(+), 4 deletions(-)
tools/perf/util/config.c +0 -0
index dad7d82..5604392 100644
@@ -15,6 +15,7 @@
  #include "util/llvm-utils.h"   /* perf_llvm_config */
  #include "config.h"
Quoted 4 lines

tools/perf/util/config.c line 18

Quoted 1 line
tools/perf/util/config.c +1 -0
+#define MAX_CONFIGS 64

Do we have to add another arbitrary maximum? Where is it used?

IMHO, it is my idea. If you want to avoid using this arbitrary maxinum, I'd modify the code.

MAX_CONFIGS is used in order to declare two-dimensional arrays 'default_config_items'
as below.

struct perf_config_item default_config_items[][MAX_CONFIGS] = {
     [CONFIG_COLORS] = {
         CONF_STR_VAR("top", "red, default"),
         CONF_STR_VAR("medium", "green, default"),
Quoted 6 lines

...(omitted)...

IMHO, it is because of perf_config_set__init() in [PATCH v8 4/4]. If we don't use two-dimensional arrays, I think the code of perf_config_set__init() could be complicated.

Quoted 4 lines

As above, I used MAX_CONFIGS because of two-dimensinal arrays 'default_config_items'.

What do you think about it ?

I also agree that we'd better to avoid the arbitrary maximum.

Quoted 4 lines

We don't need to add this arbitrary maximum ?
or would you mind, if I look for other way about
'default_config_item' ?

What about this?

struct perf_config_item color_config_items[] = {
	CONF_STR_VAR("top", "red, default"),
	CONF_STR_VAR("medium", "green, default"),
	...
};
struct perf_config_item tui_config_items[] = {
	CONF_BOOL_VAR("report", true),
	CONF_BOOL_VAR("annotate", true),
	...
};
struct perf_config_item *default_config_items[] = {
	&color_config_items,
	&tui_config_items,
	...
};

This way we can access the config array by using constant index without the hard-coded maximum size IMHO.

Thanks,
Namhyung

Quoted 245 lines
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
tools/perf/util/config.c +8 -0
+    [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"),
Quoted 8 lines
Quoted 8 lines

tools/perf/util/config.c static int config_file_eof; · line 66

Quoted 8 lines
tools/perf/util/config.c +8 -0
+        CONF_END()
+    },
+    [CONFIG_TUI] = {
+        CONF_BOOL_VAR("report", true),
+        CONF_BOOL_VAR("annotate", true),
+        CONF_BOOL_VAR("top", true),
+        CONF_END()
+    },

Thanks,
Taeung

Quoted 223 lines
Quoted 223 lines
Quoted 223 lines
  #define MAXNAME (256)

  #define DEBUG_CACHE_DIR ".debug"
tools/perf/util/config.c +107 -2
@@ -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;

@@ -659,7 +765,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);
  }

@@ -677,7 +783,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(&section->name);
+    zfree((char **)&section->name);
      free(section);

  }
tools/perf/util/config.h +60 -3
index 22ec626..84dcc1d 100644
@@ -4,14 +4,34 @@
  #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 *name;
+    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 node;
  };
 
  struct perf_config_section {
-    char *name;
+    const char *name;
      struct list_head items;
      struct list_head node;
  };

@@ -20,6 +40,44 @@ struct perf_config_set {
      struct list_head sections;
  };
 
+enum perf_config_secion_idx {
+    CONFIG_COLORS,
+    CONFIG_TUI,
+    CONFIG_BUILDID,
+    CONFIG_ANNOTATE,
+    CONFIG_GTK,
+    CONFIG_PAGER,
+    CONFIG_HELP,
+    CONFIG_HIST,
+    CONFIG_UI,
+    CONFIG_CALL_GRAPH,
+    CONFIG_REPORT,
+    CONFIG_TOP,
+    CONFIG_MAN,
+    CONFIG_KMEM,
+    CONFIG_END
+};
+
+#define CONF_VAR(_name, _field, _val, _type)                \
+    { .name = _name, .default_value._field = _val, .type = _type }
+
+#define CONF_BOOL_VAR(_name, _val)            \
+    CONF_VAR(_name, b, _val, CONFIG_TYPE_BOOL)
+#define CONF_INT_VAR(_name, _val)            \
+    CONF_VAR(_name, i, _val, CONFIG_TYPE_INT)
+#define CONF_LONG_VAR(_name, _val)            \
+    CONF_VAR(_name, l, _val, CONFIG_TYPE_LONG)
+#define CONF_U64_VAR(_name, _val)            \
+    CONF_VAR(_name, ll, _val, CONFIG_TYPE_U64)
+#define CONF_FLOAT_VAR(_name, _val)            \
+    CONF_VAR(_name, f, _val, CONFIG_TYPE_FLOAT)
+#define CONF_DOUBLE_VAR(_name, _val)            \
+    CONF_VAR(_name, d, _val, CONFIG_TYPE_DOUBLE)
+#define CONF_STR_VAR(_name, _val)            \
+    CONF_VAR(_name, s, _val, CONFIG_TYPE_STRING)
+#define CONF_END()                    \
+    { .name = NULL }
+
  struct perf_config_set *perf_config_set__new(void);
  void perf_config_set__delete(struct perf_config_set *set);
 
--

2.5.0

maintainer
#10 lore raw

Re: [PATCH v8 3/4] perf config: Prepare all default configs

Em Wed, Apr 20, 2016 at 09:44:38PM +0900, Namhyung Kim escreveu:

Quoted 21 lines · Taeung Song
Quoted 12 lines · Taeung Song
Quoted 11 lines · Arnaldo Carvalho de Melo
Quoted 6 lines

Em Thu, Apr 14, 2016 at 04:53:20PM +0900, Taeung Song escreveu:

tools/perf/util/config.c line 15

Quoted 3 lines

+++ b/tools/perf/util/config.c

tools/perf/util/config.c +1 -0
@@ -15,6 +15,7 @@
+#define MAX_CONFIGS 64

Do we have to add another arbitrary maximum? Where is it used?

IMHO, it is my idea. If you want to avoid using this arbitrary maxinum, I'd modify the code.

Quoted 7 lines
Quoted 2 lines

MAX_CONFIGS is used in order to declare two-dimensional arrays 'default_config_items'

As above, I used MAX_CONFIGS because of two-dimensinal arrays 'default_config_items'.

What do you think about it ?

Quoted 7 lines

I also agree that we'd better to avoid the arbitrary maximum.

Quoted 3 lines

We don't need to add this arbitrary maximum ?
or would you mind, if I look for other way about
'default_config_item' ?

What about this?

Yeah, I guess this should work, no? At least this is how it is done elsewhere, see:

tools/perf/builtin-bench.c, 'struct collection' has a benchmarks array, that in turn is organized as Namhyung suggests.

Then you either use ARRAY_SIZE() somewhere to get the number of entries or use a sentinel, i.e. use NULL for the last entry.

- Arnaldo

Quoted 23 lines
struct perf_config_item color_config_items[] = {
	CONF_STR_VAR("top", "red, default"),
	CONF_STR_VAR("medium", "green, default"),
	...
};
struct perf_config_item tui_config_items[] = {
	CONF_BOOL_VAR("report", true),
	CONF_BOOL_VAR("annotate", true),
	...
};
struct perf_config_item *default_config_items[] = {
	&color_config_items,
	&tui_config_items,
	...
};

This way we can access the config array by using constant index without the hard-coded maximum size IMHO.

Thanks,
Namhyung

author
#11 lore raw

Re: [PATCH v8 3/4] perf config: Prepare all default configs

Hi, Arnaldo :-)

Quoted 28 lines · Arnaldo Carvalho de Melo

Em Wed, Apr 20, 2016 at 09:44:38PM +0900, Namhyung Kim escreveu:

Quoted 27 lines · Taeung Song
Quoted 12 lines · Taeung Song
Quoted 11 lines · Arnaldo Carvalho de Melo
Quoted 6 lines

Em Thu, Apr 14, 2016 at 04:53:20PM +0900, Taeung Song escreveu:

tools/perf/util/config.c line 15

Quoted 3 lines

+++ b/tools/perf/util/config.c

tools/perf/util/config.c +1 -0
@@ -15,6 +15,7 @@
+#define MAX_CONFIGS 64

Do we have to add another arbitrary maximum? Where is it used?

IMHO, it is my idea. If you want to avoid using this arbitrary maxinum, I'd modify the code.

Quoted 7 lines
Quoted 2 lines

MAX_CONFIGS is used in order to declare two-dimensional arrays 'default_config_items'

As above, I used MAX_CONFIGS because of two-dimensinal arrays 'default_config_items'.

What do you think about it ?

I also agree that we'd better to avoid the arbitrary maximum.

Quoted 3 lines

We don't need to add this arbitrary maximum ?
or would you mind, if I look for other way about
'default_config_item' ?

Quoted 7 lines
Quoted 1 line

What about this?

Yeah, I guess this should work, no? At least this is how it is done elsewhere, see:

tools/perf/builtin-bench.c, 'struct collection' has a benchmarks array, that in turn is organized as Namhyung suggests.

I got it!

Quoted 3 lines

Then you either use ARRAY_SIZE() somewhere to get the number of entries or use a sentinel, i.e. use NULL for the last entry.

I think that it would better to use ARRAY_SIZE() than to use NULL. It makes no odds but it seem to be clean, IMHO. :)

After changing this patchset, I'll send v9 !

Thanks,
Taeung

Quoted 24 lines
Quoted 24 lines
struct perf_config_item color_config_items[] = {
	CONF_STR_VAR("top", "red, default"),
	CONF_STR_VAR("medium", "green, default"),
	...
};
struct perf_config_item tui_config_items[] = {
	CONF_BOOL_VAR("report", true),
	CONF_BOOL_VAR("annotate", true),
	...
};
struct perf_config_item *default_config_items[] = {
	&color_config_items,
	&tui_config_items,
	...
};

This way we can access the config array by using constant index without the hard-coded maximum size IMHO.

Thanks,
Namhyung

author
#12 lore raw

Re: [PATCH v8 3/4] perf config: Prepare all default configs

In reply to Namhyung Kim

Hi, Namhyung

Quoted 86 lines · Namhyung Kim

Hi Taeung,

Quoted 57 lines · Taeung Song

Hi, Namhyung

Quoted 47 lines · Taeung Song

Hi, Arnaldo

Quoted 28 lines · Arnaldo Carvalho de Melo

Em Thu, Apr 14, 2016 at 04:53:20PM +0900, Taeung Song escreveu:

Quoted 27 lines

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.

Acked-by Namhyung Kim <[email protected]>
Cc Masami Hiramatsu <[email protected]>
Cc Jiri Olsa <[email protected]>
Signed-off-by Taeung Song <[email protected]>
---
   tools/perf/util/config.c | 110
++++++++++++++++++++++++++++++++++++++++++++++-
   tools/perf/util/config.h |  62 +++++++++++++++++++++++++-
   2 files changed, 168 insertions(+), 4 deletions(-)
tools/perf/util/config.c +0 -0
index dad7d82..5604392 100644
@@ -15,6 +15,7 @@
   #include "util/llvm-utils.h"   /* perf_llvm_config */
   #include "config.h"
Quoted 4 lines

tools/perf/util/config.c line 18

Quoted 1 line
tools/perf/util/config.c +1 -0
+#define MAX_CONFIGS 64

Do we have to add another arbitrary maximum? Where is it used?

IMHO, it is my idea. If you want to avoid using this arbitrary maxinum, I'd modify the code.

MAX_CONFIGS is used in order to declare two-dimensional arrays 'default_config_items'
as below.

struct perf_config_item default_config_items[][MAX_CONFIGS] = {
      [CONFIG_COLORS] = {
          CONF_STR_VAR("top", "red, default"),
          CONF_STR_VAR("medium", "green, default"),
Quoted 6 lines

...(omitted)...

IMHO, it is because of perf_config_set__init() in [PATCH v8 4/4]. If we don't use two-dimensional arrays, I think the code of perf_config_set__init() could be complicated.

Quoted 4 lines

As above, I used MAX_CONFIGS because of two-dimensinal arrays 'default_config_items'.

What do you think about it ?

I also agree that we'd better to avoid the arbitrary maximum.

Quoted 4 lines

We don't need to add this arbitrary maximum ?
or would you mind, if I look for other way about
'default_config_item' ?

What about this?

struct perf_config_item color_config_items[] = {
	CONF_STR_VAR("top", "red, default"),
	CONF_STR_VAR("medium", "green, default"),
	...
};
struct perf_config_item tui_config_items[] = {
	CONF_BOOL_VAR("report", true),
	CONF_BOOL_VAR("annotate", true),
	...
};
Quoted 9 lines
struct perf_config_item *default_config_items[] = {
	&color_config_items,
	&tui_config_items,
	...
};

This way we can access the config array by using constant index without the hard-coded maximum size IMHO.

OK, I got it.
I was hesitating a bit but I'll change this patch by this way you told. :-)

Thanks,
Taeung

Quoted 247 lines
Quoted 247 lines
Quoted 18 lines
Quoted 18 lines
Quoted 8 lines
tools/perf/util/config.c +6 -0
+    [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"),

tools/perf/util/config.c static int config_file_eof; · line 66

Quoted 10 lines
tools/perf/util/config.c +10 -0
+        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()
+    },

Thanks,
Taeung

Quoted 223 lines
Quoted 223 lines
Quoted 223 lines
   #define MAXNAME (256)

   #define DEBUG_CACHE_DIR ".debug"
tools/perf/util/config.c +107 -2
@@ -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;

@@ -659,7 +765,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);
   }

@@ -677,7 +783,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(&section->name);
+    zfree((char **)&section->name);
       free(section);

   }
tools/perf/util/config.h +60 -3
index 22ec626..84dcc1d 100644
@@ -4,14 +4,34 @@
   #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 *name;
+    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 node;
   };
 
   struct perf_config_section {
-    char *name;
+    const char *name;
       struct list_head items;
       struct list_head node;
   };

@@ -20,6 +40,44 @@ struct perf_config_set {
       struct list_head sections;
   };
 
+enum perf_config_secion_idx {
+    CONFIG_COLORS,
+    CONFIG_TUI,
+    CONFIG_BUILDID,
+    CONFIG_ANNOTATE,
+    CONFIG_GTK,
+    CONFIG_PAGER,
+    CONFIG_HELP,
+    CONFIG_HIST,
+    CONFIG_UI,
+    CONFIG_CALL_GRAPH,
+    CONFIG_REPORT,
+    CONFIG_TOP,
+    CONFIG_MAN,
+    CONFIG_KMEM,
+    CONFIG_END
+};
+
+#define CONF_VAR(_name, _field, _val, _type)                \
+    { .name = _name, .default_value._field = _val, .type = _type }
+
+#define CONF_BOOL_VAR(_name, _val)            \
+    CONF_VAR(_name, b, _val, CONFIG_TYPE_BOOL)
+#define CONF_INT_VAR(_name, _val)            \
+    CONF_VAR(_name, i, _val, CONFIG_TYPE_INT)
+#define CONF_LONG_VAR(_name, _val)            \
+    CONF_VAR(_name, l, _val, CONFIG_TYPE_LONG)
+#define CONF_U64_VAR(_name, _val)            \
+    CONF_VAR(_name, ll, _val, CONFIG_TYPE_U64)
+#define CONF_FLOAT_VAR(_name, _val)            \
+    CONF_VAR(_name, f, _val, CONFIG_TYPE_FLOAT)
+#define CONF_DOUBLE_VAR(_name, _val)            \
+    CONF_VAR(_name, d, _val, CONFIG_TYPE_DOUBLE)
+#define CONF_STR_VAR(_name, _val)            \
+    CONF_VAR(_name, s, _val, CONFIG_TYPE_STRING)
+#define CONF_END()                    \
+    { .name = NULL }
+
   struct perf_config_set *perf_config_set__new(void);
   void perf_config_set__delete(struct perf_config_set *set);
 
--

2.5.0

author
#13 lore raw

PATCH v8 4/4 perf config: Initialize perf_config_set with all default configs

In reply to 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 Masami Hiramatsu <[email protected]>
Cc Namhyung Kim <[email protected]>
Cc Jiri Olsa <[email protected]>
Signed-off-by Taeung Song <[email protected]>
---
 tools/perf/builtin-config.c | 11 +++++++----
 tools/perf/util/config.c    | 39 ++++++++++++++++++++++++++++++++++-----
 tools/perf/util/config.h    |  7 +++++++
 3 files changed, 48 insertions(+), 9 deletions(-)
tools/perf/builtin-config.c +7 -4
index fe1b77f..0fe9bc5 100644
@@ -35,6 +35,7 @@ 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;

@@ -43,19 +44,21 @@ static int show_config(struct perf_config_set *set)
 		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;
 
-			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 +34 -5
index 5604392..61af657 100644
@@ -663,6 +663,7 @@ static struct perf_config_section *add_section(struct list_head *sections,
 	if (!section)
 		return NULL;
 
+	section->is_allocated = true;
 	INIT_LIST_HEAD(&section->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 *sect
 	if (!item)
 		return NULL;
 
+	item->is_allocated = true;
 	item->name = strdup(name);
 	if (!item->name) {
 		pr_debug("%s: strdup failed\n", __func__);

@@ -751,12 +753,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(&section->items);
+
+		items = default_config_items[i];
+		for (j = 0; items[j].name != NULL; j++)
+			list_add_tail(&items[j].node, &section->items);
+
+		list_add_tail(&section->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);
 	}
 

@@ -765,9 +790,11 @@ struct perf_config_set *perf_config_set__new(void)
 
 static void perf_config_item__delete(struct perf_config_item *item)
 {
-	zfree((char **)&item->name);
 	zfree(&item->value);
-	free(item);
+	if (item->is_allocated) {
+		zfree((char **)&item->name);
+		free(item);
+	}
 }
 
 static void perf_config_section__purge(struct perf_config_section *section)

@@ -783,8 +810,10 @@ 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((char **)&section->name);
-	free(section);
+	if (section->is_allocated) {
+		zfree((char **)&section->name);
+		free(section);
+	}
 }
 
 static void perf_config_set__purge(struct perf_config_set *set)
tools/perf/util/config.h +7 -0
index 84dcc1d..35c0075 100644
@@ -14,6 +14,11 @@ enum perf_config_type {
 	CONFIG_TYPE_STRING
 };
 
+/**
+ * struct perf_config_item - element of perf's configs
+ *
+ * @is_allocated: 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_allocated;
 	struct list_head node;
 };
 
 struct perf_config_section {
 	const char *name;
+	bool is_allocated;
 	struct list_head items;
 	struct list_head node;
 };
Signature · 1 line
2.5.0