LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH][RFC v2] ACPI: acpi_pad: Do not launch acpi_pad threads on idle cpus
@ 2018-05-05 11:53 Chen Yu
  2018-05-13  9:30 ` Rafael J. Wysocki
  2018-12-10  6:31 ` joeyli
  0 siblings, 2 replies; 9+ messages in thread
From: Chen Yu @ 2018-05-05 11:53 UTC (permalink / raw)
  To: Rafael J. Wysocki, Len Brown
  Cc: linux-kernel, Chen Yu, Lenny Szubowicz, Jacob Pan, Rui Zhang, linux-acpi

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 <lszubowi@redhat.com>
Suggested-by: Len Brown <lenb@kernel.org>
Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
Cc: Lenny Szubowicz <lszubowi@redhat.com>
Cc: Len Brown <lenb@kernel.org>
Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
Cc: Rui Zhang <rui.zhang@intel.com>
Cc: linux-acpi@vger.kernel.org
Signed-off-by: Chen Yu <yu.c.chen@intel.com>
---
 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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH][RFC v2] ACPI: acpi_pad: Do not launch acpi_pad threads on idle cpus
  2018-05-05 11:53 [PATCH][RFC v2] ACPI: acpi_pad: Do not launch acpi_pad threads on idle cpus Chen Yu
@ 2018-05-13  9:30 ` Rafael J. Wysocki
  2018-05-14 15:45   ` Yu Chen
  2018-12-10  6:31 ` joeyli
  1 sibling, 1 reply; 9+ messages in thread
From: Rafael J. Wysocki @ 2018-05-13  9:30 UTC (permalink / raw)
  To: Chen Yu
  Cc: Len Brown, linux-kernel, Lenny Szubowicz, Jacob Pan, Rui Zhang,
	linux-acpi

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH][RFC v2] ACPI: acpi_pad: Do not launch acpi_pad threads on idle cpus
  2018-05-13  9:30 ` Rafael J. Wysocki
@ 2018-05-14 15:45   ` Yu Chen
  2018-05-14 20:42     ` Rafael J. Wysocki
  2018-10-03  9:38     ` Rafael J. Wysocki
  0 siblings, 2 replies; 9+ messages in thread
From: Yu Chen @ 2018-05-14 15:45 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Len Brown, linux-kernel, Lenny Szubowicz, Jacob Pan, Rui Zhang,
	linux-acpi

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
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH][RFC v2] ACPI: acpi_pad: Do not launch acpi_pad threads on idle cpus
  2018-05-14 15:45   ` Yu Chen
@ 2018-05-14 20:42     ` Rafael J. Wysocki
  2018-10-03  9:38     ` Rafael J. Wysocki
  1 sibling, 0 replies; 9+ messages in thread
From: Rafael J. Wysocki @ 2018-05-14 20:42 UTC (permalink / raw)
  To: Yu Chen
  Cc: Rafael J. Wysocki, Len Brown, Linux Kernel Mailing List,
	Lenny Szubowicz, Jacob Pan, Rui Zhang, ACPI Devel Maling List

On Mon, May 14, 2018 at 5:45 PM, Yu Chen <yu.c.chen@intel.com> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH][RFC v2] ACPI: acpi_pad: Do not launch acpi_pad threads on idle cpus
  2018-05-14 15:45   ` Yu Chen
  2018-05-14 20:42     ` Rafael J. Wysocki
@ 2018-10-03  9:38     ` Rafael J. Wysocki
  1 sibling, 0 replies; 9+ messages in thread
From: Rafael J. Wysocki @ 2018-10-03  9:38 UTC (permalink / raw)
  To: Yu Chen
  Cc: Len Brown, linux-kernel, Lenny Szubowicz, Jacob Pan, Rui Zhang,
	linux-acpi

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH][RFC v2] ACPI: acpi_pad: Do not launch acpi_pad threads on idle cpus
  2018-05-05 11:53 [PATCH][RFC v2] ACPI: acpi_pad: Do not launch acpi_pad threads on idle cpus Chen Yu
  2018-05-13  9:30 ` Rafael J. Wysocki
@ 2018-12-10  6:31 ` joeyli
  2018-12-11  3:12   ` Yu Chen
  1 sibling, 1 reply; 9+ messages in thread
