| 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: <CAPDyKFprdJ8hTAdtbMf+P4q0N8EhpSywteFK7HvxoSpVf50Pew@mail.gmail.com>
Date: Wed, 19 Dec 2018 10:25:02 +0100
From: Ulf Hansson <ulf.hansson@...aro.org>
To: Vincent Guittot <vincent.guittot@...aro.org>
Cc: Linux PM <linux-pm@...r.kernel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
"Rafael J. Wysocki" <rjw@...ysocki.net>,
Thara Gopinath <thara.gopinath@...aro.org>,
jani.nikula@...ux.intel.com,
Joonas Lahtinen <joonas.lahtinen@...ux.intel.com>,
rodrigo.vivi@...el.com, David Airlie <airlied@...ux.ie>,
"Intel graphics driver community testing & development"
<intel-gfx@...ts.freedesktop.org>,
dri-devel <dri-devel@...ts.freedesktop.org>
Subject: Re: [PATCH v2 1/3] PM/pm_runtime: move autosuspend on hrtimer
On Fri, 14 Dec 2018 at 15:22, Vincent Guittot
<vincent.guittot@...aro.org> wrote:
>
> pm runtime uses the timer infrastructure for autosuspend. This implies that
> the minimum time before autosuspending a device is in the range
> of 1 tick included to 2 ticks excluded
> -On arm64 this means between 4ms and 8ms with default jiffies configuration
> -And on arm, it is between 10ms and 20ms
>
> These values are quite high for embedded system which sometimes wants
> duration in the range of 1 ms.
>
> We can move autosuspend on hrtimer to get finer granularity for short
> duration and takes advantage of slack to keep some margins and gathers
> long timeout in minimum wakeups.
>
> On an arm64 platform that uses 1ms for autosuspending timeout of its GPU,
> the power consumption improves by 10% for idle use case with hrtimer.
>
> The latency impact on arm64 hikey octo cores is :
> - mark_last_busy: from 1.11 us to 1.25 us
> - rpm_suspend: from 15.54 us to 15.38 us
> Only the code path of rpm_suspend that starts hrtimer has been measured
>
> arm64 image (arm64 default defconfig) decreases by around 3KB
> with following details:
>
> $ size vmlinux-timer
> text data bss dec hex filename
> 12034646 6869268 386840 19290754 1265a82 vmlinux
>
> $ size vmlinux-hrtimer
> text data bss dec hex filename
> 12030550 6870164 387032 19287746 1264ec2 vmlinux
>
> The latency impact on arm 32bits snowball dual cores is :
> - mark_last_busy: from 0.31 us usec to 0.77 us
> - rpm_suspend: from 6.83 us to 6.67 usec
>
> The increase of the image for snowball platform that I used for testing
> performance impact, is neglictable (244B)
>
> $ size vmlinux-timer
> text data bss dec hex filename
> 7157961 2119580 264120 9541661 91981d build-ux500/vmlinux
>
> size vmlinux-hrtimer
> text data bss dec hex filename
> 7157773 2119884 264248 9541905 919911 vmlinux-hrtimer
>
> And arm 32bits image (multi_v7_defconfig) increases by around 1.7KB
> with following details:
>
> $ size vmlinux-timer
> text data bss dec hex filename
> 13304443 6803420 402768 20510631 138f7a7 vmlinux
>
> $ size vmlinux-hrtimer
> text data bss dec hex filename
> 13304299 6805276 402768 20512343 138fe57 vmlinux
>
> Signed-off-by: Vincent Guittot <vincent.guittot@...aro.org>
If not too late, feel free to add:
Reviewed-by: Ulf Hansson <ulf.hansson@...aro.org>
Kind regards
Uffe
> ---
> drivers/base/power/runtime.c | 63 ++++++++++++++++++++++++--------------------
> include/linux/pm.h | 5 ++--
> include/linux/pm_runtime.h | 6 ++---
> 3 files changed, 40 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
> index beb85c3..7062469 100644
> --- a/drivers/base/power/runtime.c
> +++ b/drivers/base/power/runtime.c
> @@ -8,6 +8,8 @@
> */
>
> #include <linux/sched/mm.h>
> +#include <linux/ktime.h>
> +#include <linux/hrtimer.h>
> #include <linux/export.h>
> #include <linux/pm_runtime.h>
> #include <linux/pm_wakeirq.h>
> @@ -93,7 +95,7 @@ static void __update_runtime_status(struct device *dev, enum rpm_status status)
> static void pm_runtime_deactivate_timer(struct device *dev)
> {
> if (dev->power.timer_expires > 0) {
> - del_timer(&dev->power.suspend_timer);
> + hrtimer_cancel(&dev->power.suspend_timer);
> dev->power.timer_expires = 0;
> }
> }
> @@ -124,12 +126,11 @@ static void pm_runtime_cancel_pending(struct device *dev)
> * This function may be called either with or without dev->power.lock held.
> * Either way it can be racy, since power.last_busy may be updated at any time.
> */
> -unsigned long pm_runtime_autosuspend_expiration(struct device *dev)
> +u64 pm_runtime_autosuspend_expiration(struct device *dev)
> {
> int autosuspend_delay;
> - long elapsed;
> - unsigned long last_busy;
> - unsigned long expires = 0;
> + u64 last_busy, expires = 0;
> + u64 now = ktime_to_ns(ktime_get());
>
> if (!dev->power.use_autosuspend)
> goto out;
> @@ -139,19 +140,9 @@ unsigned long pm_runtime_autosuspend_expiration(struct device *dev)
> goto out;
>
> last_busy = READ_ONCE(dev->power.last_busy);
> - elapsed = jiffies - last_busy;
> - if (elapsed < 0)
> - goto out; /* jiffies has wrapped around. */
>
> - /*
> - * If the autosuspend_delay is >= 1 second, align the timer by rounding
> - * up to the nearest second.
> - */
> - expires = last_busy + msecs_to_jiffies(autosuspend_delay);
> - if (autosuspend_delay >= 1000)
> - expires = round_jiffies(expires);
> - expires += !expires;
> - if (elapsed >= expires - last_busy)
> + expires = last_busy + autosuspend_delay * NSEC_PER_MSEC;
> + if (expires <= now)
> expires = 0; /* Already expired. */
>
> out:
> @@ -515,7 +506,7 @@ static int rpm_suspend(struct device *dev, int rpmflags)
> /* If the autosuspend_delay time hasn't expired yet, reschedule. */
> if ((rpmflags & RPM_AUTO)
> && dev->power.runtime_status != RPM_SUSPENDING) {
> - unsigned long expires = pm_runtime_autosuspend_expiration(dev);
> + u64 expires = pm_runtime_autosuspend_expiration(dev);
>
> if (expires != 0) {
> /* Pending requests need to be canceled. */
> @@ -528,10 +519,20 @@ static int rpm_suspend(struct device *dev, int rpmflags)
> * expire; pm_suspend_timer_fn() will take care of the
> * rest.
> */
> - if (!(dev->power.timer_expires && time_before_eq(
> - dev->power.timer_expires, expires))) {
> + if (!(dev->power.timer_expires &&
> + dev->power.timer_expires <= expires)) {
> + /*
> + * We add a slack of 25% to gather wakeups
> + * without sacrificing the granularity.
> + */
> + u64 slack = READ_ONCE(dev->power.autosuspend_delay) *
> + (NSEC_PER_MSEC >> 2);
> +
> dev->power.timer_expires = expires;
> - mod_timer(&dev->power.suspend_timer, expires);
> + hrtimer_start_range_ns(&dev->power.suspend_timer,
> + ns_to_ktime(expires),
> + slack,
> + HRTIMER_MODE_ABS);
> }
> dev->power.timer_autosuspends = 1;
> goto out;
> @@ -895,23 +896,25 @@ static void pm_runtime_work(struct work_struct *work)
> *
> * Check if the time is right and queue a suspend request.
> */
> -static void pm_suspend_timer_fn(struct timer_list *t)
> +static enum hrtimer_restart pm_suspend_timer_fn(struct hrtimer *timer)
> {
> - struct device *dev = from_timer(dev, t, power.suspend_timer);
> + struct device *dev = container_of(timer, struct device, power.suspend_timer);
> unsigned long flags;
> - unsigned long expires;
> + u64 expires;
>
> spin_lock_irqsave(&dev->power.lock, flags);
>
> expires = dev->power.timer_expires;
> /* If 'expire' is after 'jiffies' we've been called too early. */
> - if (expires > 0 && !time_after(expires, jiffies)) {
> + if (expires > 0 && expires < ktime_to_ns(ktime_get())) {
> dev->power.timer_expires = 0;
> rpm_suspend(dev, dev->power.timer_autosuspends ?
> (RPM_ASYNC | RPM_AUTO) : RPM_ASYNC);
> }
>
> spin_unlock_irqrestore(&dev->power.lock, flags);
> +
> + return HRTIMER_NORESTART;
> }
>
> /**
> @@ -922,6 +925,7 @@ static void pm_suspend_timer_fn(struct timer_list *t)
> int pm_schedule_suspend(struct device *dev, unsigned int delay)
> {
> unsigned long flags;
> + ktime_t expires;
> int retval;
>
> spin_lock_irqsave(&dev->power.lock, flags);
> @@ -938,10 +942,10 @@ int pm_schedule_suspend(struct device *dev, unsigned int delay)
> /* Other scheduled or pending requests need to be canceled. */
> pm_runtime_cancel_pending(dev);
>
> - dev->power.timer_expires = jiffies + msecs_to_jiffies(delay);
> - dev->power.timer_expires += !dev->power.timer_expires;
> + expires = ktime_add(ktime_get(), ms_to_ktime(delay));
> + dev->power.timer_expires = ktime_to_ns(expires);
> dev->power.timer_autosuspends = 0;
> - mod_timer(&dev->power.suspend_timer, dev->power.timer_expires);
> + hrtimer_start(&dev->power.suspend_timer, expires, HRTIMER_MODE_ABS);
>
> out:
> spin_unlock_irqrestore(&dev->power.lock, flags);
> @@ -1491,7 +1495,8 @@ void pm_runtime_init(struct device *dev)
> INIT_WORK(&dev->power.work, pm_runtime_work);
>
> dev->power.timer_expires = 0;
> - timer_setup(&dev->power.suspend_timer, pm_suspend_timer_fn, 0);
> + hrtimer_init(&dev->power.suspend_timer, CLOCK_MONOTONIC, HRTIMER_MODE_ABS);
> + dev->power.suspend_timer.function = pm_suspend_timer_fn;
>
> init_waitqueue_head(&dev->power.wait_queue);
> }
> diff --git a/include/linux/pm.h b/include/linux/pm.h
> index e723b78..0bd9de1 100644
> --- a/include/linux/pm.h
> +++ b/include/linux/pm.h
> @@ -26,6 +26,7 @@
> #include <linux/spinlock.h>
> #include <linux/wait.h>
> #include <linux/timer.h>
> +#include <linux/hrtimer.h>
> #include <linux/completion.h>
>
> /*
> @@ -608,7 +609,7 @@ struct dev_pm_info {
> unsigned int should_wakeup:1;
> #endif
> #ifdef CONFIG_PM
> - struct timer_list suspend_timer;
> + struct hrtimer suspend_timer;
> unsigned long timer_expires;
> struct work_struct work;
> wait_queue_head_t wait_queue;
> @@ -631,7 +632,7 @@ struct dev_pm_info {
> enum rpm_status runtime_status;
> int runtime_error;
> int autosuspend_delay;
> - unsigned long last_busy;
> + u64 last_busy;
> unsigned long active_jiffies;
> unsigned long suspended_jiffies;
> unsigned long accounting_timestamp;
> diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h
> index f0fc470..54af4ee 100644
> --- a/include/linux/pm_runtime.h
> +++ b/include/linux/pm_runtime.h
> @@ -51,7 +51,7 @@ extern void pm_runtime_no_callbacks(struct device *dev);
> extern void pm_runtime_irq_safe(struct device *dev);
> extern void __pm_runtime_use_autosuspend(struct device *dev, bool use);
> extern void pm_runtime_set_autosuspend_delay(struct device *dev, int delay);
> -extern unsigned long pm_runtime_autosuspend_expiration(struct device *dev);
> +extern u64 pm_runtime_autosuspend_expiration(struct device *dev);
> extern void pm_runtime_update_max_time_suspended(struct device *dev,
> s64 delta_ns);
> extern void pm_runtime_set_memalloc_noio(struct device *dev, bool enable);
> @@ -105,7 +105,7 @@ static inline bool pm_runtime_callbacks_present(struct device *dev)
>
> static inline void pm_runtime_mark_last_busy(struct device *dev)
> {
> - WRITE_ONCE(dev->power.last_busy, jiffies);
> + WRITE_ONCE(dev->power.last_busy, ktime_to_ns(ktime_get()));
> }
>
> static inline bool pm_runtime_is_irq_safe(struct device *dev)
> @@ -168,7 +168,7 @@ static inline void __pm_runtime_use_autosuspend(struct device *dev,
> bool use) {}
> static inline void pm_runtime_set_autosuspend_delay(struct device *dev,
> int delay) {}
> -static inline unsigned long pm_runtime_autosuspend_expiration(
> +static inline u64 pm_runtime_autosuspend_expiration(
> struct device *dev) { return 0; }
> static inline void pm_runtime_set_memalloc_noio(struct device *dev,
> bool enable){}
> --
> 2.7.4
>
Powered by blists - more mailing lists