Re: [PATCH][RFC v2] ACPI: acpi_pad: Do not launch acpi_pad threads on idle cpus
Hi, On Tue, Dec 11, 2018 at 04:37:54PM +0800, joeyli wrote: > Hi Yu Chen, > > Thanks for your response! > > On Tue, Dec 11, 2018 at 11:12:21AM +0800, Yu Chen wrote: > > Hi Joey, > > On Mon, Dec 10, 2018 at 02:31:53PM +0800, joeyli wrote: > > > Hi Chen Yu and ACPI experts, > > > > > > On Sat, May 05, 2018 at 07:53:22PM +0800, Chen Yu wrote: > > > > According to current implementation of acpi_pad driver, > > > > it does not make sense to spawn any power saving threads > > > > on the cpus which are already idle - it might bring > > > > unnecessary overhead on these idle cpus and causes power > > > > waste. So verify the condition that if the number of 'busy' > > > > cpus exceeds the amount of the 'forced idle' cpus is met. > > > > This is applicable due to round-robin attribute of the > > > > power saving threads, otherwise ignore the setting/ACPI > > > > notification. > > > > > > > > Suggested-by: Lenny Szubowicz > > > > Suggested-by: Len Brown > > > > Cc: "Rafael J. Wysocki" > > > > Cc: Lenny Szubowicz > > > > Cc: Len Brown > > > > Cc: Jacob Pan > > > > Cc: Rui Zhang > > > > Cc: linux-a...@vger.kernel.org > > > > Signed-off-by: Chen Yu > > > > > > Do you have any news for this patch? Why it did not merged by kernel > > > maineline? > > > > > We are evaluating if this could be integrated into idle injection framework. > > May I know if there's any requirement/background from SUSE on this? > > > > I am also looking at your patch and idle injection framework. Currently I do > not > have good suggestion for your patches yet. But I will try to ready my > knowledge when > you send out new version. > Thanks. I mean, does SUSE get report from customers who encountered this issue? BTW, may I know the status of the encryption hibernation please?(Usin the TPM?) Best, Yu(Ryan) > Thanks a lot! > Joey Lee
Re: [PATCH][RFC v2] ACPI: acpi_pad: Do not launch acpi_pad threads on idle cpus
Hi Yu Chen,
Thanks for your response!
On Tue, Dec 11, 2018 at 11:12:21AM +0800, Yu Chen wrote:
> Hi Joey,
> On Mon, Dec 10, 2018 at 02:31:53PM +0800, joeyli wrote:
> > Hi Chen Yu and ACPI experts,
> >
> > On Sat, May 05, 2018 at 07:53:22PM +0800, Chen Yu wrote:
> > > According to current implementation of acpi_pad driver,
> > > it does not make sense to spawn any power saving threads
> > > on the cpus which are already idle - it might bring
> > > unnecessary overhead on these idle cpus and causes power
> > > waste. So verify the condition that if the number of 'busy'
> > > cpus exceeds the amount of the 'forced idle' cpus is met.
> > > This is applicable due to round-robin attribute of the
> > > power saving threads, otherwise ignore the setting/ACPI
> > > notification.
> > >
> > > Suggested-by: Lenny Szubowicz
> > > Suggested-by: Len Brown
> > > Cc: "Rafael J. Wysocki"
> > > Cc: Lenny Szubowicz
> > > Cc: Len Brown
> > > Cc: Jacob Pan
> > > Cc: Rui Zhang
> > > Cc: linux-a...@vger.kernel.org
> > > Signed-off-by: Chen Yu
> >
> > Do you have any news for this patch? Why it did not merged by kernel
> > maineline?
> >
> We are evaluating if this could be integrated into idle injection framework.
> May I know if there's any requirement/background from SUSE on this?
>
I am also looking at your patch and idle injection framework. Currently I do not
have good suggestion for your patches yet. But I will try to ready my knowledge
when
you send out new version.
Thanks a lot!
Joey Lee
> >
> > > ---
> > > drivers/acpi/acpi_pad.c | 52
> > > -
> > > 1 file changed, 51 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/acpi/acpi_pad.c b/drivers/acpi/acpi_pad.c
> > > index 552c1f7..515e60e 100644
> > > --- a/drivers/acpi/acpi_pad.c
> > > +++ b/drivers/acpi/acpi_pad.c
> > > @@ -254,12 +254,62 @@ static void set_power_saving_task_num(unsigned int
> > > num)
> > > }
> > > }
> > >
> > > +/*
> > > + * Extra acpi_pad threads should not be created until
> > > + * the requested idle count is less than/equals to the
> > > + * number of the busy cpus - it does not make sense to
> > > + * throttle the idle cpus.
> > > + */
> > > +#define SAMPLE_INTERVAL_JIF 20
> > > +
> > > +static u64 get_idle_time(int cpu)
> > > +{
> > > + u64 idle, idle_usecs = -1ULL;
> > > +
> > > + idle_usecs = get_cpu_idle_time_us(cpu, NULL);
> > > +
> > > + if (idle_usecs == -1ULL)
> > > + idle = kcpustat_cpu(cpu).cpustat[CPUTIME_IDLE];
> > > + else
> > > + idle = idle_usecs * NSEC_PER_USEC;
> > > +
> > > + return idle;
> > > +}
> > > +
> > > +static bool idle_nr_valid(unsigned int num_cpus)
> > > +{
> > > + int busy_nr = 0, i = 0, load_thresh = 100 - idle_pct;
> > > +
> > > + if (!num_cpus)
> > > + return true;
> > > +
> > > + for_each_online_cpu(i) {
> > > + u64 wall_time, idle_time;
> > > + unsigned int elapsed_delta, idle_delta, load;
> > > +
> > > + wall_time = jiffies64_to_nsecs(get_jiffies_64());
> > > + idle_time = get_idle_time(i);
> > > + /* Wait and see... */
> > > + schedule_timeout_uninterruptible(SAMPLE_INTERVAL_JIF);
> > > +
> > > + idle_delta = get_idle_time(i) - idle_time;
> > > + elapsed_delta = jiffies64_to_nsecs(get_jiffies_64()) -
> > > wall_time;
> > > + idle_delta = (idle_delta > elapsed_delta) ? elapsed_delta :
> > > idle_delta;
> > > + load = 100 * (elapsed_delta - idle_delta) / elapsed_delta;
> > > + if (load >= load_thresh)
> > > + busy_nr++;
> > > + }
> > > +
> > > + return (busy_nr >= num_cpus) ? true : false;
> > > +}
> > > +
> > > static void acpi_pad_idle_cpus(unsigned int num_cpus)
> > > {
> > > get_online_cpus();
> > >
> > > num_cpus = min_t(unsigned int, num_cpus, num_online_cpus());
> > > - set_power_saving_task_num(num_cpus);
> > > + if (idle_nr_valid(num_cpus))
> > > + set_power_saving_task_num(num_cpus);
> > >
> > > put_online_cpus();
> > > }
> > > --
> > > 2.7.4
> > >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> > > the body of a message to majord...@vger.kernel.org
> > > More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH][RFC v2] ACPI: acpi_pad: Do not launch acpi_pad threads on idle cpus
Hi Joey,
On Mon, Dec 10, 2018 at 02:31:53PM +0800, joeyli wrote:
> Hi Chen Yu and ACPI experts,
>
> On Sat, May 05, 2018 at 07:53:22PM +0800, Chen Yu wrote:
> > According to current implementation of acpi_pad driver,
> > it does not make sense to spawn any power saving threads
> > on the cpus which are already idle - it might bring
> > unnecessary overhead on these idle cpus and causes power
> > waste. So verify the condition that if the number of 'busy'
> > cpus exceeds the amount of the 'forced idle' cpus is met.
> > This is applicable due to round-robin attribute of the
> > power saving threads, otherwise ignore the setting/ACPI
> > notification.
> >
> > Suggested-by: Lenny Szubowicz
> > Suggested-by: Len Brown
> > Cc: "Rafael J. Wysocki"
> > Cc: Lenny Szubowicz
> > Cc: Len Brown
> > Cc: Jacob Pan
> > Cc: Rui Zhang
> > Cc: linux-a...@vger.kernel.org
> > Signed-off-by: Chen Yu
>
> Do you have any news for this patch? Why it did not merged by kernel
> maineline?
>
We are evaluating if this could be integrated into idle injection framework.
May I know if there's any requirement/background from SUSE on this?
Best,
Ryan(Yu)
> Thanks a lot!
> Joey Lee
>
> > ---
> > drivers/acpi/acpi_pad.c | 52
> > -
> > 1 file changed, 51 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/acpi/acpi_pad.c b/drivers/acpi/acpi_pad.c
> > index 552c1f7..515e60e 100644
> > --- a/drivers/acpi/acpi_pad.c
> > +++ b/drivers/acpi/acpi_pad.c
> > @@ -254,12 +254,62 @@ static void set_power_saving_task_num(unsigned int
> > num)
> > }
> > }
> >
> > +/*
> > + * Extra acpi_pad threads should not be created until
> > + * the requested idle count is less than/equals to the
> > + * number of the busy cpus - it does not make sense to
> > + * throttle the idle cpus.
> > + */
> > +#define SAMPLE_INTERVAL_JIF20
> > +
> > +static u64 get_idle_time(int cpu)
> > +{
> > + u64 idle, idle_usecs = -1ULL;
> > +
> > + idle_usecs = get_cpu_idle_time_us(cpu, NULL);
> > +
> > + if (idle_usecs == -1ULL)
> > + idle = kcpustat_cpu(cpu).cpustat[CPUTIME_IDLE];
> > + else
> > + idle = idle_usecs * NSEC_PER_USEC;
> > +
> > + return idle;
> > +}
> > +
> > +static bool idle_nr_valid(unsigned int num_cpus)
> > +{
> > + int busy_nr = 0, i = 0, load_thresh = 100 - idle_pct;
> > +
> > + if (!num_cpus)
> > + return true;
> > +
> > + for_each_online_cpu(i) {
> > + u64 wall_time, idle_time;
> > + unsigned int elapsed_delta, idle_delta, load;
> > +
> > + wall_time = jiffies64_to_nsecs(get_jiffies_64());
> > + idle_time = get_idle_time(i);
> > + /* Wait and see... */
> > + schedule_timeout_uninterruptible(SAMPLE_INTERVAL_JIF);
> > +
> > + idle_delta = get_idle_time(i) - idle_time;
> > + elapsed_delta = jiffies64_to_nsecs(get_jiffies_64()) -
> > wall_time;
> > + idle_delta = (idle_delta > elapsed_delta) ? elapsed_delta :
> > idle_delta;
> > + load = 100 * (elapsed_delta - idle_delta) / elapsed_delta;
> > + if (load >= load_thresh)
> > + busy_nr++;
> > + }
> > +
> > + return (busy_nr >= num_cpus) ? true : false;
> > +}
> > +
> > static void acpi_pad_idle_cpus(unsigned int num_cpus)
> > {
> > get_online_cpus();
> >
> > num_cpus = min_t(unsigned int, num_cpus, num_online_cpus());
> > - set_power_saving_task_num(num_cpus);
> > + if (idle_nr_valid(num_cpus))
> > + set_power_saving_task_num(num_cpus);
> >
> > put_online_cpus();
> > }
> > --
> > 2.7.4
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH][RFC v2] ACPI: acpi_pad: Do not launch acpi_pad threads on idle cpus
Hi Chen Yu and ACPI experts,
On Sat, May 05, 2018 at 07:53:22PM +0800, Chen Yu wrote:
> According to current implementation of acpi_pad driver,
> it does not make sense to spawn any power saving threads
> on the cpus which are already idle - it might bring
> unnecessary overhead on these idle cpus and causes power
> waste. So verify the condition that if the number of 'busy'
> cpus exceeds the amount of the 'forced idle' cpus is met.
> This is applicable due to round-robin attribute of the
> power saving threads, otherwise ignore the setting/ACPI
> notification.
>
> Suggested-by: Lenny Szubowicz
> Suggested-by: Len Brown
> Cc: "Rafael J. Wysocki"
> Cc: Lenny Szubowicz
> Cc: Len Brown
> Cc: Jacob Pan
> Cc: Rui Zhang
> Cc: linux-a...@vger.kernel.org
> Signed-off-by: Chen Yu
Do you have any news for this patch? Why it did not merged by kernel
maineline?
Thanks a lot!
Joey Lee
> ---
> drivers/acpi/acpi_pad.c | 52
> -
> 1 file changed, 51 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/acpi/acpi_pad.c b/drivers/acpi/acpi_pad.c
> index 552c1f7..515e60e 100644
> --- a/drivers/acpi/acpi_pad.c
> +++ b/drivers/acpi/acpi_pad.c
> @@ -254,12 +254,62 @@ static void set_power_saving_task_num(unsigned int num)
> }
> }
>
> +/*
> + * Extra acpi_pad threads should not be created until
> + * the requested idle count is less than/equals to the
> + * number of the busy cpus - it does not make sense to
> + * throttle the idle cpus.
> + */
> +#define SAMPLE_INTERVAL_JIF 20
> +
> +static u64 get_idle_time(int cpu)
> +{
> + u64 idle, idle_usecs = -1ULL;
> +
> + idle_usecs = get_cpu_idle_time_us(cpu, NULL);
> +
> + if (idle_usecs == -1ULL)
> + idle = kcpustat_cpu(cpu).cpustat[CPUTIME_IDLE];
> + else
> + idle = idle_usecs * NSEC_PER_USEC;
> +
> + return idle;
> +}
> +
> +static bool idle_nr_valid(unsigned int num_cpus)
> +{
> + int busy_nr = 0, i = 0, load_thresh = 100 - idle_pct;
> +
> + if (!num_cpus)
> + return true;
> +
> + for_each_online_cpu(i) {
> + u64 wall_time, idle_time;
> + unsigned int elapsed_delta, idle_delta, load;
> +
> + wall_time = jiffies64_to_nsecs(get_jiffies_64());
> + idle_time = get_idle_time(i);
> + /* Wait and see... */
> + schedule_timeout_uninterruptible(SAMPLE_INTERVAL_JIF);
> +
> + idle_delta = get_idle_time(i) - idle_time;
> + elapsed_delta = jiffies64_to_nsecs(get_jiffies_64()) -
> wall_time;
> + idle_delta = (idle_delta > elapsed_delta) ? elapsed_delta :
> idle_delta;
> + load = 100 * (elapsed_delta - idle_delta) / elapsed_delta;
> + if (load >= load_thresh)
> + busy_nr++;
> + }
> +
> + return (busy_nr >= num_cpus) ? true : false;
> +}
> +
> static void acpi_pad_idle_cpus(unsigned int num_cpus)
> {
> get_online_cpus();
>
> num_cpus = min_t(unsigned int, num_cpus, num_online_cpus());
> - set_power_saving_task_num(num_cpus);
> + if (idle_nr_valid(num_cpus))
> + set_power_saving_task_num(num_cpus);
>
> put_online_cpus();
> }
> --
> 2.7.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH][RFC v2] ACPI: acpi_pad: Do not launch acpi_pad threads on idle cpus
On Monday, May 14, 2018 5:45:23 PM CEST Yu Chen wrote: > On Sun, May 13, 2018 at 11:30:52AM +0200, Rafael J. Wysocki wrote: > > On Saturday, May 5, 2018 1:53:22 PM CEST Chen Yu wrote: > > > According to current implementation of acpi_pad driver, > > > it does not make sense to spawn any power saving threads > > > on the cpus which are already idle - it might bring > > > unnecessary overhead on these idle cpus and causes power > > > waste. So verify the condition that if the number of 'busy' > > > cpus exceeds the amount of the 'forced idle' cpus is met. > > > This is applicable due to round-robin attribute of the > > > power saving threads, otherwise ignore the setting/ACPI > > > notification. > > > > OK, but CPUs are busy, because they are running tasks. If acpi_pad > > kthreads run on them, the tasks they are running will migrate to the > > currently idle CPUs (unless they have specific CPU affinity) and the > > throttling will not really be effective. > > > OK, I think this makes sense, I missed the load balance scenario. > > I would think that acpi_pad should ensure that the requested number of > > CPUs will not run anything other than throttling kthreads. Isn't that > > the case? > > > Do you mean, we should check if the number of 'idle'(rather than the 'busy' > one > in this patch) cpus is larger than the requested one? Then I think we should > also > add a patch to use the play_idle() as power_clamp to treat the throttling > kthreads > as idle threads thus to stop system tick. Such as the patch Jacob proposed: I wonder if that can be switched over to the new idle injection framework added recently? Thanks, Rafael
Re: [PATCH][RFC v2] ACPI: acpi_pad: Do not launch acpi_pad threads on idle cpus
On Mon, May 14, 2018 at 5:45 PM, Yu Chen wrote:
> On Sun, May 13, 2018 at 11:30:52AM +0200, Rafael J. Wysocki wrote:
>> On Saturday, May 5, 2018 1:53:22 PM CEST Chen Yu wrote:
>> > According to current implementation of acpi_pad driver,
>> > it does not make sense to spawn any power saving threads
>> > on the cpus which are already idle - it might bring
>> > unnecessary overhead on these idle cpus and causes power
>> > waste. So verify the condition that if the number of 'busy'
>> > cpus exceeds the amount of the 'forced idle' cpus is met.
>> > This is applicable due to round-robin attribute of the
>> > power saving threads, otherwise ignore the setting/ACPI
>> > notification.
>>
>> OK, but CPUs are busy, because they are running tasks. If acpi_pad
>> kthreads run on them, the tasks they are running will migrate to the
>> currently idle CPUs (unless they have specific CPU affinity) and the
>> throttling will not really be effective.
>>
> OK, I think this makes sense, I missed the load balance scenario.
>
>> I would think that acpi_pad should ensure that the requested number of
>> CPUs will not run anything other than throttling kthreads. Isn't that
>> the case?
>>
> Do you mean, we should check if the number of 'idle'(rather than the 'busy'
> one
> in this patch) cpus is larger than the requested one?
What I really meant was that acpi_pad kthreads should be started on as
many CPUs as requested by the firmware to prevent anything else from
running on them.
> Then I think we should also add a patch to use the play_idle() as power_clamp
> to treat
> the throttling kthreads as idle threads thus to stop system tick.
Yes, we should.
> Such as the patch Jacob proposed:
Looks OK from a quick glance, but I'll have a deeper look at it tomorrow.
> Index: linux/drivers/acpi/acpi_pad.c
> ===
> --- linux.orig/drivers/acpi/acpi_pad.c
> +++ linux/drivers/acpi/acpi_pad.c
> @@ -144,7 +144,6 @@ static unsigned int round_robin_time = 1
> static int power_saving_thread(void *data)
> {
> struct sched_param param = {.sched_priority = 1};
> - int do_sleep;
> unsigned int tsk_index = (unsigned long)data;
> u64 last_jiffies = 0;
>
> @@ -160,33 +159,13 @@ static int power_saving_thread(void *dat
> round_robin_cpu(tsk_index);
> }
>
> - do_sleep = 0;
> -
> - expire_time = jiffies + HZ * (100 - idle_pct) / 100;
> -
> - while (!need_resched()) {
> - if (tsc_detected_unstable && !tsc_marked_unstable) {
> - /* TSC could halt in idle, so notify users */
> - mark_tsc_unstable("TSC halts in idle");
> - tsc_marked_unstable = 1;
> - }
> - local_irq_disable();
> - tick_broadcast_enable();
> - tick_broadcast_enter();
> - stop_critical_timings();
> -
> - mwait_idle_with_hints(power_saving_mwait_eax, 1);
> -
> - start_critical_timings();
> - tick_broadcast_exit();
> - local_irq_enable();
> -
> - if (time_before(expire_time, jiffies)) {
> - do_sleep = 1;
> - break;
> - }
> + if (tsc_detected_unstable && !tsc_marked_unstable) {
> + /* TSC could halt in idle, so notify users */
> + mark_tsc_unstable("TSC halts in idle");
> + tsc_marked_unstable = 1;
> }
>
> + play_idle(jiffies_to_msecs(HZ * (100 - idle_pct) / 100));
> /*
> * current sched_rt has threshold for rt task running time.
> * When a rt task uses 95% CPU time, the rt thread will be
> @@ -196,8 +175,7 @@ static int power_saving_thread(void *dat
> * borrow CPU time from this CPU and cause RT task use > 95%
> * CPU time. To make 'avoid starvation' work, takes a nap
> here.
> */
> - if (unlikely(do_sleep))
> - schedule_timeout_killable(HZ * idle_pct / 100);
> + schedule_timeout_killable(HZ * idle_pct / 100);
>
> /* If an external event has set the need_resched flag, then
> * we need to deal with it, or this loop will continue to
>> Thanks,
>> Rafael
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH][RFC v2] ACPI: acpi_pad: Do not launch acpi_pad threads on idle cpus
On Sun, May 13, 2018 at 11:30:52AM +0200, Rafael J. Wysocki wrote:
> On Saturday, May 5, 2018 1:53:22 PM CEST Chen Yu wrote:
> > According to current implementation of acpi_pad driver,
> > it does not make sense to spawn any power saving threads
> > on the cpus which are already idle - it might bring
> > unnecessary overhead on these idle cpus and causes power
> > waste. So verify the condition that if the number of 'busy'
> > cpus exceeds the amount of the 'forced idle' cpus is met.
> > This is applicable due to round-robin attribute of the
> > power saving threads, otherwise ignore the setting/ACPI
> > notification.
>
> OK, but CPUs are busy, because they are running tasks. If acpi_pad
> kthreads run on them, the tasks they are running will migrate to the
> currently idle CPUs (unless they have specific CPU affinity) and the
> throttling will not really be effective.
>
OK, I think this makes sense, I missed the load balance scenario.
> I would think that acpi_pad should ensure that the requested number of
> CPUs will not run anything other than throttling kthreads. Isn't that
> the case?
>
Do you mean, we should check if the number of 'idle'(rather than the 'busy' one
in this patch) cpus is larger than the requested one? Then I think we should
also
add a patch to use the play_idle() as power_clamp to treat the throttling
kthreads
as idle threads thus to stop system tick. Such as the patch Jacob proposed:
Index: linux/drivers/acpi/acpi_pad.c
===
--- linux.orig/drivers/acpi/acpi_pad.c
+++ linux/drivers/acpi/acpi_pad.c
@@ -144,7 +144,6 @@ static unsigned int round_robin_time = 1
static int power_saving_thread(void *data)
{
struct sched_param param = {.sched_priority = 1};
- int do_sleep;
unsigned int tsk_index = (unsigned long)data;
u64 last_jiffies = 0;
@@ -160,33 +159,13 @@ static int power_saving_thread(void *dat
round_robin_cpu(tsk_index);
}
- do_sleep = 0;
-
- expire_time = jiffies + HZ * (100 - idle_pct) / 100;
-
- while (!need_resched()) {
- if (tsc_detected_unstable && !tsc_marked_unstable) {
- /* TSC could halt in idle, so notify users */
- mark_tsc_unstable("TSC halts in idle");
- tsc_marked_unstable = 1;
- }
- local_irq_disable();
- tick_broadcast_enable();
- tick_broadcast_enter();
- stop_critical_timings();
-
- mwait_idle_with_hints(power_saving_mwait_eax, 1);
-
- start_critical_timings();
- tick_broadcast_exit();
- local_irq_enable();
-
- if (time_before(expire_time, jiffies)) {
- do_sleep = 1;
- break;
- }
+ if (tsc_detected_unstable && !tsc_marked_unstable) {
+ /* TSC could halt in idle, so notify users */
+ mark_tsc_unstable("TSC halts in idle");
+ tsc_marked_unstable = 1;
}
+ play_idle(jiffies_to_msecs(HZ * (100 - idle_pct) / 100));
/*
* current sched_rt has threshold for rt task running time.
* When a rt task uses 95% CPU time, the rt thread will be
@@ -196,8 +175,7 @@ static int power_saving_thread(void *dat
* borrow CPU time from this CPU and cause RT task use > 95%
* CPU time. To make 'avoid starvation' work, takes a nap here.
*/
- if (unlikely(do_sleep))
- schedule_timeout_killable(HZ * idle_pct / 100);
+ schedule_timeout_killable(HZ * idle_pct / 100);
/* If an external event has set the need_resched flag, then
* we need to deal with it, or this loop will continue to
> Thanks,
> Rafael
>
Re: [PATCH][RFC v2] ACPI: acpi_pad: Do not launch acpi_pad threads on idle cpus
On Saturday, May 5, 2018 1:53:22 PM CEST Chen Yu wrote: > According to current implementation of acpi_pad driver, > it does not make sense to spawn any power saving threads > on the cpus which are already idle - it might bring > unnecessary overhead on these idle cpus and causes power > waste. So verify the condition that if the number of 'busy' > cpus exceeds the amount of the 'forced idle' cpus is met. > This is applicable due to round-robin attribute of the > power saving threads, otherwise ignore the setting/ACPI > notification. OK, but CPUs are busy, because they are running tasks. If acpi_pad kthreads run on them, the tasks they are running will migrate to the currently idle CPUs (unless they have specific CPU affinity) and the throttling will not really be effective. I would think that acpi_pad should ensure that the requested number of CPUs will not run anything other than throttling kthreads. Isn't that the case? Thanks, Rafael
[PATCH][RFC v2] ACPI: acpi_pad: Do not launch acpi_pad threads on idle cpus
According to current implementation of acpi_pad driver,
it does not make sense to spawn any power saving threads
on the cpus which are already idle - it might bring
unnecessary overhead on these idle cpus and causes power
waste. So verify the condition that if the number of 'busy'
cpus exceeds the amount of the 'forced idle' cpus is met.
This is applicable due to round-robin attribute of the
power saving threads, otherwise ignore the setting/ACPI
notification.
Suggested-by: Lenny Szubowicz
Suggested-by: Len Brown
Cc: "Rafael J. Wysocki"
Cc: Lenny Szubowicz
Cc: Len Brown
Cc: Jacob Pan
Cc: Rui Zhang
Cc: linux-a...@vger.kernel.org
Signed-off-by: Chen Yu
---
drivers/acpi/acpi_pad.c | 52 -
1 file changed, 51 insertions(+), 1 deletion(-)
diff --git a/drivers/acpi/acpi_pad.c b/drivers/acpi/acpi_pad.c
index 552c1f7..515e60e 100644
--- a/drivers/acpi/acpi_pad.c
+++ b/drivers/acpi/acpi_pad.c
@@ -254,12 +254,62 @@ static void set_power_saving_task_num(unsigned int num)
}
}
+/*
+ * Extra acpi_pad threads should not be created until
+ * the requested idle count is less than/equals to the
+ * number of the busy cpus - it does not make sense to
+ * throttle the idle cpus.
+ */
+#define SAMPLE_INTERVAL_JIF20
+
+static u64 get_idle_time(int cpu)
+{
+ u64 idle, idle_usecs = -1ULL;
+
+ idle_usecs = get_cpu_idle_time_us(cpu, NULL);
+
+ if (idle_usecs == -1ULL)
+ idle = kcpustat_cpu(cpu).cpustat[CPUTIME_IDLE];
+ else
+ idle = idle_usecs * NSEC_PER_USEC;
+
+ return idle;
+}
+
+static bool idle_nr_valid(unsigned int num_cpus)
+{
+ int busy_nr = 0, i = 0, load_thresh = 100 - idle_pct;
+
+ if (!num_cpus)
+ return true;
+
+ for_each_online_cpu(i) {
+ u64 wall_time, idle_time;
+ unsigned int elapsed_delta, idle_delta, load;
+
+ wall_time = jiffies64_to_nsecs(get_jiffies_64());
+ idle_time = get_idle_time(i);
+ /* Wait and see... */
+ schedule_timeout_uninterruptible(SAMPLE_INTERVAL_JIF);
+
+ idle_delta = get_idle_time(i) - idle_time;
+ elapsed_delta = jiffies64_to_nsecs(get_jiffies_64()) -
wall_time;
+ idle_delta = (idle_delta > elapsed_delta) ? elapsed_delta :
idle_delta;
+ load = 100 * (elapsed_delta - idle_delta) / elapsed_delta;
+ if (load >= load_thresh)
+ busy_nr++;
+ }
+
+ return (busy_nr >= num_cpus) ? true : false;
+}
+
static void acpi_pad_idle_cpus(unsigned int num_cpus)
{
get_online_cpus();
num_cpus = min_t(unsigned int, num_cpus, num_online_cpus());
- set_power_saving_task_num(num_cpus);
+ if (idle_nr_valid(num_cpus))
+ set_power_saving_task_num(num_cpus);
put_online_cpus();
}
--
2.7.4
