| 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
| ||
|
Date: Mon, 13 Jan 2020 17:19:51 +0000
From: Leonard Crestez <leonard.crestez@....com>
To: Lukasz Luba <lukasz.luba@....com>,
Dmitry Osipenko <digetx@...il.com>,
Bjorn Andersson <bjorn.andersson@...aro.org>,
Chanwoo Choi <cw00.choi@...sung.com>
CC: "linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"a.swigon@...sung.com" <a.swigon@...sung.com>,
"m.szyprowski@...sung.com" <m.szyprowski@...sung.com>,
"enric.balletbo@...labora.com" <enric.balletbo@...labora.com>,
"hl@...k-chips.com" <hl@...k-chips.com>,
"jcrouse@...eaurora.org" <jcrouse@...eaurora.org>,
"chanwoo@...nel.org" <chanwoo@...nel.org>,
"myungjoo.ham@...sung.com" <myungjoo.ham@...sung.com>,
"kyungmin.park@...sung.com" <kyungmin.park@...sung.com>
Subject: Re: [PATCH 2/2] PM / devfreq: Add devfreq_transitions debugfs file
On 08.01.2020 17:44, Lukasz Luba wrote:
> On 1/8/20 2: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.
+1 for tracing
> In my opinion this debugfs interface should be considered as a helpful
> validation entry point. We had some issues with wrong bootloader
> configurations in clock tree, where some frequencies could not be set
> in the kernel. Similar useful description can be find in clock subsystem
> where there is clock tree summary file.
>
> It is much cheaper to poke a few files in debug dir by some automated
> test than starting tracing, provoking desired code flow in the
> devfreq for every device, paring the results... A simple boot test
> which reads only these new files can be enough to rise the flag.
Tracepoints are also very powerful for debugging boot issues! You can
add "tp_printk trace_event=devfreq:*" to boot arguments and you will see
console messages for all relevant events. This works even if boot fails
before userspace is available to mount debugfs.
> Secondly the tracing is not always compiled.
Tracing is deliberately light-weight and should be enabled even on
production systems.
> It could capture old/wrong bootloaders which pinned devices
> improperly to PLLs or wrong DT values in OPP table.
> (a workaround for Odroid xu4 patchset:
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flkml.org%2Flkml%2F2019%2F7%2F15%2F276&data=02%7C01%7Cleonard.crestez%40nxp.com%7C8397d37b41474137f8cf08d79451a007%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637140950611913278&sdata=rcbWCyFmf0ZO7LU27D05mftTf8YdSvGPYNsst1GnNjQ%3D&reserved=0
> )
>
> Chanwoo what do think about some sanity check summary?
> It could be presented in a 3rd file: 'devfreq_sanity', which
> could report if the devices could set their registered OPPs
> and got the same values, i.e. set 166MHz --> set to 150MHz
> in reality. If a config option i.e. DEVFREQ_SANITY is set
> then during the registration of a new device it checks OPPs
> if they are possible to set. It could be done before assigning
> the governor for the device and results present in of of your files.
The new devfreq_transition tracepoint could include a field for
"new_effective freq" next to "old_freq" and "new_requested_freq".
For imx8m-ddrc I handled this inside the target() function: clk_get_rate
is called after the transition and an error is reported if rate doesn't
match.
It might make sense for devfreq core to handle this internally by
calling get_cur_freq instead.
--
Regards,
Leonard
Powered by blists - more mailing lists