Messages in this thread | | | Subject | Re: [RFC][PATCH] perf config: Introduce perf_config_set class | From | Taeung Song <> | Date | Sat, 12 Mar 2016 19:28:59 +0900 |
| |
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
| |