From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753565AbbGSNmG (ORCPT ); Sun, 19 Jul 2015 09:42:06 -0400 Received: from mail-pa0-f41.google.com ([209.85.220.41]:33128 "EHLO mail-pa0-f41.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753378AbbGSNmE (ORCPT ); Sun, 19 Jul 2015 09:42:04 -0400 Date: Sun, 19 Jul 2015 22:39:45 +0900 From: Namhyung Kim To: Taeung Song Cc: Arnaldo Carvalho de Melo , linux-kernel@vger.kernel.org, jolsa@redhat.com, Ingo Molnar Subject: Re: [PATCH v4 0/5] perf tools: Add 'perf-config' command Message-ID: <20150719133945.GF25163@danjae.kornet> References: <1437097202-1915-1-git-send-email-treeze.taeung@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1437097202-1915-1-git-send-email-treeze.taeung@gmail.com> User-Agent: Mutt/1.5.23+89 (0255b37be491) (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Taeung, On Fri, Jul 17, 2015 at 10:39:57AM +0900, Taeung Song wrote: > Changes in v4: > - If some config value is default value, notice it is '(default)'as below. > > # perf config colors.normal > colors.normal=lightgray, default (default) > > - A config file path is only decided in perf_config(). > And a perf-config command depend on perf_config() deciding a config file path. > So if the config file path is NULL because of some reason like no such file, > a perf-config command can't write to the file. So, I added new options > which are '--global' and '--system' to be enable to select > config file path to be used without perf_config(). > > Changes in v3: > - Add a config variable 'kmem.default' with a default value > into 'struct default_configset' which has default config variables and values. > > - Add a option '--global' and '--local' to enable config file location to be selected > > Changes in v2: > - Renaming variables a more suitable name > 1. '--list-all' instead of '--all' > 2. 'name' instead of 'subkey' > 3. 'section, name, value' instead of 'given_section, subkey, value' > > - Correct small infelicities or typing errors in a perf-config documention. > > - Remove a part description of report.children > because it was duplicated in Documentation/callchain-overhead-calculation.txt > > - Use a variable 'int actions' instead of struct params which has > 'bool list_action' , 'bool get_action’ and etc. , to simplify a branching statement > for perf-config options. > > - Declaration a global variable 'static struct default_configsets' has config variables > with default values instead of using a 'util/PERFCONFIG-DEFAULT' file > and remove functions merge() and perse_key() to get perf config default values. > > - Add a function to normalize a value and check data type of it. > > - Simplify parsing arguments as arguments is just divided by '=' and then > in front of '.' is a section, between '.' and '=' is a name, and behind '=' is a value. > > - Print config variables 'struct default_configsets' haven't > > - Make a command ’perf config' without a option work as with a option '—list' > instead of '--list-all’. It seems the changelog itself is too big. I suggest make it more compact and put the details in the corresponding patches. Also you'd be better keeping the general description for the patchset here. Thanks, Namhyung > > Taeung Song (5): > perf tools: Add 'perf-config' command > perf config: Add '--system' and '--global' options to be able to > select which config file to be used > perf config: Add functions which can get or set perf config variables > perf config: Add a option 'list-all' to perf-config > perf config: Add a option 'remove' to perf-config > > tools/perf/Build | 1 + > tools/perf/Documentation/perf-config.txt | 407 ++++++++++++++++ > tools/perf/Documentation/perfconfig.example | 73 ++- > tools/perf/builtin-config.c | 718 ++++++++++++++++++++++++++++ > tools/perf/builtin.h | 1 + > tools/perf/command-list.txt | 1 + > tools/perf/perf.c | 1 + > tools/perf/util/cache.h | 19 + > tools/perf/util/config.c | 29 +- > 9 files changed, 1236 insertions(+), 14 deletions(-) > create mode 100644 tools/perf/Documentation/perf-config.txt > create mode 100644 tools/perf/builtin-config.c > > -- > 1.9.1 >