From: joeyli @ 2018-12-10  6:31 UTC (permalink / raw)
  To: Chen Yu
  Cc: Rafael J. Wysocki, Len Brown, linux-kernel, Lenny Szubowicz,
	Jacob Pan, Rui Zhang, linux-acpi

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 <lszubowi@redhat.com>
> Suggested-by: Len Brown <lenb@kernel.org>
> Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> Cc: Lenny Szubowicz <lszubowi@redhat.com>
> Cc: Len Brown <lenb@kernel.org>
> Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
> Cc: Rui Zhang <rui.zhang@intel.com>
> Cc: linux-acpi@vger.kernel.org
> Signed-off-by: Chen Yu <yu.c.chen@intel.com>

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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH][RFC v2] ACPI: acpi_pad: Do not launch acpi_pad threads on idle cpus
  2018-12-10  6:31 ` joeyli
@ 2018-12-11  3:12   ` Yu Chen
  2018-12-11  8:37     ` joeyli
  0 siblings, 1 reply; 9+ messages in thread
From: Yu Chen @ 2018-12-11  3:12 UTC (permalink / raw)
  To: joeyli
  Cc: Rafael J. Wysocki, Len Brown, linux-kernel, Lenny Szubowicz,
	Jacob Pan, Rui Zhang, linux-acpi

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 <lszubowi@redhat.com>
> > Suggested-by: Len Brown <lenb@kernel.org>
> > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> > Cc: Lenny Szubowicz <lszubowi@redhat.com>
> > Cc: Len Brown <lenb@kernel.org>
> > Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
> > Cc: Rui Zhang <rui.zhang@intel.com>
> > Cc: linux-acpi@vger.kernel.org
> > Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> 
> 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_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 majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH][RFC v2] ACPI: acpi_pad: Do not launch acpi_pad threads on idle cpus
  2018-12-11  3:12   ` Yu Chen
@ 2018-12-11  8:37     ` joeyli
  2018-12-12  1:56       ` Yu Chen
  0 siblings, 1 reply; 9+ messages in thread
From: joeyli @ 2018-12-11  8:37 UTC (permalink / raw)
  To: Yu Chen
  Cc: Rafael J. Wysocki, Len Brown, linux-kernel, Lenny Szubowicz,
	Jacob Pan, Rui Zhang, linux-acpi

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 <lszubowi@redhat.com>
> > > Suggested-by: Len Brown <lenb@kernel.org>
> > > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> > > Cc: Lenny Szubowicz <lszubowi@redhat.com>
> > > Cc: Len Brown <lenb@kernel.org>
> > > Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
> > > Cc: Rui Zhang <rui.zhang@intel.com>
> > > Cc: linux-acpi@vger.kernel.org
> > > Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> > 
> > 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 majordomo@vger.kernel.org
> > > More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH][RFC v2] ACPI: acpi_pad: Do not launch acpi_pad threads on idle cpus
  2018-12-11  8:37     ` joeyli
@ 2018-12-12  1:56       ` Yu Chen
  0 siblings, 0 replies; 9+ messages in thread
From: Yu Chen @ 2018-12-12  1:56 UTC (permalink / raw)
  To: joeyli
  Cc: Rafael J. Wysocki, Len Brown, linux-kernel, Lenny Szubowicz,
	Jacob Pan, Rui Zhang, linux-acpi

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 <lszubowi@redhat.com>
> > > > Suggested-by: Len Brown <lenb@kernel.org>
> > > > Cc: "Rafael J. Wysocki" <rjw@rjwysocki.net>
> > > > Cc: Lenny Szubowicz <lszubowi@redhat.com>
> > > > Cc: Len Brown <lenb@kernel.org>
> > > > Cc: Jacob Pan <jacob.jun.pan@linux.intel.com>
> > > > Cc: Rui Zhang <rui.zhang@intel.com>
> > > > Cc: linux-acpi@vger.kernel.org
> > > > Signed-off-by: Chen Yu <yu.c.chen@intel.com>
> > > 
> > > 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 

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2018-12-12  1:49 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-05 11:53 [PATCH][RFC v2] ACPI: acpi_pad: Do not launch acpi_pad threads on idle cpus Chen Yu
2018-05-13  9:30 ` Rafael J. Wysocki
2018-05-14 15:45   ` Yu Chen
2018-05-14 20:42     ` Rafael J. Wysocki
2018-10-03  9:38     ` Rafael J. Wysocki
2018-12-10  6:31 ` joeyli
2018-12-11  3:12   ` Yu Chen
2018-12-11  8:37     ` joeyli
2018-12-12  1:56       ` Yu Chen

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).