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, 3 Feb 2020 10:10:00 +0900 From: Chanwoo Choi <cw00.choi@...sung.com> To: Lukasz Luba <lukasz.luba@....com>, myungjoo.ham@...sung.com, kyungmin.park@...sung.com, linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org Cc: b.zolnierkie@...sung.com, Kamil Konieczny <k.konieczny@...sung.com> Subject: Re: [PATCH 0/1] drivers: devfreq: use DELAYED_WORK in DEVFREQ monitoring subsystem On 1/31/20 6:38 PM, Lukasz Luba wrote: > Hi Chanwoo, > > On 1/31/20 12:47 AM, Chanwoo Choi wrote: >> On 1/31/20 9:42 AM, Chanwoo Choi wrote: >>> Hi Lukasz, >>> >>> On 1/30/20 8:47 PM, Lukasz Luba wrote: >>>> Hi Chanwoo, MyungJoo, >>>> >>>> Gentle ping. The issue is not only in the devfreq itself, >>>> but also it affects thermal. The devfreq cooling rely on >>>> busy_time and total_time updated by the devfreq monitoring >>>> (in simple_ondemand). >>>> Thermal uses DELAYED_WORK and is more reliable, but uses stale >>>> data from devfreq_dev_stats. It is especially visible when >>>> you have cgroup spanning one cluster. Android uses cgroups >>>> heavily. You can make easily this setup using 'taskset', >>>> run some benchmarks and observe 'devfreq_monitor' traces and >>>> timestamps, i.e. for your exynos-bus. >>>> >>>> The patch is really non-invasive and simple. It can be a good starting >>>> point for testing and proposing other solutions. >>> >>> Sorry for late reply. I'm preparing the RFC patch about my approach >>> to support this requirement as following: >>> >>> As you knew, DEFERRABLE_WORK with CONFIG_NO_HZ focuses on removing >>> the redundant of power-consumption by preventing the unneeded wakeup >>> from idle state if there are no any interrupts and runnable threads. >>> >>> Finally, I agree the requirement of delaywd_work for devfreq subsystem. >>> But, I would like to support both deferrable_work and delayed_work >>> on devfreq subsystem. It is better to select either deferrable_work >>> or delayed_work by user like Kamil's suggestion[1]. >>> [1] https://lore.kernel.org/patchwork/patch/1164317/ >>> - [2/4] PM / devfreq: add possibility for delayed work >>> >>> But, I want to change the timer type for devfreq device >>> using simple_ondemand governor via sysfs as following: >>> >>> Example: >>> >>> 1. >>> enum work_timer_type { >>> DEVFREQ_WORK_TIMER_DEFERRABLE = 0, >>> DEVFREQ_WORK_TIMER_DELAYED = 0, >>> }; >>> >>> struct devfreq_simple_ondemand_data { >>> unsigned int upthreshold; >>> unsigned int downdifferential; >>> enum work_timer_type timer_type; >>> }; >>> >>> The developer of devfreq device driver can choose >>> the default work time type by initializing the 'timer_type of >>> struct devfreq_simple_ondemand_data'. >>> >>> 2. Change the work timer type at the runtime >>> - Change the work timer type from 'deferrable' to 'delayed' >>> $ echo delayed > /sys/class/devfreq/devfreq0/work_timer_type >>> $ cat /sys/class/devfreq/devfreq0/work_timer_type >>> delayed >>> >>> - Change the work timer type from 'delayed' to 'deferrable' >>> $ echo deferrable > /sys/class/devfreq/devfreq0/work_timer_type >>> $ cat /sys/class/devfreq/devfreq0/work_timer_type >>> deferrable >>> >> >> And >> Only show '/sys/class/devfreq/devfreq0/work_timer_type' sysfs attribute, >> if devfreq device uses the simple_ondemand. Because this 'work_timer_type' >> sysfs attribute only depends on simple_ondemand governor and are useful. >> >> So, 'work_timer_type' sysfs attribute will be handled >> at drivers/devfreq/governor_simpleondemand.c. >> >> After posting my suggestion, we can discuss it. >> >> >>> I'm developing the RFC patch and then I'll send it as soon as possible. > > Good, thank you for the explanation. For the first glance the design > looks OK, we can discuss it a bit more in you RFC series. > I would recommend to not make it conditional on simple_ondemand governor > just add a comment that for i.e. performance or passive governors it has > less sense to use this setting. There might be some other governors > loaded as modules, which could benefit from it, or in Android e.g. > https://android.googlesource.com/kernel/msm/+/refs/heads/android-msm-coral-4.14-android10/drivers/devfreq/governor_msm_adreno_tz.c OK. Instead, I'll add the flag for governors. The governor flag indicates each sysfs entries like polling_interval, work_timer_type. If each governor want to use the specific sysfs attributes, just set the flag when governor is defined. Thanks. > > It would be good if it can land in mainline before v5.8-v5.9. > > Regards, > Lukasz > > > > -- Best Regards, Chanwoo Choi Samsung Electronics
Powered by blists - more mailing lists