Re: [PATCH V6] powercap/drivers/idle_injection: Add an idle injection framework

2018-06-15 Thread Daniel Lezcano
On 12/06/2018 19:35, Peter Zijlstra wrote: [ ... ] Yes, drifting is not an issue if that happens. This scheme is simpler and safer than setting the timer ahead before waking up the tasks with the risk it expires before all the tasks ended their idle cycles. >>> >>> sloppy though..

Re: [PATCH V6] powercap/drivers/idle_injection: Add an idle injection framework

2018-06-13 Thread Rafael J. Wysocki
On Wed, Jun 13, 2018 at 10:35 PM, Daniel Lezcano wrote: > On 13/06/2018 22:32, Pandruvada, Srinivas wrote: >> On Wed, 2018-06-13 at 22:19 +0200, Daniel Lezcano wrote: >>> On 13/06/2018 22:07, Pandruvada, Srinivas wrote: On Wed, 2018-06-13 at 22:00 +0200, Daniel Lezcano wrote: > On

Re: [PATCH V6] powercap/drivers/idle_injection: Add an idle injection framework

2018-06-13 Thread Daniel Lezcano
On 13/06/2018 22:32, Pandruvada, Srinivas wrote: > On Wed, 2018-06-13 at 22:19 +0200, Daniel Lezcano wrote: >> On 13/06/2018 22:07, Pandruvada, Srinivas wrote: >>> On Wed, 2018-06-13 at 22:00 +0200, Daniel Lezcano wrote: On 13/06/2018 17:54, Pandruvada, Srinivas wrote: > On Tue,

Re: [PATCH V6] powercap/drivers/idle_injection: Add an idle injection framework

2018-06-13 Thread Pandruvada, Srinivas
On Wed, 2018-06-13 at 22:19 +0200, Daniel Lezcano wrote: > On 13/06/2018 22:07, Pandruvada, Srinivas wrote: > > On Wed, 2018-06-13 at 22:00 +0200, Daniel Lezcano wrote: > > > On 13/06/2018 17:54, Pandruvada, Srinivas wrote: > > > > On Tue, 2018-06-12 at 14:00 +0200, Daniel Lezcano wrote: > > > > >

Re: [PATCH V6] powercap/drivers/idle_injection: Add an idle injection framework

2018-06-13 Thread Daniel Lezcano
On 13/06/2018 22:07, Pandruvada, Srinivas wrote: > On Wed, 2018-06-13 at 22:00 +0200, Daniel Lezcano wrote: >> On 13/06/2018 17:54, Pandruvada, Srinivas wrote: >>> On Tue, 2018-06-12 at 14:00 +0200, Daniel Lezcano wrote: Initially, the cpu_cooling device for ARM was changed by adding a

Re: [PATCH V6] powercap/drivers/idle_injection: Add an idle injection framework

2018-06-13 Thread Pandruvada, Srinivas
On Wed, 2018-06-13 at 22:00 +0200, Daniel Lezcano wrote: > On 13/06/2018 17:54, Pandruvada, Srinivas wrote: > > On Tue, 2018-06-12 at 14:00 +0200, Daniel Lezcano wrote: > > > Initially, the cpu_cooling device for ARM was changed by adding a > > > new > > > policy inserting idle cycles. The

Re: [PATCH V6] powercap/drivers/idle_injection: Add an idle injection framework

2018-06-13 Thread Daniel Lezcano
On 13/06/2018 17:54, Pandruvada, Srinivas wrote: > On Tue, 2018-06-12 at 14:00 +0200, Daniel Lezcano wrote: >> Initially, the cpu_cooling device for ARM was changed by adding a new >> policy inserting idle cycles. The intel_powerclamp driver does a >> similar action. >> >> Instead of implementing

Re: [PATCH V6] powercap/drivers/idle_injection: Add an idle injection framework

2018-06-13 Thread Pandruvada, Srinivas
On Tue, 2018-06-12 at 14:00 +0200, Daniel Lezcano wrote: > Initially, the cpu_cooling device for ARM was changed by adding a new > policy inserting idle cycles. The intel_powerclamp driver does a > similar action. > > Instead of implementing idle injections privately in the cpu_cooling > device,

Re: [PATCH V6] powercap/drivers/idle_injection: Add an idle injection framework

