* [PATCH 2/2] powercap/drivers/idle_inject: Use a higher resolution for the idle injection
2019-08-02 17:34 [PATCH 1/2] cpuidle: play_idle: Increase the resolution to usec Daniel Lezcano
@ 2019-08-02 17:34 ` Daniel Lezcano
2019-08-12 8:59 ` [PATCH 1/2] cpuidle: play_idle: Increase the resolution to usec Daniel Lezcano
1 sibling, 0 replies; 6+ messages in thread
From: Daniel Lezcano @ 2019-08-02 17:34 UTC (permalink / raw)
To: rafael; +Cc: linux-pm
The resolution of the idle injection is limited to 1ms. If there is a
need for an injection of 1.2ms, it is not possible.
The idle injection API is not yet used, so it is safe to convert the
existing API to the new time unit instead of adding more functions.
Convert to microsecond in order to use a finer grain time unit when
injecting idle cycles.
Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
---
drivers/powercap/idle_inject.c | 53 +++++++++++++++++-----------------
include/linux/idle_inject.h | 8 ++---
2 files changed, 31 insertions(+), 30 deletions(-)
diff --git a/drivers/powercap/idle_inject.c b/drivers/powercap/idle_inject.c
index 10601f4bdf72..cd1270614cc6 100644
--- a/drivers/powercap/idle_inject.c
+++ b/drivers/powercap/idle_inject.c
@@ -59,14 +59,14 @@ struct idle_inject_thread {
/**
* struct idle_inject_device - idle injection data
* @timer: idle injection period timer
- * @idle_duration_ms: duration of CPU idle time to inject
- * @run_duration_ms: duration of CPU run time to allow
+ * @idle_duration_us: duration of CPU idle time to inject
+ * @run_duration_us: duration of CPU run time to allow
* @cpumask: mask of CPUs affected by idle injection
*/
struct idle_inject_device {
struct hrtimer timer;
- unsigned int idle_duration_ms;
- unsigned int run_duration_ms;
+ unsigned int idle_duration_us;
+ unsigned int run_duration_us;
unsigned long int cpumask[0];
};
@@ -104,16 +104,16 @@ static void idle_inject_wakeup(struct idle_inject_device *ii_dev)
*/
static enum hrtimer_restart idle_inject_timer_fn(struct hrtimer *timer)
{
- unsigned int duration_ms;
+ unsigned int duration_us;
struct idle_inject_device *ii_dev =
container_of(timer, struct idle_inject_device, timer);
- duration_ms = READ_ONCE(ii_dev->run_duration_ms);
- duration_ms += READ_ONCE(ii_dev->idle_duration_ms);
+ duration_us = READ_ONCE(ii_dev->run_duration_us);
+ duration_us += READ_ONCE(ii_dev->idle_duration_us);
idle_inject_wakeup(ii_dev);
- hrtimer_forward_now(timer, ms_to_ktime(duration_ms));
+ hrtimer_forward_now(timer, ns_to_ktime(duration_us * NSEC_PER_USEC));
return HRTIMER_RESTART;
}
@@ -138,35 +138,35 @@ static void idle_inject_fn(unsigned int cpu)
*/
iit->should_run = 0;
- play_idle(READ_ONCE(ii_dev->idle_duration_ms) * USEC_PER_MSEC);
+ play_idle(READ_ONCE(ii_dev->idle_duration_us));
}
/**
* idle_inject_set_duration - idle and run duration update helper
- * @run_duration_ms: CPU run time to allow in milliseconds
- * @idle_duration_ms: CPU idle time to inject in milliseconds
+ * @run_duration_us: CPU run time to allow in microseconds
+ * @idle_duration_us: CPU idle time to inject in microseconds
*/
void idle_inject_set_duration(struct idle_inject_device *ii_dev,
- unsigned int run_duration_ms,
- unsigned int idle_duration_ms)
+ unsigned int run_duration_us,
+ unsigned int idle_duration_us)
{
- if (run_duration_ms && idle_duration_ms) {
- WRITE_ONCE(ii_dev->run_duration_ms, run_duration_ms);
- WRITE_ONCE(ii_dev->idle_duration_ms, idle_duration_ms);
+ if (run_duration_us && idle_duration_us) {
+ WRITE_ONCE(ii_dev->run_duration_us, run_duration_us);
+ WRITE_ONCE(ii_dev->idle_duration_us, idle_duration_us);
}
}
/**
* idle_inject_get_duration - idle and run duration retrieval helper
- * @run_duration_ms: memory location to store the current CPU run time
- * @idle_duration_ms: memory location to store the current CPU idle time
+ * @run_duration_us: memory location to store the current CPU run time
+ * @idle_duration_us: memory location to store the current CPU idle time
*/
void idle_inject_get_duration(struct idle_inject_device *ii_dev,
- unsigned int *run_duration_ms,
- unsigned int *idle_duration_ms)
+ unsigned int *run_duration_us,
+ unsigned int *idle_duration_us)
{
- *run_duration_ms = READ_ONCE(ii_dev->run_duration_ms);
- *idle_duration_ms = READ_ONCE(ii_dev->idle_duration_ms);
+ *run_duration_us = READ_ONCE(ii_dev->run_duration_us);
+ *idle_duration_us = READ_ONCE(ii_dev->idle_duration_us);
}
/**
@@ -181,10 +181,10 @@ void idle_inject_get_duration(struct idle_inject_device *ii_dev,
*/
int idle_inject_start(struct idle_inject_device *ii_dev)
{
- unsigned int idle_duration_ms = READ_ONCE(ii_dev->idle_duration_ms);
- unsigned int run_duration_ms = READ_ONCE(ii_dev->run_duration_ms);
+ unsigned int idle_duration_us = READ_ONCE(ii_dev->idle_duration_us);
+ unsigned int run_duration_us = READ_ONCE(ii_dev->run_duration_us);
- if (!idle_duration_ms || !run_duration_ms)
+ if (!idle_duration_us || !run_duration_us)
return -EINVAL;
pr_debug("Starting injecting idle cycles on CPUs '%*pbl'\n",
@@ -193,7 +193,8 @@ int idle_inject_start(struct idle_inject_device *ii_dev)
idle_inject_wakeup(ii_dev);
hrtimer_start(&ii_dev->timer,
- ms_to_ktime(idle_duration_ms + run_duration_ms),
+ ns_to_ktime((idle_duration_us + run_duration_us) *
+ NSEC_PER_USEC),
HRTIMER_MODE_REL);
return 0;
diff --git a/include/linux/idle_inject.h b/include/linux/idle_inject.h
index bdc0293fb6cb..a445cd1a36c5 100644
--- a/include/linux/idle_inject.h
+++ b/include/linux/idle_inject.h
@@ -20,10 +20,10 @@ int idle_inject_start(struct idle_inject_device *ii_dev);
void idle_inject_stop(struct idle_inject_device *ii_dev);
void idle_inject_set_duration(struct idle_inject_device *ii_dev,
- unsigned int run_duration_ms,
- unsigned int idle_duration_ms);
+ unsigned int run_duration_us,
+ unsigned int idle_duration_us);
void idle_inject_get_duration(struct idle_inject_device *ii_dev,
- unsigned int *run_duration_ms,
- unsigned int *idle_duration_ms);
+ unsigned int *run_duration_us,
+ unsigned int *idle_duration_us);
#endif /* __IDLE_INJECT_H__ */
--
2.17.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 1/2] cpuidle: play_idle: Increase the resolution to usec
2019-08-02 17:34 [PATCH 1/2] cpuidle: play_idle: Increase the resolution to usec Daniel Lezcano
2019-08-02 17:34 ` [PATCH 2/2] powercap/drivers/idle_inject: Use a higher resolution for the idle injection Daniel Lezcano
@ 2019-08-12 8:59 ` Daniel Lezcano
2019-08-12 9:01 ` Rafael J. Wysocki
1 sibling, 1 reply; 6+ messages in thread
From: Daniel Lezcano @ 2019-08-12 8:59 UTC (permalink / raw)
To: rafael; +Cc: linux-pm
Hi Rafael,
Can you consider these two patches for merging. There is no functional
changes.
On 02/08/2019 19:34, Daniel Lezcano wrote:
> The play_idle resolution is 1ms. The intel_powerclamp bases the idle
> duration on jiffies. The idle injection API is also using msec based
> duration but has no user yet.
>
> Unfortunately, msec based time does not fit well when we want to
> inject idle cycle precisely with shallow idle state.
>
> In order to set the scene for the incoming idle injection user, move
> the precision up to usec when calling play_idle.
>
> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org>
> ---
> drivers/powercap/idle_inject.c | 2 +-
> drivers/thermal/intel/intel_powerclamp.c | 2 +-
> include/linux/cpu.h | 2 +-
> kernel/sched/idle.c | 7 ++++---
> 4 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/powercap/idle_inject.c b/drivers/powercap/idle_inject.c
> index 24ff2a068978..10601f4bdf72 100644
> --- a/drivers/powercap/idle_inject.c
> +++ b/drivers/powercap/idle_inject.c
> @@ -138,7 +138,7 @@ static void idle_inject_fn(unsigned int cpu)
> */
> iit->should_run = 0;
>
> - play_idle(READ_ONCE(ii_dev->idle_duration_ms));
> + play_idle(READ_ONCE(ii_dev->idle_duration_ms) * USEC_PER_MSEC);
> }
>
> /**
> diff --git a/drivers/thermal/intel/intel_powerclamp.c b/drivers/thermal/intel/intel_powerclamp.c
> index 5149a817456b..53216dcbe173 100644
> --- a/drivers/thermal/intel/intel_powerclamp.c
> +++ b/drivers/thermal/intel/intel_powerclamp.c
> @@ -430,7 +430,7 @@ static void clamp_idle_injection_func(struct kthread_work *work)
> if (should_skip)
> goto balance;
>
> - play_idle(jiffies_to_msecs(w_data->duration_jiffies));
> + play_idle(jiffies_to_usecs(w_data->duration_jiffies));
>
> balance:
> if (clamping && w_data->clamping && cpu_online(w_data->cpu))
> diff --git a/include/linux/cpu.h b/include/linux/cpu.h
> index fcb1386bb0d4..88dc0c653925 100644
> --- a/include/linux/cpu.h
> +++ b/include/linux/cpu.h
> @@ -179,7 +179,7 @@ void arch_cpu_idle_dead(void);
> int cpu_report_state(int cpu);
> int cpu_check_up_prepare(int cpu);
> void cpu_set_state_online(int cpu);
> -void play_idle(unsigned long duration_ms);
> +void play_idle(unsigned long duration_us);
>
> #ifdef CONFIG_HOTPLUG_CPU
> bool cpu_wait_death(unsigned int cpu, int seconds);
> diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
> index 80940939b733..b98283fc6914 100644
> --- a/kernel/sched/idle.c
> +++ b/kernel/sched/idle.c
> @@ -311,7 +311,7 @@ static enum hrtimer_restart idle_inject_timer_fn(struct hrtimer *timer)
> return HRTIMER_NORESTART;
> }
>
> -void play_idle(unsigned long duration_ms)
> +void play_idle(unsigned long duration_us)
> {
> struct idle_timer it;
>
> @@ -323,7 +323,7 @@ void play_idle(unsigned long duration_ms)
> WARN_ON_ONCE(current->nr_cpus_allowed != 1);
> WARN_ON_ONCE(!(current->flags & PF_KTHREAD));
> WARN_ON_ONCE(!(current->flags & PF_NO_SETAFFINITY));
> - WARN_ON_ONCE(!duration_ms);
> + WARN_ON_ONCE(!duration_us);
>
> rcu_sleep_check();
> preempt_disable();
> @@ -333,7 +333,8 @@ void play_idle(unsigned long duration_ms)
> it.done = 0;
> hrtimer_init_on_stack(&it.timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
> it.timer.function = idle_inject_timer_fn;
> - hrtimer_start(&it.timer, ms_to_ktime(duration_ms), HRTIMER_MODE_REL_PINNED);
> + hrtimer_start(&it.timer, ns_to_ktime(duration_us * NSEC_PER_USEC),
> + HRTIMER_MODE_REL_PINNED);
>
> while (!READ_ONCE(it.done))
> do_idle();
>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 6+ messages in thread