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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Fri, 22 Apr 2022 16:12:45 +0200
From:   "Rafael J. Wysocki" <rafael@...nel.org>
To:     Ulf Hansson <ulf.hansson@...aro.org>
Cc:     "Rafael J . Wysocki" <rafael@...nel.org>,
        Linux PM <linux-pm@...r.kernel.org>,
        Kevin Hilman <khilman@...nel.org>,
        Vincent Guittot <vincent.guittot@...aro.org>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        Dmitry Osipenko <digetx@...il.com>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] PM: domains: Move genpd's time-accounting to ktime_get_mono_fast_ns()

On Tue, Apr 19, 2022 at 7:29 PM Ulf Hansson <ulf.hansson@...aro.org> wrote:
>
> To move towards a more consistent behaviour between genpd and the runtime
> PM core, let's start by converting genpd's time-accounting from ktime_get()
> into ktime_get_mono_fast_ns().
>
> Signed-off-by: Ulf Hansson <ulf.hansson@...aro.org>

Applied as 5.19 material, thanks!

> ---
>  drivers/base/power/domain.c | 77 +++++++++++++++++++++----------------
>  include/linux/pm_domain.h   |  6 +--
>  2 files changed, 46 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 1ee878d126fd..18cd7965e5d6 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -225,24 +225,23 @@ static void genpd_debug_remove(struct generic_pm_domain *genpd)
>
>  static void genpd_update_accounting(struct generic_pm_domain *genpd)
>  {
> -       ktime_t delta, now;
> +       u64 delta, now;
>
> -       now = ktime_get();
> -       delta = ktime_sub(now, genpd->accounting_time);
> +       now = ktime_get_mono_fast_ns();
> +       if (now <= genpd->accounting_time)
> +               return;
> +
> +       delta = now - genpd->accounting_time;
>
>         /*
>          * If genpd->status is active, it means we are just
>          * out of off and so update the idle time and vice
>          * versa.
>          */
> -       if (genpd->status == GENPD_STATE_ON) {
> -               int state_idx = genpd->state_idx;
> -
> -               genpd->states[state_idx].idle_time =
> -                       ktime_add(genpd->states[state_idx].idle_time, delta);
> -       } else {
> -               genpd->on_time = ktime_add(genpd->on_time, delta);
> -       }
> +       if (genpd->status == GENPD_STATE_ON)
> +               genpd->states[genpd->state_idx].idle_time += delta;
> +       else
> +               genpd->on_time += delta;
>
>         genpd->accounting_time = now;
>  }
> @@ -1999,7 +1998,7 @@ int pm_genpd_init(struct generic_pm_domain *genpd,
>         genpd->max_off_time_changed = true;
>         genpd->provider = NULL;
>         genpd->has_provider = false;
> -       genpd->accounting_time = ktime_get();
> +       genpd->accounting_time = ktime_get_mono_fast_ns();
>         genpd->domain.ops.runtime_suspend = genpd_runtime_suspend;
>         genpd->domain.ops.runtime_resume = genpd_runtime_resume;
>         genpd->domain.ops.prepare = genpd_prepare;
> @@ -3163,6 +3162,7 @@ static int sub_domains_show(struct seq_file *s, void *data)
>  static int idle_states_show(struct seq_file *s, void *data)
>  {
>         struct generic_pm_domain *genpd = s->private;
> +       u64 now, delta, idle_time = 0;
>         unsigned int i;
>         int ret = 0;
>
> @@ -3173,17 +3173,19 @@ static int idle_states_show(struct seq_file *s, void *data)
>         seq_puts(s, "State          Time Spent(ms) Usage          Rejected\n");
>
>         for (i = 0; i < genpd->state_count; i++) {
> -               ktime_t delta = 0;
> -               s64 msecs;
> +               idle_time += genpd->states[i].idle_time;
>
> -               if ((genpd->status == GENPD_STATE_OFF) &&
> -                               (genpd->state_idx == i))
> -                       delta = ktime_sub(ktime_get(), genpd->accounting_time);
> +               if (genpd->status == GENPD_STATE_OFF && genpd->state_idx == i) {
> +                       now = ktime_get_mono_fast_ns();
> +                       if (now > genpd->accounting_time) {
> +                               delta = now - genpd->accounting_time;
> +                               idle_time += delta;
> +                       }
> +               }
>
> -               msecs = ktime_to_ms(
> -                       ktime_add(genpd->states[i].idle_time, delta));
> -               seq_printf(s, "S%-13i %-14lld %-14llu %llu\n", i, msecs,
> -                             genpd->states[i].usage, genpd->states[i].rejected);
> +               do_div(idle_time, NSEC_PER_MSEC);
> +               seq_printf(s, "S%-13i %-14llu %-14llu %llu\n", i, idle_time,
> +                          genpd->states[i].usage, genpd->states[i].rejected);
>         }
>
>         genpd_unlock(genpd);
> @@ -3193,18 +3195,22 @@ static int idle_states_show(struct seq_file *s, void *data)
>  static int active_time_show(struct seq_file *s, void *data)
>  {
>         struct generic_pm_domain *genpd = s->private;
> -       ktime_t delta = 0;
> +       u64 now, on_time, delta = 0;
>         int ret = 0;
>
>         ret = genpd_lock_interruptible(genpd);
>         if (ret)
>                 return -ERESTARTSYS;
>
> -       if (genpd->status == GENPD_STATE_ON)
> -               delta = ktime_sub(ktime_get(), genpd->accounting_time);
> +       if (genpd->status == GENPD_STATE_ON) {
> +               now = ktime_get_mono_fast_ns();
> +               if (now > genpd->accounting_time)
> +                       delta = now - genpd->accounting_time;
> +       }
>
> -       seq_printf(s, "%lld ms\n", ktime_to_ms(
> -                               ktime_add(genpd->on_time, delta)));
> +       on_time = genpd->on_time + delta;
> +       do_div(on_time, NSEC_PER_MSEC);
> +       seq_printf(s, "%llu ms\n", on_time);
>
>         genpd_unlock(genpd);
>         return ret;
> @@ -3213,7 +3219,7 @@ static int active_time_show(struct seq_file *s, void *data)
>  static int total_idle_time_show(struct seq_file *s, void *data)
>  {
>         struct generic_pm_domain *genpd = s->private;
> -       ktime_t delta = 0, total = 0;
> +       u64 now, delta, total = 0;
>         unsigned int i;
>         int ret = 0;
>
> @@ -3222,16 +3228,19 @@ static int total_idle_time_show(struct seq_file *s, void *data)
>                 return -ERESTARTSYS;
>
>         for (i = 0; i < genpd->state_count; i++) {
> +               total += genpd->states[i].idle_time;
>
> -               if ((genpd->status == GENPD_STATE_OFF) &&
> -                               (genpd->state_idx == i))
> -                       delta = ktime_sub(ktime_get(), genpd->accounting_time);
> -
> -               total = ktime_add(total, genpd->states[i].idle_time);
> +               if (genpd->status == GENPD_STATE_OFF && genpd->state_idx == i) {
> +                       now = ktime_get_mono_fast_ns();
> +                       if (now > genpd->accounting_time) {
> +                               delta = now - genpd->accounting_time;
> +                               total += delta;
> +                       }
> +               }
>         }
> -       total = ktime_add(total, delta);
>
> -       seq_printf(s, "%lld ms\n", ktime_to_ms(total));
> +       do_div(total, NSEC_PER_MSEC);
> +       seq_printf(s, "%llu ms\n", total);
>
>         genpd_unlock(genpd);
>         return ret;
> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> index 67017c9390c8..043d48e4420a 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -98,7 +98,7 @@ struct genpd_power_state {
>         u64 usage;
>         u64 rejected;
>         struct fwnode_handle *fwnode;
> -       ktime_t idle_time;
> +       u64 idle_time;
>         void *data;
>  };
>
> @@ -149,8 +149,8 @@ struct generic_pm_domain {
>                             unsigned int state_count);
>         unsigned int state_count; /* number of states */
>         unsigned int state_idx; /* state that genpd will go to when off */
> -       ktime_t on_time;
> -       ktime_t accounting_time;
> +       u64 on_time;
> +       u64 accounting_time;
>         const struct genpd_lock_ops *lock_ops;
>         union {
>                 struct mutex mlock;
> --
> 2.25.1
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