[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAJZ5v0im-pdwemdx3B3SaF+PBoaLDWOdTT7-6kwn2GB0rW0kCQ@mail.gmail.com>
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