From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965680AbcKOCUz (ORCPT ); Mon, 14 Nov 2016 21:20:55 -0500 Received: from mail-pg0-f66.google.com ([74.125.83.66]:35479 "EHLO mail-pg0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S964788AbcKOCUx (ORCPT ); Mon, 14 Nov 2016 21:20:53 -0500 Subject: Re: [PATCH 09/15] perf config: Add support setting variables in a config file To: Arnaldo Carvalho de Melo , Ingo Molnar References: <1479173927-24613-1-git-send-email-acme@kernel.org> <1479173927-24613-10-git-send-email-acme@kernel.org> Cc: linux-kernel@vger.kernel.org, Jiri Olsa , Nambong Ha , Namhyung Kim , Peter Zijlstra , Wang Nan , Wookje Kwon , Arnaldo Carvalho de Melo From: Taeung Song Message-ID: Date: Tue, 15 Nov 2016 11:20:46 +0900 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <1479173927-24613-10-git-send-email-acme@kernel.org> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi, Arnaldo :) On 11/15/2016 10:38 AM, Arnaldo Carvalho de Melo wrote: > From: Taeung Song > > Add setting feature that can add config variables with their values to a > config file (i.e. user or system config file) or modify config key-value > pairs in a config file. For the syntax examples: > > perf config [] [section.name[=value] ...] > > e.g. You can set the ui.show-headers to false with > > # perf config ui.show-headers=false > > If you want to add or modify several config items, you can do like > > # perf config annotate.show_nr_jumps=false kmem.default=slab > > Committer notes: > > Testing it: > > $ perf config -l > top.children=true > report.children=false > $ > $ perf config top.children=false > $ perf config -l > top.children=false > report.children=false > $ > $ perf config kmem.default=slab > $ perf config -l > top.children=false > report.children=false > kmem.default=slab > $ > > Signed-off-by: Taeung Song > Tested-by: Arnaldo Carvalho de Melo > Cc: Jiri Olsa > Cc: Nambong Ha > Cc: Namhyung Kim > Cc: Peter Zijlstra > Cc: Wang Nan > Cc: Wookje Kwon > Link: http://lkml.kernel.org/r/1478241862-31230-5-git-send-email-treeze.taeung@gmail.com > [ Combined patch with docs update with this one ] > Signed-off-by: Arnaldo Carvalho de Melo > > Signed-off-by: Arnaldo Carvalho de Melo > --- > tools/perf/Documentation/perf-config.txt | 19 ++++++++- > tools/perf/builtin-config.c | 68 +++++++++++++++++++++++++++++--- > tools/perf/util/config.c | 6 +++ > tools/perf/util/config.h | 2 + > 4 files changed, 88 insertions(+), 7 deletions(-) > > diff --git a/tools/perf/Documentation/perf-config.txt b/tools/perf/Documentation/perf-config.txt > index 1714b0c8c8e1..9365b75fd04f 100644 > --- a/tools/perf/Documentation/perf-config.txt > +++ b/tools/perf/Documentation/perf-config.txt > @@ -8,7 +8,7 @@ perf-config - Get and set variables in a configuration file. > SYNOPSIS > -------- > [verse] > -'perf config' [] [section.name ...] > +'perf config' [] [section.name[=value] ...] > or > 'perf config' [] -l | --list > > @@ -120,6 +120,23 @@ Given a $HOME/.perfconfig like this: > children = true > group = true > > +You can hide source code of annotate feature setting the config to false with > + > + % perf config annotate.hide_src_code=true > + > +If you want to add or modify several config items, you can do like > + > + % perf config ui.show-headers=false kmem.default=slab > + > +To modify the sort order of report functionality in user config file(i.e. `~/.perfconfig`), do > + > + % perf config --user report sort-order=srcline > + > +To change colors of selected line to other foreground and background colors > +in system config file (i.e. `$(sysconf)/perfconfig`), do > + > + % perf config --system colors.selected=yellow,green > + > To query the record mode of call graph, do > > % perf config call-graph.record-mode > diff --git a/tools/perf/builtin-config.c b/tools/perf/builtin-config.c > index 88a43fe4963c..7c861b54f3a6 100644 > --- a/tools/perf/builtin-config.c > +++ b/tools/perf/builtin-config.c > @@ -17,7 +17,7 @@ > static bool use_system_config, use_user_config; > > static const char * const config_usage[] = { > - "perf config [] [options] [section.name ...]", > + "perf config [] [options] [section.name[=value] ...]", > NULL > }; > > @@ -33,6 +33,39 @@ static struct option config_options[] = { > OPT_END() > }; > > +static int set_config(struct perf_config_set *set, const char *file_name, > + const char *var, const char *value) > +{ > + struct perf_config_section *section = NULL; > + struct perf_config_item *item = NULL; > + const char *first_line = "# this file is auto-generated."; > + FILE *fp; > + > + if (set == NULL) > + return -1; > + > + fp = fopen(file_name, "w"); > + if (!fp) > + return -1; > + > + perf_config_set__collect(set, var, value); > + fprintf(fp, "%s\n", first_line); > + > + /* overwrite configvariables */ We need to add a space between two word as you said.. But it is so minor part, is it ok ? Thanks, Taeung > + perf_config_items__for_each_entry(&set->sections, section) { > + fprintf(fp, "[%s]\n", section->name); > + > + perf_config_items__for_each_entry(§ion->items, item) { > + if (item->value) > + fprintf(fp, "\t%s = %s\n", > + item->name, item->value); > + } > + } > + fclose(fp); > + > + return 0; > +} > + > static int show_spec_config(struct perf_config_set *set, const char *var) > { > struct perf_config_section *section; > @@ -82,7 +115,7 @@ static int show_config(struct perf_config_set *set) > return 0; > } > > -static int parse_config_arg(char *arg, char **var) > +static int parse_config_arg(char *arg, char **var, char **value) > { > const char *last_dot = strchr(arg, '.'); > > @@ -99,7 +132,21 @@ static int parse_config_arg(char *arg, char **var) > return -1; > } > > - *var = arg; > + *value = strchr(arg, '='); > + if (*value == NULL) > + *var = arg; > + else if (!strcmp(*value, "=")) { > + pr_err("The config variable does not contain a value: %s\n", arg); > + return -1; > + } else { > + *value = *value + 1; /* excluding a first character '=' */ > + *var = strsep(&arg, "="); > + if (*var[0] == '\0') { > + pr_err("invalid config variable: %s\n", arg); > + return -1; > + } > + } > + > return 0; > } > > @@ -153,7 +200,8 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused) > default: > if (argc) { > for (i = 0; argv[i]; i++) { > - char *var, *arg = strdup(argv[i]); > + char *var, *value; > + char *arg = strdup(argv[i]); > > if (!arg) { > pr_err("%s: strdup failed\n", __func__); > @@ -161,13 +209,21 @@ int cmd_config(int argc, const char **argv, const char *prefix __maybe_unused) > break; > } > > - if (parse_config_arg(arg, &var) < 0) { > + if (parse_config_arg(arg, &var, &value) < 0) { > free(arg); > ret = -1; > break; > } > > - ret = show_spec_config(set, var); > + if (value == NULL) > + ret = show_spec_config(set, var); > + else { > + const char *config_filename = config_exclusive_filename; > + > + if (!config_exclusive_filename) > + config_filename = user_config; > + ret = set_config(set, config_filename, var, value); > + } > free(arg); > } > } else > diff --git a/tools/perf/util/config.c b/tools/perf/util/config.c > index 18dae745034f..c8fb65d923cb 100644 > --- a/tools/perf/util/config.c > +++ b/tools/perf/util/config.c > @@ -602,6 +602,12 @@ static int collect_config(const char *var, const char *value, > return -1; > } > > +int perf_config_set__collect(struct perf_config_set *set, > + const char *var, const char *value) > +{ > + return collect_config(var, value, set); > +} > + > static int perf_config_set__init(struct perf_config_set *set) > { > int ret = -1; > diff --git a/tools/perf/util/config.h b/tools/perf/util/config.h > index 6f813d46045e..0fcdb8c594b0 100644 > --- a/tools/perf/util/config.h > +++ b/tools/perf/util/config.h > @@ -33,6 +33,8 @@ const char *perf_etc_perfconfig(void); > > struct perf_config_set *perf_config_set__new(void); > void perf_config_set__delete(struct perf_config_set *set); > +int perf_config_set__collect(struct perf_config_set *set, > + const char *var, const char *value); > void perf_config__init(void); > void perf_config__exit(void); > void perf_config__refresh(void); >