Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752310AbcCLK3P (ORCPT ); Sat, 12 Mar 2016 05:29:15 -0500 Received: from mail-pa0-f49.google.com ([209.85.220.49]:34989 "EHLO mail-pa0-f49.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751241AbcCLK3E (ORCPT ); Sat, 12 Mar 2016 05:29:04 -0500 Subject: Re: [RFC][PATCH] perf config: Introduce perf_config_set class To: Namhyung Kim , Jiri Olsa References: <1457618667-19915-1-git-send-email-treeze.taeung@gmail.com> <20160311141131.GB25533@danjae.kornet> <56E2DF84.7080406@gmail.com> <20160312084541.GB27257@danjae.kornet> Cc: Arnaldo Carvalho de Melo , linux-kernel@vger.kernel.org, Peter Zijlstra , Ingo Molnar From: Taeung Song Message-ID: <56E3EF6B.9050700@gmail.com> Date: Sat, 12 Mar 2016 19:28:59 +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: <20160312084541.GB27257@danjae.kornet> 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: 5512 Lines: 213 On 03/12/2016 05:45 PM, Namhyung Kim wrote: > Hi Taeung, > > On Sat, Mar 12, 2016 at 12:08:52AM +0900, Taeung Song wrote: >> Hi, Namhyung >> >> On 03/11/2016 11:11 PM, Namhyung Kim wrote: >>> Also I think it'd be better just keeping a single config value instead >>> of 3 kinds. Maybe you can read system-wide config first and overwrite >>> them with user config (for the 'both' case). >>> >> >> I know what you mean. I agonized about it. >> >> IMHO, I think that if keeping a single config value instead of 3 kinds and >> perf-config has setting functionality when writing a changed config >> on a specific config file, some problems can occur e.g. > > Do you plan to support 'set' and 'get' operation at the same time? > IOW is it possible to do? > > $ perf config --set aaa.bbb=xx --get ccc.ddd > > I don't think it's very useful. > > If we don't do it, I think we can simply read a single config file > (default to user file) and re-write it for the 'set' operation. I agree. I think that what you said is a simple way for 'set' operation. But I have a plan about perf-config interface like 'sysctl' (suggested by jiri) http://marc.info/?l=linux-kernel&m=142842999926479&w=2 For examples, sysctl [options] [variable[=value]] [...] sysctl -p [file or regexp] [...] # display current config perf config # display current config plus all keys with default values perf config -a # display key value: perf config report.queue # set key value: perf config report.queue=100M # remove key (not in sysctl) perf config -r report.queue If we do so, perf-config support 'set' and 'get' operation at the same time e.g sysctl: # sysctl vm.stat_interval vm.stat_interval=2 vm.user_reserve_kbytes vm.stat_interval = 1 vm.stat_interval = 2 vm.user_reserve_kbytes = 131072 perf-config: # perf config report.queue-size report.queue-size=100M top.children report.queue-size=1 report.queue-size=104857600 top.children=true jiri, is it right ? or the above situation wasn't what you mean ? (I understood so) then, namhyung, is it better to use the simple way for 'set' and 'get' operation ? (instead of 'sysctl' style) > Or maybe we can add a field (like 'origin'?) in the perf_config_item > struct to mark where it comes from. And then it should write items > matching 'origin' only. > I understood a field 'origin' e.g struct perf_config_item { (..omitted..) enum config_file { USER_CONFIG, SYSTEM_CONFIG } origin; } And if the two files have same variables, user config file has a high priority. (as I understood) IMHO, I think that even if we use 'origin', some problem can occur when handling same variables e.g. User wide: # cat ~/.perfconfig [report] queue-size = 1 System wide: # cat /usr/local/etc/perfconfig [report] queue-size = 2 [top] children = false If user or system config files has same variable as above, # perf config --system top.children=true # perf config --system --list top.children=false the above problem can occur. When rewriting items matching 'origin', because a item for 'report.queue-size' has 'origin' marked as user config file, 'report.queue-size' of system config file can be removed. And even though perf_config_item has a field 'origin' to mark where it comes from, if perf_config_item has only single value we can't know old value of 'report.queue-size' for system config file as ever because of overwriting a single value of perf_config_item when collecting configs from the config files. Did I misunderstand your intention using a field 'origin' ? :-\ Or when the two files have same config variables, are there two items each other (for system and user config file) ? Thanks, Taeung > > >> >> (Because setting functionality I design is that overwrite >> a specific config file by the perf config list) >> (the perf config list : all perf configs from the config files) >> >> User wide: >> >> # cat ~/.perfconfig >> [report] >> queue-size = 1 >> [test] >> location = user >> >> System wide: >> >> # cat /usr/local/etc/perfconfig >> [ui] >> show-headers = false >> [test] >> location = system >> >> And if perf-config has setting functionality, >> >> # perf config --system top.children=false >> >> We hoped for: >> >> # cat /usr/local/etc/perfconfig >> [ui] >> show-headers = false >> [test] >> location = system >> [top] >> children = false >> >> But actual result can be: >> >> # cat /usr/local/etc/perfconfig >> [ui] >> show-headers = false >> [report] >> queue-size = 1 >> [test] >> location = user >> [top] >> children = false >> >> We wouldn't want that system config file contain contents of >> user config file. >> The reason of this problem is that setting functionality I design >> work with perf config list overwriting a specific config file >> and if perf config list has only single value each config, >> we don't exactly know old values of system config. >> >> Don't design setting functionality that overwrite by perf config list ? >> (writing '# this file is auto-generated.' at the top of config file) >> >> Add a changed config into a specific config file by other way ? :-\ >> >> Or >> Not now, when add setting functionality into perf-config, >> consider this problem ? >> >> Thanks, >> Taeung