From: "Rafael J. Wysocki" <rjw@rjwysocki.net>
To: Ulf Hansson <ulf.hansson@linaro.org>
Cc: linux-pm@vger.kernel.org,
Frederic Weisbecker <fweisbec@gmail.com>,
Thomas Gleixner <tglx@linutronix.de>,
Sudeep Holla <sudeep.holla@arm.com>,
Lorenzo Pieralisi <Lorenzo.Pieralisi@arm.com>,
Mark Rutland <mark.rutland@arm.com>,
Daniel Lezcano <daniel.lezcano@linaro.org>,
"Raju P . L . S . S . S . N" <rplsssn@codeaurora.org>,
Stephen Boyd <sboyd@kernel.org>, Tony Lindgren <tony@atomide.com>,
Kevin Hilman <khilman@kernel.org>,
Lina Iyer <ilina@codeaurora.org>,
Viresh Kumar <viresh.kumar@linaro.org>,
Vincent Guittot <vincent.guittot@linaro.org>,
Geert Uytterhoeven <geert+renesas@glider.be>,
linux-arm-kernel@lists.infradead.org,
linux-arm-msm@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v12 3/4] cpuidle: Export the next timer/tick expiration for a CPU
Date: Mon, 25 Mar 2019 13:19:22 +0100 [thread overview]
Message-ID: <2942111.nn8VVVUCAL@aspire.rjw.lan> (raw)
In-Reply-To: <20190227195836.24739-4-ulf.hansson@linaro.org>
On Wednesday, February 27, 2019 8:58:35 PM CET Ulf Hansson wrote:
> To be able to predict the sleep duration for a CPU that is entering idle,
> knowing when the next timer/tick is going to expire, is extremely useful.
> Both the teo and the menu cpuidle governors already makes use of this
> information, while selecting an idle state.
>
> Moving forward, the similar prediction needs to be done, but for a group of
> idle CPUs rather than for a single idle CPU. Following changes implements a
> new genpd governor, which needs this.
>
> Support this, by sharing a new function called
> tick_nohz_get_next_hrtimer(), which returns the next hrtimer or the next
> tick, whatever that expires first.
>
> Additionally, when cpuidle is about to invoke the ->enter() callback, then
> call tick_nohz_get_next_hrtimer() and store its return value in the per CPU
> struct cpuidle_device, as to make it available outside cpuidle.
>
> Do note, at the point when cpuidle calls tick_nohz_get_next_hrtimer(), the
> governor's ->select() callback has already made a decision whether to stop
> the tick or not. In this way, tick_nohz_get_next_hrtimer() actually returns
> the next timer expiration, whatever origin.
>
> Cc: Lina Iyer <ilina@codeaurora.org>
> Co-developed-by: Lina Iyer <lina.iyer@linaro.org>
> Co-developed-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
> ---
>
> Changes in v12:
> - New patch.
>
> ---
> drivers/cpuidle/cpuidle.c | 8 ++++++++
> include/linux/cpuidle.h | 1 +
> include/linux/tick.h | 7 ++++++-
> kernel/time/tick-sched.c | 12 ++++++++++++
> 4 files changed, 27 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index 7f108309e871..255365b1a6ab 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -328,6 +328,14 @@ int cpuidle_select(struct cpuidle_driver *drv, struct cpuidle_device *dev,
> int cpuidle_enter(struct cpuidle_driver *drv, struct cpuidle_device *dev,
> int index)
> {
> + /*
> + * Store the next hrtimer, which becomes either next tick or the next
> + * timer event, whatever expires first. Additionally, to make this data
> + * useful for consumers outside cpuidle, we rely on that the governor's
> + * ->select() callback have decided, whether to stop the tick or not.
> + */
> + dev->next_hrtimer = tick_nohz_get_next_hrtimer();
I would use WRITE_ONCE() to set next_hrtimer here and READ_ONCE() for
reading that value in the next patch, as a matter of annotation if
nothing else.
> +
> if (cpuidle_state_is_coupled(drv, index))
> return cpuidle_enter_state_coupled(dev, drv, index);
> return cpuidle_enter_state(dev, drv, index);
Also I would clear next_hrtimer here to avoid dragging stale values
around.
Apart from this the series LGTM.
Thanks!
next prev parent reply other threads:[~2019-03-25 12:21 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-02-27 19:58 [PATCH v12 0/4] PM / Domains: Support hierarchical CPU arrangement (PSCI/ARM) Ulf Hansson
2019-02-27 19:58 ` [PATCH v12 1/4] PM / Domains: Add a generic data pointer to the genpd_power_state struct Ulf Hansson
2019-03-21 7:47 ` Daniel Lezcano
2019-03-21 9:36 ` Ulf Hansson
2019-03-21 9:52 ` Daniel Lezcano
2019-02-27 19:58 ` [PATCH v12 2/4] PM / Domains: Add support for CPU devices to genpd Ulf Hansson
2019-03-21 9:54 ` Daniel Lezcano
2019-02-27 19:58 ` [PATCH v12 3/4] cpuidle: Export the next timer/tick expiration for a CPU Ulf Hansson
2019-03-21 10:04 ` Daniel Lezcano
2019-03-25 12:19 ` Rafael J. Wysocki [this message]
2019-03-25 14:23 ` Ulf Hansson
2019-03-26 10:36 ` Rafael J. Wysocki
2019-03-26 11:29 ` Ulf Hansson
2019-02-27 19:58 ` [PATCH v12 4/4] PM / Domains: Add genpd governor for CPUs Ulf Hansson
2019-03-21 11:41 ` Daniel Lezcano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=2942111.nn8VVVUCAL@aspire.rjw.lan \
--to=rjw@rjwysocki.net \
--cc=Lorenzo.Pieralisi@arm.com \
--cc=daniel.lezcano@linaro.org \
--cc=fweisbec@gmail.com \
--cc=geert+renesas@glider.be \
--cc=ilina@codeaurora.org \
--cc=khilman@kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-arm-msm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-pm@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=rplsssn@codeaurora.org \
--cc=sboyd@kernel.org \
--cc=sudeep.holla@arm.com \
--cc=tglx@linutronix.de \
--cc=tony@atomide.com \
--cc=ulf.hansson@linaro.org \
--cc=vincent.guittot@linaro.org \
--cc=viresh.kumar@linaro.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).