2018-06-13 Thread Viresh Kumar
On 13-06-18, 11:03, Daniel Lezcano wrote: > nr_threads(smpboot) <> nr_threads(idleinject) > > If we are facing races issues, it is because we are trying to avoid > using locks in the code path. With lock and proper refcounting that > should be solved, AFAICT there are similar races with inodes.

Re: [PATCH V6] powercap/drivers/idle_injection: Add an idle injection framework

2018-06-13 Thread Daniel Lezcano
On 13/06/2018 10:55, Viresh Kumar wrote: > On 12-06-18, 19:35, Peter Zijlstra wrote: >> On Tue, Jun 12, 2018 at 07:02:57PM +0200, Daniel Lezcano wrote: >>> Mmh, it is unclear for me if the park() vs wakeup() can happen at the >>> same time. >>> >>> If the park() function is called, that means the

Re: [PATCH V6] powercap/drivers/idle_injection: Add an idle injection framework

2018-06-13 Thread Viresh Kumar
On 12-06-18, 19:35, Peter Zijlstra wrote: > On Tue, Jun 12, 2018 at 07:02:57PM +0200, Daniel Lezcano wrote: > > Mmh, it is unclear for me if the park() vs wakeup() can happen at the > > same time. > > > > If the park() function is called, that means the hotplug is allowed. > > No, it means we're

Re: [PATCH V6] powercap/drivers/idle_injection: Add an idle injection framework

