Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751989AbcDTPLz (ORCPT ); Wed, 20 Apr 2016 11:11:55 -0400 Received: from mail-pf0-f194.google.com ([209.85.192.194]:35855 "EHLO mail-pf0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750928AbcDTPLx (ORCPT ); Wed, 20 Apr 2016 11:11:53 -0400 Subject: Re: [PATCH v8 3/4] perf config: Prepare all default configs To: Namhyung Kim References: <1460620401-23430-1-git-send-email-treeze.taeung@gmail.com> <1460620401-23430-4-git-send-email-treeze.taeung@gmail.com> <20160414121916.GJ9056@kernel.org> <570FC86D.5060909@gmail.com> <5714F556.1000308@gmail.com> <20160420124438.GA29186@danjae.aot.lge.com> Cc: Arnaldo Carvalho de Melo , linux-kernel@vger.kernel.org, Jiri Olsa , Ingo Molnar , Peter Zijlstra , Masami Hiramatsu From: Taeung Song Message-ID: <57179C35.7050204@gmail.com> Date: Thu, 21 Apr 2016 00:11:49 +0900 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.6.0 MIME-Version: 1.0 In-Reply-To: <20160420124438.GA29186@danjae.aot.lge.com> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 11360 Lines: 352 Hi, Namhyung On 04/20/2016 09:44 PM, Namhyung Kim wrote: > Hi Taeung, > > On Mon, Apr 18, 2016 at 11:55:18PM +0900, Taeung Song wrote: >> Hi, Namhyung >> >> On 04/15/2016 01:42 AM, Taeung Song wrote: >>> Hi, Arnaldo >>> >>> On 04/14/2016 09:19 PM, Arnaldo Carvalho de Melo wrote: >>>> Em Thu, Apr 14, 2016 at 04:53:20PM +0900, Taeung Song escreveu: >>>>> 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 >>>>> Cc: Masami Hiramatsu >>>>> Cc: Jiri Olsa >>>>> Signed-off-by: Taeung Song >>>>> --- >>>>> tools/perf/util/config.c | 110 >>>>> ++++++++++++++++++++++++++++++++++++++++++++++- >>>>> tools/perf/util/config.h | 62 +++++++++++++++++++++++++- >>>>> 2 files changed, 168 insertions(+), 4 deletions(-) >>>>> >>>>> diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c >>>>> index dad7d82..5604392 100644 >>>>> --- a/tools/perf/util/config.c >>>>> +++ b/tools/perf/util/config.c >>>>> @@ -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? >>>> >>> >>> 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. >>> >> >> 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. > >> >> 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. > OK, I got it. I was hesitating a bit but I'll change this patch by this way you told. :-) Thanks, Taeung > > >>>>> + [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() >>>>> + }, > > >> >> Thanks, >> Taeung >> >>>>> #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(§ion->name); >>>>> + zfree((char **)§ion->name); >>>>> free(section); >>>>> } >>>>> >>>>> diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h >>>>> index 22ec626..84dcc1d 100644 >>>>> --- a/tools/perf/util/config.h >>>>> +++ b/tools/perf/util/config.h >>>>> @@ -4,14 +4,34 @@ >>>>> #include >>>>> #include >>>>> >>>>> +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