lkml.org 
[lkml]   [2016]   [Mar]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [RFC][PATCH] perf config: Introduce perf_config_set class
From
Date


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

\
 
 \ /
  Last update: 2016-03-12 12:21    [W:0.077 / U:0.108 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site