Messages in this thread | | | Subject | Re: [PATCH 2/2] PM / devfreq: Add devfreq_transitions debugfs file | From | Chanwoo Choi <> | Date | Fri, 10 Jan 2020 15:56:28 +0900 |
| |
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
| |