| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <6fca978a-471c-8429-ef3a-5e1aefcced8d@samsung.com>
Date: Fri, 10 Jan 2020 15:56:28 +0900
From: Chanwoo Choi <cw00.choi@...sung.com>
To: Bjorn Andersson <bjorn.andersson@...aro.org>,
Steven Rostedt <rostedt@...dmis.org>
Cc: Dmitry Osipenko <digetx@...il.com>, linux-pm@...r.kernel.org,
linux-kernel@...r.kernel.org, leonard.crestez@....com,
lukasz.luba@....com, a.swigon@...sung.com,
m.szyprowski@...sung.com, enric.balletbo@...labora.com,
hl@...k-chips.com, jcrouse@...eaurora.org, chanwoo@...nel.org,
myungjoo.ham@...sung.com, kyungmin.park@...sung.com
Subject: Re: [PATCH 2/2] PM / devfreq: Add devfreq_transitions debugfs file
Hi Bjorn,
On 1/10/20 2:04 PM, Chanwoo Choi wrote:
> On 1/10/20 2:21 AM, Bjorn Andersson wrote:
>> On Thu 09 Jan 00:07 PST 2020, Chanwoo Choi wrote:
>>
>>> Hi Bjorn and Dmitry,
>>>
>>> I replied from Bjorn and Dmitry opinion.
>>>
>>> On 1/8/20 11:20 PM, Dmitry Osipenko wrote:
>>>> 08.01.2020 00:48, Bjorn Andersson ??????????:
>>>>> On Tue 07 Jan 01:05 PST 2020, Chanwoo Choi wrote:
>>>>>
>>>>>> Add new devfreq_transitions debugfs file to track the frequency transitions
>>>>>> of all devfreq devices for the simple profiling as following:
>>>>>> - /sys/kernel/debug/devfreq/devfreq_transitions
>>>>>>
>>>>>> And the user can decide the storage size (CONFIG_NR_DEVFREQ_TRANSITIONS)
>>>>>> in Kconfig in order to save the transition history.
>>>>>>
>>>>>> [Detailed description of each field of 'devfreq_transitions' debugfs file]
>>>>>> - time_ms : Change time of frequency transition. (unit: millisecond)
>>>>>> - dev_name : Device name of h/w.
>>>>>> - dev : Device name made by devfreq core.
>>>>>> - parent_dev : If devfreq device uses the passive governor,
>>>>>> show parent devfreq device name.
>>>>>> - load_% : If devfreq device uses the simple_ondemand governor,
>>>>>> load is used by governor whene deciding the new frequency.
>>>>>> (unit: percentage)
>>>>>> - old_freq_hz : Frequency before changing. (unit: hz)
>>>>>> - new_freq_hz : Frequency after changed. (unit: hz)
>>>>>>
>>>>>> [For example on Exynos5422-based Odroid-XU3 board]
>>>>>> $ cat /sys/kernel/debug/devfreq/devfreq_transitions
>>>>>> time_ms dev_name dev parent_dev load_% old_freq_hz new_freq_hz
>>>>>> ---------- ------------------------------ ---------- ---------- ---------- ------------ ------------
>>>>>> 14600 soc:bus_noc devfreq2 devfreq1 0 100000000 67000000
>>>>>> 14600 soc:bus_fsys_apb devfreq3 devfreq1 0 200000000 100000000
>>>>>> 14600 soc:bus_fsys devfreq4 devfreq1 0 200000000 100000000
>>>>>> 14600 soc:bus_fsys2 devfreq5 devfreq1 0 150000000 75000000
>>>>>> 14602 soc:bus_mfc devfreq6 devfreq1 0 222000000 96000000
>>>>>> 14602 soc:bus_gen devfreq7 devfreq1 0 267000000 89000000
>>>>>> 14602 soc:bus_g2d devfreq9 devfreq1 0 300000000 84000000
>>>>>> 14602 soc:bus_g2d_acp devfreq10 devfreq1 0 267000000 67000000
>>>>>> 14602 soc:bus_jpeg devfreq11 devfreq1 0 300000000 75000000
>>>>>> 14602 soc:bus_jpeg_apb devfreq12 devfreq1 0 167000000 84000000
>>>>>> 14603 soc:bus_disp1_fimd devfreq13 devfreq1 0 200000000 120000000
>>>>>> 14603 soc:bus_disp1 devfreq14 devfreq1 0 300000000 120000000
>>>>>> 14606 soc:bus_gscl_scaler devfreq15 devfreq1 0 300000000 150000000
>>>>>> 14606 soc:bus_mscl devfreq16 devfreq1 0 333000000 84000000
>>>>>> 14608 soc:bus_wcore devfreq1 9 333000000 84000000
>>>>>> 14783 10c20000.memory-controller devfreq0 35 825000000 633000000
>>>>>> 15873 soc:bus_wcore devfreq1 41 84000000 400000000
>>>>>> 15873 soc:bus_noc devfreq2 devfreq1 0 67000000 100000000
>>>>>> [snip]
>>>>>>
>>>>>
>>>>> Wouldn't it make more sense to expose this through the tracing
>>>>> framework - like many other subsystems does?
>>>>
>>>> I think devfreq core already has some tracing support and indeed it
>>>> should be better to extend it rather than duplicate.
>>>>
>>>
>>> First of all, thanks for comments.
>>>
>>> Before developing it, I have considered what is better to
>>> support debugging features for devfreq device. As you commented,
>>> trace event is more general way to catch the detailed behavior.
>>>
>>
>> It's more general, it has already dealt with the locking and life cycle
>> questions that was brought up by others and it allows getting traces
>> devfreq traces in the same timeline as other events (to give insight in
>> cross-framework behavior).
>>
>>> But, I hope to provide the very easy simple profiling way
>>> for user if it is not harmful to the principle of linux kernel.
>>>
>>
>> You would achieve the same simplicity by integrating with the trace
>> framework instead of rolling your own subset of the functionality.
>>
>> I know that it's the principle of the Linux kernel that everyone should
>> have their own ring buffer implementation, but you should try to use the
>> existing ones when it makes sense. And in my view this is a prime
>> example - with many additional benefits of doing so.
>
> When we are usually using the profiling tool, existing trace framework
> is the best. Actually, might need to read the frequency transitions
> on the user-space process which is related to monitoring, without
> the enabled trace configuration.
>
>>
>>> In order to prevent the performance regression when DEBUG_FS is enabled,
>>> I will add the CONFIG_DEVFREQ_TRANSITIONS_DEBUG for 'devfreq_transitions'
>>> to make it selectable.
>>>
>>
>> The tracing framework has both static and dynamic mechanisms for
>> avoiding performance penalties when tracing is disabled and does provide
>> better performance than your proposal when active.
>
> It provides the separate configuration to select them by user.
> It is optional. It means that if CONFIG_DEVFREQ_TRANSITIONS_DEBUG
> is enabled by user, the user has made a choice this situation with
> even if the some regression happen and instead get the frequency
> transition for monitoring on user-space process.
>
>>
>> Relying on a Kconfig option means that with e.g. arm64 devices being
>> built from a single defconfig we will either all be missing this feature
>> or we will all always keep logging devfreq transitions to your buffer.
>
> The single defconfig doesn't contain the all configuration provided
> from linux kernel. Furthermore, the debug option is optional by user.
> I think that it doesn't matter.
>
Basically, I agree that trace point is better. Just as I said,
I hope to provide the very easy simple profiling way.
But, as you suggested and I knew that it seems that
duplicate feature. So, I'll drop it and extend the existing
trace point of devfreq. Thanks for your comment and discussion.
--
Best Regards,
Chanwoo Choi
Samsung Electronics
Powered by blists - more mailing lists