2018-06-13 Thread Viresh Kumar
On 12-06-18, 14:59, Peter Zijlstra wrote: > On Tue, Jun 12, 2018 at 02:00:11PM +0200, Daniel Lezcano wrote: > > +struct idle_injection_device { > > remove this: > > + cpumask_var_t cpumask; > > > + struct hrtimer timer; > > + struct completion stop_complete; > > + unsigned int

Re: [PATCH V6] powercap/drivers/idle_injection: Add an idle injection framework

2018-06-12 Thread Peter Zijlstra
On Tue, Jun 12, 2018 at 07:02:57PM +0200, Daniel Lezcano wrote: > Mmh, it is unclear for me if the park() vs wakeup() can happen at the > same time. > > If the park() function is called, that means the hotplug is allowed. No, it means we're inside hot-un-plug, but that doesn't stop the hrtimer

Re: [PATCH V6] powercap/drivers/idle_injection: Add an idle injection framework

2018-06-12 Thread Daniel Lezcano
On 12/06/2018 17:58, Peter Zijlstra wrote: > On Tue, Jun 12, 2018 at 04:37:17PM +0200, Daniel Lezcano wrote: >> On 12/06/2018 16:06, Peter Zijlstra wrote: >>> On Tue, Jun 12, 2018 at 03:02:14PM +0200, Daniel Lezcano wrote: On 12/06/2018 14:52, Peter Zijlstra wrote: > In this case, you can

Re: [PATCH V6] powercap/drivers/idle_injection: Add an idle injection framework

2018-06-12 Thread Peter Zijlstra
On Tue, Jun 12, 2018 at 04:37:17PM +0200, Daniel Lezcano wrote: > On 12/06/2018 16:06, Peter Zijlstra wrote: > > On Tue, Jun 12, 2018 at 03:02:14PM +0200, Daniel Lezcano wrote: > >> On 12/06/2018 14:52, Peter Zijlstra wrote: > >>> In this case, you can do: > >> > >> That is what we had before but

Re: [PATCH V6] powercap/drivers/idle_injection: Add an idle injection framework

2018-06-12 Thread Daniel Lezcano
On 12/06/2018 16:26, Peter Zijlstra wrote: > On Tue, Jun 12, 2018 at 02:00:11PM +0200, Daniel Lezcano wrote: >> +void idle_injection_stop(struct idle_injection_device *ii_dev) >> +{ >> +pr_debug("Stopping injecting idle cycles on CPUs '%*pbl'\n", >> +

Re: [PATCH V6] powercap/drivers/idle_injection: Add an idle injection framework

2018-06-12 Thread Daniel Lezcano
On 12/06/2018 16:23, Peter Zijlstra wrote: > On Tue, Jun 12, 2018 at 02:00:11PM +0200, Daniel Lezcano wrote: >> +static void idle_injection_last_man(struct idle_injection_device *ii_dev) >> +{ >> +unsigned int run_duration_ms; >> + >> +run_duration_ms = READ_ONCE(ii_dev->run_duration_ms);

Re: [PATCH V6] powercap/drivers/idle_injection: Add an idle injection framework

2018-06-12 Thread Daniel Lezcano
On 12/06/2018 16:06, Peter Zijlstra wrote: > On Tue, Jun 12, 2018 at 03:02:14PM +0200, Daniel Lezcano wrote: >> On 12/06/2018 14:52, Peter Zijlstra wrote: >>> In this case, you can do: >> >> That is what we had before but we change the code to set the count >> before waking up the task, so compute

Re: [PATCH V6] powercap/drivers/idle_injection: Add an idle injection framework

2018-06-12 Thread Peter Zijlstra
On Tue, Jun 12, 2018 at 02:00:11PM +0200, Daniel Lezcano wrote: > +void idle_injection_stop(struct idle_injection_device *ii_dev) > +{ > + pr_debug("Stopping injecting idle cycles on CPUs '%*pbl'\n", > + cpumask_pr_args(ii_dev->cpumask)); > + > +

Re: [PATCH V6] powercap/drivers/idle_injection: Add an idle injection framework

2018-06-12 Thread Peter Zijlstra
On Tue, Jun 12, 2018 at 02:00:11PM +0200, Daniel Lezcano wrote: > +static void idle_injection_last_man(struct idle_injection_device *ii_dev) > +{ > + unsigned int run_duration_ms; > + > + run_duration_ms = READ_ONCE(ii_dev->run_duration_ms); > + if (run_duration_ms) { > +

Re: [PATCH V6] powercap/drivers/idle_injection: Add an idle injection framework

2018-06-12 Thread Peter Zijlstra
On Tue, Jun 12, 2018 at 03:02:14PM +0200, Daniel Lezcano wrote: > On 12/06/2018 14:52, Peter Zijlstra wrote: > > In this case, you can do: > > That is what we had before but we change the code to set the count > before waking up the task, so compute the cpumask_weight of the > resulting AND right

Re: [PATCH V6] powercap/drivers/idle_injection: Add an idle injection framework

2018-06-12 Thread Daniel Lezcano
On 12/06/2018 14:52, Peter Zijlstra wrote: > On Tue, Jun 12, 2018 at 02:44:29PM +0200, Daniel Lezcano wrote: >> On 12/06/2018 14:30, Peter Zijlstra wrote: >>> On Tue, Jun 12, 2018 at 02:00:11PM +0200, Daniel Lezcano wrote: +static void __idle_injection_wakeup(struct idle_injection_device

Re: [PATCH V6] powercap/drivers/idle_injection: Add an idle injection framework

2018-06-12 Thread Peter Zijlstra
On Tue, Jun 12, 2018 at 02:00:11PM +0200, Daniel Lezcano wrote: > +struct idle_injection_device { remove this: > + cpumask_var_t cpumask; > + struct hrtimer timer; > + struct completion stop_complete; > + unsigned int idle_duration_ms; > + unsigned int run_duration_ms; > +

Re: [PATCH V6] powercap/drivers/idle_injection: Add an idle injection framework

2018-06-12 Thread Peter Zijlstra
On Tue, Jun 12, 2018 at 02:44:29PM +0200, Daniel Lezcano wrote: > On 12/06/2018 14:30, Peter Zijlstra wrote: > > On Tue, Jun 12, 2018 at 02:00:11PM +0200, Daniel Lezcano wrote: > >> +static void __idle_injection_wakeup(struct idle_injection_device *ii_dev) > >> +{ > >> + struct

Re: [PATCH V6] powercap/drivers/idle_injection: Add an idle injection framework

2018-06-12 Thread Daniel Lezcano
On 12/06/2018 14:30, Peter Zijlstra wrote: > On Tue, Jun 12, 2018 at 02:00:11PM +0200, Daniel Lezcano wrote: >> +static void __idle_injection_wakeup(struct idle_injection_device *ii_dev) >> +{ >> +struct idle_injection_thread *iit; >> +struct cpumask tmp; >> +unsigned int cpu; >> + >>

Re: [PATCH V6] powercap/drivers/idle_injection: Add an idle injection framework

2018-06-12 Thread Peter Zijlstra
On Tue, Jun 12, 2018 at 02:00:11PM +0200, Daniel Lezcano wrote: > +static void __idle_injection_wakeup(struct idle_injection_device *ii_dev) > +{ > + struct idle_injection_thread *iit; > + struct cpumask tmp; > + unsigned int cpu; > + > + cpumask_and(, ii_dev->cpumask,