linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RESEND PATCH v2 1/2] power/cpuidle: enhance the precision of state select
@ 2016-06-17  9:13 Zhaoyang Huang
  2016-06-17  9:13 ` [RESEND PATCH v2 2/2] power/idle: enhance the precision of sleep_length Zhaoyang Huang
  2016-06-25  1:09 ` [RESEND PATCH v2 1/2] power/cpuidle: enhance the precision of state select Rafael J. Wysocki
  0 siblings, 2 replies; 16+ messages in thread
From: Zhaoyang Huang @ 2016-06-17  9:13 UTC (permalink / raw)
  To: linux-kernel, linux-pm, mingo, peterz, zhaoyang.huang, tglx

In previous version, cpu_pm_enter is invoked after the governor
select the state, which cause the executing time of cpu_pm_enter
is included in the idle time. Moving it before the state selection.

Signed-off-by: Zhaoyang Huang <zhaoyang.huang@spreadtrum.com>
---
 kernel/sched/idle.c |   18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
index bd12c6c..929da2e 100644
--- a/kernel/sched/idle.c
+++ b/kernel/sched/idle.c
@@ -5,6 +5,7 @@
 #include <linux/cpu.h>
 #include <linux/cpuidle.h>
 #include <linux/cpuhotplug.h>
+#include <linux/cpu_pm.h>
 #include <linux/tick.h>
 #include <linux/mm.h>
 #include <linux/stackprotector.h>
@@ -130,6 +131,7 @@ static void cpuidle_idle_call(void)
 	struct cpuidle_device *dev = __this_cpu_read(cpuidle_devices);
 	struct cpuidle_driver *drv = cpuidle_get_cpu_driver(dev);
 	int next_state, entered_state;
+	int ret;
 
 	/*
 	 * Check if the idle task must be rescheduled. If it is the
@@ -174,12 +176,16 @@ static void cpuidle_idle_call(void)
 		/*
 		 * Ask the cpuidle framework to choose a convenient idle state.
 		 */
-		next_state = cpuidle_select(drv, dev);
-		entered_state = call_cpuidle(drv, dev, next_state);
-		/*
-		 * Give the governor an opportunity to reflect on the outcome
-		 */
-		cpuidle_reflect(dev, entered_state);
+		ret = cpu_pm_enter();
+		if (!ret) {
+			next_state = cpuidle_select(drv, dev);
+			entered_state = call_cpuidle(drv, dev, next_state);
+			cpu_pm_exit();
+			/*
+			 * Give the governor an opportunity to reflect on the outcome
+			 */
+			cpuidle_reflect(dev, entered_state);
+		}
 	}
 
 exit_idle:
-- 
1.7.9.5

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

* [RESEND PATCH v2 2/2] power/idle: enhance the precision of sleep_length
  2016-06-17  9:13 [RESEND PATCH v2 1/2] power/cpuidle: enhance the precision of state select Zhaoyang Huang
@ 2016-06-17  9:13 ` Zhaoyang Huang
  2016-06-17  9:27   ` Thomas Gleixner
  2016-06-25  1:09 ` [RESEND PATCH v2 1/2] power/cpuidle: enhance the precision of state select Rafael J. Wysocki
  1 sibling, 1 reply; 16+ messages in thread
From: Zhaoyang Huang @ 2016-06-17  9:13 UTC (permalink / raw)
  To: linux-kernel, linux-pm, mingo, peterz, zhaoyang.huang, tglx

There should be a gap between tick_nohz_idle_enter and
tick_nohz_get_sleep_length when idle, which will cause the
sleep_length is not very precised. Change it in this patch.

Signed-off-by: Zhaoyang Huang <zhaoyang.huang@spreadtrum.com>
---
 kernel/time/tick-sched.c |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
index 536ada8..ee3be3d 100644
--- a/kernel/time/tick-sched.c
+++ b/kernel/time/tick-sched.c
@@ -975,6 +975,11 @@ void tick_nohz_irq_exit(void)
 ktime_t tick_nohz_get_sleep_length(void)
 {
 	struct tick_sched *ts = this_cpu_ptr(&tick_cpu_sched);
+	struct clock_event_device *dev = __this_cpu_read(tick_cpu_device.evtdev);
+	ktime_t now;
+
+	now = ktime_get();
+	ts->sleep_length = ktime_sub(dev->next_event, now);
 
 	return ts->sleep_length;
 }
-- 
1.7.9.5

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

* Re: [RESEND PATCH v2 2/2] power/idle: enhance the precision of sleep_length
  2016-06-17  9:13 ` [RESEND PATCH v2 2/2] power/idle: enhance the precision of sleep_length Zhaoyang Huang
@ 2016-06-17  9:27   ` Thomas Gleixner
  2016-06-17 11:18     ` Zhaoyang Huang
  0 siblings, 1 reply; 16+ messages in thread
From: Thomas Gleixner @ 2016-06-17  9:27 UTC (permalink / raw)
  To: Zhaoyang Huang; +Cc: linux-kernel, linux-pm, mingo, peterz, zhaoyang.huang

On Fri, 17 Jun 2016, Zhaoyang Huang wrote:
> There should be a gap between tick_nohz_idle_enter and
> tick_nohz_get_sleep_length when idle, which will cause the
> sleep_length is not very precised. Change it in this patch.

What kind of imprecision are we talking about? Seconds, nanoseconds or
lightyears?

Your changelog lacks any form of useful information.

Thanks,

	tglx

 

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

* Re: [RESEND PATCH v2 2/2] power/idle: enhance the precision of sleep_length
  2016-06-17  9:27   ` Thomas Gleixner
@ 2016-06-17 11:18     ` Zhaoyang Huang
  2016-06-17 11:50       ` Thomas Gleixner
  0 siblings, 1 reply; 16+ messages in thread
From: Zhaoyang Huang @ 2016-06-17 11:18 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: linux-kernel, linux-pm, mingo, peterz,
	Zhaoyang Huang (黄朝阳)

On 17 June 2016 at 17:27, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Fri, 17 Jun 2016, Zhaoyang Huang wrote:
>> There should be a gap between tick_nohz_idle_enter and
>> tick_nohz_get_sleep_length when idle, which will cause the
>> sleep_length is not very precised. Change it in this patch.
>
> What kind of imprecision are we talking about? Seconds, nanoseconds or
> lightyears?
>
> Your changelog lacks any form of useful information.
>
> Thanks,
>
>         tglx
>
>
sorry for the confusion. The imprecision can be caused by, for
example, the callback function registered for CPU_PM_ENTER, which may
consume a period of time within the 'idle' time. Besides, I also
wonder why not calc the 'sleep_length' in the
tick_nohz_get_sleep_length?  This value is calculated at very
beginning of the idle in current approach.

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

* Re: [RESEND PATCH v2 2/2] power/idle: enhance the precision of sleep_length
  2016-06-17 11:18     ` Zhaoyang Huang
@ 2016-06-17 11:50       ` Thomas Gleixner
  2016-06-20  1:14         ` Zhaoyang Huang
  0 siblings, 1 reply; 16+ messages in thread
From: Thomas Gleixner @ 2016-06-17 11:50 UTC (permalink / raw)
  To: Zhaoyang Huang
  Cc: linux-kernel, linux-pm, mingo, peterz,
	Zhaoyang Huang (黄朝阳)

On Fri, 17 Jun 2016, Zhaoyang Huang wrote:
> On 17 June 2016 at 17:27, Thomas Gleixner <tglx@linutronix.de> wrote:
> > On Fri, 17 Jun 2016, Zhaoyang Huang wrote:
> >> There should be a gap between tick_nohz_idle_enter and
> >> tick_nohz_get_sleep_length when idle, which will cause the
> >> sleep_length is not very precised. Change it in this patch.
> >
> > What kind of imprecision are we talking about? Seconds, nanoseconds or
> > lightyears?
> >
> > Your changelog lacks any form of useful information.
> >
> sorry for the confusion. The imprecision can be caused by, for
> example, the callback function registered for CPU_PM_ENTER, which may
> consume a period of time within the 'idle' time. Besides, I also
> wonder why not calc the 'sleep_length' in the
> tick_nohz_get_sleep_length?  This value is calculated at very
> beginning of the idle in current approach.

You still are not explaining the amount of imprecision. What are you talking
about and is it really relevant in any way or are you just trying to solve an
acedemic issue?

Thanks,

	tglx

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

* Re: [RESEND PATCH v2 2/2] power/idle: enhance the precision of sleep_length
  2016-06-17 11:50       ` Thomas Gleixner
@ 2016-06-20  1:14         ` Zhaoyang Huang
  2016-06-22  3:01           ` Zhaoyang Huang
  0 siblings, 1 reply; 16+ messages in thread
From: Zhaoyang Huang @ 2016-06-20  1:14 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: linux-kernel, linux-pm, mingo, peterz,
	Zhaoyang Huang (黄朝阳)

On 17 June 2016 at 19:50, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Fri, 17 Jun 2016, Zhaoyang Huang wrote:
>> On 17 June 2016 at 17:27, Thomas Gleixner <tglx@linutronix.de> wrote:
>> > On Fri, 17 Jun 2016, Zhaoyang Huang wrote:
>> >> There should be a gap between tick_nohz_idle_enter and
>> >> tick_nohz_get_sleep_length when idle, which will cause the
>> >> sleep_length is not very precised. Change it in this patch.
>> >
>> > What kind of imprecision are we talking about? Seconds, nanoseconds or
>> > lightyears?
>> >
>> > Your changelog lacks any form of useful information.
>> >
>> sorry for the confusion. The imprecision can be caused by, for
>> example, the callback function registered for CPU_PM_ENTER, which may
>> consume a period of time within the 'idle' time. Besides, I also
>> wonder why not calc the 'sleep_length' in the
>> tick_nohz_get_sleep_length?  This value is calculated at very
>> beginning of the idle in current approach.
>
> You still are not explaining the amount of imprecision. What are you talking
> about and is it really relevant in any way or are you just trying to solve an
> acedemic issue?
>
> Thanks,
>
>         tglx
>
Indeed, it is depends on how deep the idle state is. For example, the
lightest level for my current platform is 1100us, which sums up the
entry,exit and min time. However, there is a callback which do memory
management(merge the same page) in CPU_PM_ENTER will consume at least
500us. In current approach, it cause 50% imprecision for this level of
idle state.

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

* Re: [RESEND PATCH v2 2/2] power/idle: enhance the precision of sleep_length
  2016-06-20  1:14         ` Zhaoyang Huang
@ 2016-06-22  3:01           ` Zhaoyang Huang
  2016-06-23  7:01             ` Thomas Gleixner
  0 siblings, 1 reply; 16+ messages in thread
From: Zhaoyang Huang @ 2016-06-22  3:01 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: linux-kernel, linux-pm, mingo, peterz,
	Zhaoyang Huang (黄朝阳)

On 20 June 2016 at 09:14, Zhaoyang Huang <zhaoyang.huang@linaro.org> wrote:
> On 17 June 2016 at 19:50, Thomas Gleixner <tglx@linutronix.de> wrote:
>> On Fri, 17 Jun 2016, Zhaoyang Huang wrote:
>>> On 17 June 2016 at 17:27, Thomas Gleixner <tglx@linutronix.de> wrote:
>>> > On Fri, 17 Jun 2016, Zhaoyang Huang wrote:
>>> >> There should be a gap between tick_nohz_idle_enter and
>>> >> tick_nohz_get_sleep_length when idle, which will cause the
>>> >> sleep_length is not very precised. Change it in this patch.
>>> >
>>> > What kind of imprecision are we talking about? Seconds, nanoseconds or
>>> > lightyears?
>>> >
>>> > Your changelog lacks any form of useful information.
>>> >
>>> sorry for the confusion. The imprecision can be caused by, for
>>> example, the callback function registered for CPU_PM_ENTER, which may
>>> consume a period of time within the 'idle' time. Besides, I also
>>> wonder why not calc the 'sleep_length' in the
>>> tick_nohz_get_sleep_length?  This value is calculated at very
>>> beginning of the idle in current approach.
>>
>> You still are not explaining the amount of imprecision. What are you talking
>> about and is it really relevant in any way or are you just trying to solve an
>> acedemic issue?
>>
>> Thanks,
>>
>>         tglx
>>
> Indeed, it is depends on how deep the idle state is. For example, the
> lightest level for my current platform is 1100us, which sums up the
> entry,exit and min time. However, there is a callback which do memory
> management(merge the same page) in CPU_PM_ENTER will consume at least
> 500us. In current approach, it cause 50% imprecision for this level of
> idle state.
Hi Thomas,
Any further comments on that?

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

* Re: [RESEND PATCH v2 2/2] power/idle: enhance the precision of sleep_length
  2016-06-22  3:01           ` Zhaoyang Huang
@ 2016-06-23  7:01             ` Thomas Gleixner
  2016-06-23  7:26               ` Zhaoyang Huang
  0 siblings, 1 reply; 16+ messages in thread
From: Thomas Gleixner @ 2016-06-23  7:01 UTC (permalink / raw)
  To: Zhaoyang Huang
  Cc: linux-kernel, linux-pm, mingo, peterz,
	Zhaoyang Huang (黄朝阳)

On Wed, 22 Jun 2016, Zhaoyang Huang wrote:
> On 20 June 2016 at 09:14, Zhaoyang Huang <zhaoyang.huang@linaro.org> wrote:
> > On 17 June 2016 at 19:50, Thomas Gleixner <tglx@linutronix.de> wrote:
> >> On Fri, 17 Jun 2016, Zhaoyang Huang wrote:
> >>> On 17 June 2016 at 17:27, Thomas Gleixner <tglx@linutronix.de> wrote:
> >>> > On Fri, 17 Jun 2016, Zhaoyang Huang wrote:
> >>> >> There should be a gap between tick_nohz_idle_enter and
> >>> >> tick_nohz_get_sleep_length when idle, which will cause the
> >>> >> sleep_length is not very precised. Change it in this patch.
> >>> >
> >>> > What kind of imprecision are we talking about? Seconds, nanoseconds or
> >>> > lightyears?
> >>> >
> >>> > Your changelog lacks any form of useful information.
> >>> >
> >>> sorry for the confusion. The imprecision can be caused by, for
> >>> example, the callback function registered for CPU_PM_ENTER, which may
> >>> consume a period of time within the 'idle' time. Besides, I also
> >>> wonder why not calc the 'sleep_length' in the
> >>> tick_nohz_get_sleep_length?  This value is calculated at very
> >>> beginning of the idle in current approach.
> >>
> >> You still are not explaining the amount of imprecision. What are you talking
> >> about and is it really relevant in any way or are you just trying to solve an
> >> acedemic issue?
> >>
> >> Thanks,
> >>
> >>         tglx
> >>
> > Indeed, it is depends on how deep the idle state is. For example, the
> > lightest level for my current platform is 1100us, which sums up the
> > entry,exit and min time. However, there is a callback which do memory
> > management(merge the same page) in CPU_PM_ENTER will consume at least
> > 500us. In current approach, it cause 50% imprecision for this level of
> > idle state.
> Hi Thomas,
> Any further comments on that?

Yes. Why on earth do we have a 500us callback in the idle entry path? That's
just insane and needs to be fixed.

Thanks,

	tglx

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

* Re: [RESEND PATCH v2 2/2] power/idle: enhance the precision of sleep_length
  2016-06-23  7:01             ` Thomas Gleixner
@ 2016-06-23  7:26               ` Zhaoyang Huang
  2016-06-23  8:18                 ` Thomas Gleixner
  0 siblings, 1 reply; 16+ messages in thread
From: Zhaoyang Huang @ 2016-06-23  7:26 UTC (permalink / raw)
  To: Thomas Gleixner, Daniel Lezcano, Geng Ren, Alex Wang, Vincent Guittot
  Cc: linux-kernel, linux-pm, mingo, peterz,
	Zhaoyang Huang (黄朝阳)

On 23 June 2016 at 15:01, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Wed, 22 Jun 2016, Zhaoyang Huang wrote:
>> On 20 June 2016 at 09:14, Zhaoyang Huang <zhaoyang.huang@linaro.org> wrote:
>> > On 17 June 2016 at 19:50, Thomas Gleixner <tglx@linutronix.de> wrote:
>> >> On Fri, 17 Jun 2016, Zhaoyang Huang wrote:
>> >>> On 17 June 2016 at 17:27, Thomas Gleixner <tglx@linutronix.de> wrote:
>> >>> > On Fri, 17 Jun 2016, Zhaoyang Huang wrote:
>> >>> >> There should be a gap between tick_nohz_idle_enter and
>> >>> >> tick_nohz_get_sleep_length when idle, which will cause the
>> >>> >> sleep_length is not very precised. Change it in this patch.
>> >>> >
>> >>> > What kind of imprecision are we talking about? Seconds, nanoseconds or
>> >>> > lightyears?
>> >>> >
>> >>> > Your changelog lacks any form of useful information.
>> >>> >
>> >>> sorry for the confusion. The imprecision can be caused by, for
>> >>> example, the callback function registered for CPU_PM_ENTER, which may
>> >>> consume a period of time within the 'idle' time. Besides, I also
>> >>> wonder why not calc the 'sleep_length' in the
>> >>> tick_nohz_get_sleep_length?  This value is calculated at very
>> >>> beginning of the idle in current approach.
>> >>
>> >> You still are not explaining the amount of imprecision. What are you talking
>> >> about and is it really relevant in any way or are you just trying to solve an
>> >> acedemic issue?
>> >>
>> >> Thanks,
>> >>
>> >>         tglx
>> >>
>> > Indeed, it is depends on how deep the idle state is. For example, the
>> > lightest level for my current platform is 1100us, which sums up the
>> > entry,exit and min time. However, there is a callback which do memory
>> > management(merge the same page) in CPU_PM_ENTER will consume at least
>> > 500us. In current approach, it cause 50% imprecision for this level of
>> > idle state.
>> Hi Thomas,
>> Any further comments on that?
>
> Yes. Why on earth do we have a 500us callback in the idle entry path? That's
> just insane and needs to be fixed.
>
> Thanks,
>
>         tglx
Thomas, I agree with you, I have discussed the modification with the
call back owner. However, I wonder if we can make the idle's framework
to be more precised without the assumption of short CPU_PM_ENTER
callbacks. Thank you!

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

* Re: [RESEND PATCH v2 2/2] power/idle: enhance the precision of sleep_length
  2016-06-23  7:26               ` Zhaoyang Huang
@ 2016-06-23  8:18                 ` Thomas Gleixner
  2016-06-23  8:34                   ` Zhaoyang Huang
  0 siblings, 1 reply; 16+ messages in thread
From: Thomas Gleixner @ 2016-06-23  8:18 UTC (permalink / raw)
  To: Zhaoyang Huang
  Cc: Daniel Lezcano, Geng Ren, Alex Wang, Vincent Guittot,
	linux-kernel, linux-pm, mingo, peterz,
	Zhaoyang Huang (黄朝阳)

On Thu, 23 Jun 2016, Zhaoyang Huang wrote:
> On 23 June 2016 at 15:01, Thomas Gleixner <tglx@linutronix.de> wrote:
> Thomas, I agree with you, I have discussed the modification with the
> call back owner. However, I wonder if we can make the idle's framework
> to be more precised without the assumption of short CPU_PM_ENTER
> callbacks. Thank you!

What's the point? To help people who put insanities into the idle code path?

Thanks,

	tglx

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

* Re: [RESEND PATCH v2 2/2] power/idle: enhance the precision of sleep_length
  2016-06-23  8:18                 ` Thomas Gleixner
@ 2016-06-23  8:34                   ` Zhaoyang Huang
  2016-06-23  8:35                     ` Thomas Gleixner
  0 siblings, 1 reply; 16+ messages in thread
From: Zhaoyang Huang @ 2016-06-23  8:34 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Daniel Lezcano, Geng Ren, Alex Wang, Vincent Guittot,
	linux-kernel, linux-pm, mingo, peterz,
	Zhaoyang Huang (黄朝阳)

On 23 June 2016 at 16:18, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Thu, 23 Jun 2016, Zhaoyang Huang wrote:
>> On 23 June 2016 at 15:01, Thomas Gleixner <tglx@linutronix.de> wrote:
>> Thomas, I agree with you, I have discussed the modification with the
>> call back owner. However, I wonder if we can make the idle's framework
>> to be more precised without the assumption of short CPU_PM_ENTER
>> callbacks. Thank you!
>
> What's the point? To help people who put insanities into the idle code path?
>
> Thanks,
>
>         tglx
>
Hi, Thomas. If the entry,exit,min time of one idle state sums up to
500us in some platform, the 100us callback which should be common as
caused by cache miss would also generate 20% imprecision. Don't you
think it is a case we should deal with?

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

* Re: [RESEND PATCH v2 2/2] power/idle: enhance the precision of sleep_length
  2016-06-23  8:34                   ` Zhaoyang Huang
@ 2016-06-23  8:35                     ` Thomas Gleixner
  2016-06-23  8:45                       ` Zhaoyang Huang
  0 siblings, 1 reply; 16+ messages in thread
From: Thomas Gleixner @ 2016-06-23  8:35 UTC (permalink / raw)
  To: Zhaoyang Huang
  Cc: Daniel Lezcano, Geng Ren, Alex Wang, Vincent Guittot,
	linux-kernel, linux-pm, mingo, peterz,
	Zhaoyang Huang (黄朝阳)

On Thu, 23 Jun 2016, Zhaoyang Huang wrote:
> On 23 June 2016 at 16:18, Thomas Gleixner <tglx@linutronix.de> wrote:
> > On Thu, 23 Jun 2016, Zhaoyang Huang wrote:
> >> On 23 June 2016 at 15:01, Thomas Gleixner <tglx@linutronix.de> wrote:
> >> Thomas, I agree with you, I have discussed the modification with the
> >> call back owner. However, I wonder if we can make the idle's framework
> >> to be more precised without the assumption of short CPU_PM_ENTER
> >> callbacks. Thank you!
> >
> > What's the point? To help people who put insanities into the idle code path?
> >
> > Thanks,
> >
> >         tglx
> >
> Hi, Thomas. If the entry,exit,min time of one idle state sums up to
> 500us in some platform, the 100us callback which should be common as
> caused by cache miss would also generate 20% imprecision. Don't you

A cache miss is causing a 100us callback? What are you talking about?

Thanks,

	tglx

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

* Re: [RESEND PATCH v2 2/2] power/idle: enhance the precision of sleep_length
  2016-06-23  8:35                     ` Thomas Gleixner
@ 2016-06-23  8:45                       ` Zhaoyang Huang
  2016-06-23 10:16                         ` Thomas Gleixner
  0 siblings, 1 reply; 16+ messages in thread
From: Zhaoyang Huang @ 2016-06-23  8:45 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Daniel Lezcano, Geng Ren, Alex Wang, Vincent Guittot,
	linux-kernel, linux-pm, mingo, peterz,
	Zhaoyang Huang (黄朝阳),
	Serge Broslavsky

On 23 June 2016 at 16:35, Thomas Gleixner <tglx@linutronix.de> wrote:
> On Thu, 23 Jun 2016, Zhaoyang Huang wrote:
>> On 23 June 2016 at 16:18, Thomas Gleixner <tglx@linutronix.de> wrote:
>> > On Thu, 23 Jun 2016, Zhaoyang Huang wrote:
>> >> On 23 June 2016 at 15:01, Thomas Gleixner <tglx@linutronix.de> wrote:
>> >> Thomas, I agree with you, I have discussed the modification with the
>> >> call back owner. However, I wonder if we can make the idle's framework
>> >> to be more precised without the assumption of short CPU_PM_ENTER
>> >> callbacks. Thank you!
>> >
>> > What's the point? To help people who put insanities into the idle code path?
>> >
>> > Thanks,
>> >
>> >         tglx
>> >
>> Hi, Thomas. If the entry,exit,min time of one idle state sums up to
>> 500us in some platform, the 100us callback which should be common as
>> caused by cache miss would also generate 20% imprecision. Don't you
>
> A cache miss is causing a 100us callback? What are you talking about?
>
> Thanks,
>
>         tglx
By a serials of memory access which maybe missed on cache all. Anyway,
we can require the callback not to introduce schedule etc, but can not
ask them to ensure their own executing time, which they can not
control.

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

* Re: [RESEND PATCH v2 2/2] power/idle: enhance the precision of sleep_length
  2016-06-23  8:45                       ` Zhaoyang Huang
@ 2016-06-23 10:16                         ` Thomas Gleixner
  0 siblings, 0 replies; 16+ messages in thread
From: Thomas Gleixner @ 2016-06-23 10:16 UTC (permalink / raw)
  To: Zhaoyang Huang
  Cc: Daniel Lezcano, Geng Ren, Alex Wang, Vincent Guittot,
	linux-kernel, linux-pm, mingo, peterz,
	Zhaoyang Huang (黄朝阳),
	Serge Broslavsky

On Thu, 23 Jun 2016, Zhaoyang Huang wrote:
> On 23 June 2016 at 16:35, Thomas Gleixner <tglx@linutronix.de> wrote:
> >> Hi, Thomas. If the entry,exit,min time of one idle state sums up to
> >> 500us in some platform, the 100us callback which should be common as
> >> caused by cache miss would also generate 20% imprecision. Don't you
> >
> > A cache miss is causing a 100us callback? What are you talking about?
> >
> By a serials of memory access which maybe missed on cache all. Anyway,
> we can require the callback not to introduce schedule etc, but can not
> ask them to ensure their own executing time, which they can not
> control.

I really do not understand what you are trying to solve. If the enter to idle
code is delayed by cache misses for 100us then you have other serious problems
than the accuracy of that time stamp.

Thanks,

	tglx

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

* Re: [RESEND PATCH v2 1/2] power/cpuidle: enhance the precision of state select
  2016-06-17  9:13 [RESEND PATCH v2 1/2] power/cpuidle: enhance the precision of state select Zhaoyang Huang
  2016-06-17  9:13 ` [RESEND PATCH v2 2/2] power/idle: enhance the precision of sleep_length Zhaoyang Huang
@ 2016-06-25  1:09 ` Rafael J. Wysocki
  2016-06-27  1:13   ` Zhaoyang Huang
  1 sibling, 1 reply; 16+ messages in thread
From: Rafael J. Wysocki @ 2016-06-25  1:09 UTC (permalink / raw)
  To: Zhaoyang Huang
  Cc: Linux Kernel Mailing List, linux-pm, Ingo Molnar, Peter Zijlstra,
	zhaoyang.huang, Thomas Gleixner

On Fri, Jun 17, 2016 at 11:13 AM, Zhaoyang Huang
<zhaoyang.huang@linaro.org> wrote:
> In previous version, cpu_pm_enter is invoked

By whom?  Not by the core surely?

> after the governor select the state, which cause the executing time of cpu_pm_enter
> is included in the idle time. Moving it before the state selection.
>
> Signed-off-by: Zhaoyang Huang <zhaoyang.huang@spreadtrum.com>
> ---
>  kernel/sched/idle.c |   18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
> index bd12c6c..929da2e 100644
> --- a/kernel/sched/idle.c
> +++ b/kernel/sched/idle.c
> @@ -5,6 +5,7 @@
>  #include <linux/cpu.h>
>  #include <linux/cpuidle.h>
>  #include <linux/cpuhotplug.h>
> +#include <linux/cpu_pm.h>
>  #include <linux/tick.h>
>  #include <linux/mm.h>
>  #include <linux/stackprotector.h>
> @@ -130,6 +131,7 @@ static void cpuidle_idle_call(void)
>         struct cpuidle_device *dev = __this_cpu_read(cpuidle_devices);
>         struct cpuidle_driver *drv = cpuidle_get_cpu_driver(dev);
>         int next_state, entered_state;
> +       int ret;
>
>         /*
>          * Check if the idle task must be rescheduled. If it is the
> @@ -174,12 +176,16 @@ static void cpuidle_idle_call(void)
>                 /*
>                  * Ask the cpuidle framework to choose a convenient idle state.
>                  */
> -               next_state = cpuidle_select(drv, dev);
> -               entered_state = call_cpuidle(drv, dev, next_state);
> -               /*
> -                * Give the governor an opportunity to reflect on the outcome
> -                */
> -               cpuidle_reflect(dev, entered_state);
> +               ret = cpu_pm_enter();

"To move" usually means "take it away from there and put it here" as
far as kernel patches are concerned, but I only see it added here.

> +               if (!ret) {
> +                       next_state = cpuidle_select(drv, dev);
> +                       entered_state = call_cpuidle(drv, dev, next_state);
> +                       cpu_pm_exit();
> +                       /*
> +                        * Give the governor an opportunity to reflect on the outcome
> +                        */
> +                       cpuidle_reflect(dev, entered_state);
> +               }
>         }
>
>  exit_idle:
> --

No way I will agree to add that notification stuff to the core.

Thanks,
Rafael

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

* Re: [RESEND PATCH v2 1/2] power/cpuidle: enhance the precision of state select
  2016-06-25  1:09 ` [RESEND PATCH v2 1/2] power/cpuidle: enhance the precision of state select Rafael J. Wysocki
@ 2016-06-27  1:13   ` Zhaoyang Huang
  0 siblings, 0 replies; 16+ messages in thread
From: Zhaoyang Huang @ 2016-06-27  1:13 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Linux Kernel Mailing List, linux-pm, Ingo Molnar, Peter Zijlstra,
	Zhaoyang Huang (黄朝阳),
	Thomas Gleixner, Geng Ren, Alex Wang

On 25 June 2016 at 09:09, Rafael J. Wysocki <rafael@kernel.org> wrote:
> On Fri, Jun 17, 2016 at 11:13 AM, Zhaoyang Huang
> <zhaoyang.huang@linaro.org> wrote:
>> In previous version, cpu_pm_enter is invoked
>
> By whom?  Not by the core surely?
>
>> after the governor select the state, which cause the executing time of cpu_pm_enter
>> is included in the idle time. Moving it before the state selection.
>>
>> Signed-off-by: Zhaoyang Huang <zhaoyang.huang@spreadtrum.com>
>> ---
>>  kernel/sched/idle.c |   18 ++++++++++++------
>>  1 file changed, 12 insertions(+), 6 deletions(-)
>>
>> diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c
>> index bd12c6c..929da2e 100644
>> --- a/kernel/sched/idle.c
>> +++ b/kernel/sched/idle.c
>> @@ -5,6 +5,7 @@
>>  #include <linux/cpu.h>
>>  #include <linux/cpuidle.h>
>>  #include <linux/cpuhotplug.h>
>> +#include <linux/cpu_pm.h>
>>  #include <linux/tick.h>
>>  #include <linux/mm.h>
>>  #include <linux/stackprotector.h>
>> @@ -130,6 +131,7 @@ static void cpuidle_idle_call(void)
>>         struct cpuidle_device *dev = __this_cpu_read(cpuidle_devices);
>>         struct cpuidle_driver *drv = cpuidle_get_cpu_driver(dev);
>>         int next_state, entered_state;
>> +       int ret;
>>
>>         /*
>>          * Check if the idle task must be rescheduled. If it is the
>> @@ -174,12 +176,16 @@ static void cpuidle_idle_call(void)
>>                 /*
>>                  * Ask the cpuidle framework to choose a convenient idle state.
>>                  */
>> -               next_state = cpuidle_select(drv, dev);
>> -               entered_state = call_cpuidle(drv, dev, next_state);
>> -               /*
>> -                * Give the governor an opportunity to reflect on the outcome
>> -                */
>> -               cpuidle_reflect(dev, entered_state);
>> +               ret = cpu_pm_enter();
>
> "To move" usually means "take it away from there and put it here" as
> far as kernel patches are concerned, but I only see it added here.
>
The API is provided by kernel, while invoked by cpuidle driver now. We
have ever move 'CPUFREQ_PRECHANGE' from cpufreq driver to framework. I
do the same thing here.

>> +               if (!ret) {
>> +                       next_state = cpuidle_select(drv, dev);
>> +                       entered_state = call_cpuidle(drv, dev, next_state);
>> +                       cpu_pm_exit();
>> +                       /*
>> +                        * Give the governor an opportunity to reflect on the outcome
>> +                        */
>> +                       cpuidle_reflect(dev, entered_state);
>> +               }
>>         }
>>
>>  exit_idle:
>> --
>
> No way I will agree to add that notification stuff to the core.
>
It is not add, just move.
> Thanks,
> Rafael

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

end of thread, other threads:[~2016-06-27  1:13 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-17  9:13 [RESEND PATCH v2 1/2] power/cpuidle: enhance the precision of state select Zhaoyang Huang
2016-06-17  9:13 ` [RESEND PATCH v2 2/2] power/idle: enhance the precision of sleep_length Zhaoyang Huang
2016-06-17  9:27   ` Thomas Gleixner
2016-06-17 11:18     ` Zhaoyang Huang
2016-06-17 11:50       ` Thomas Gleixner
2016-06-20  1:14         ` Zhaoyang Huang
2016-06-22  3:01           ` Zhaoyang Huang
2016-06-23  7:01             ` Thomas Gleixner
2016-06-23  7:26               ` Zhaoyang Huang
2016-06-23  8:18                 ` Thomas Gleixner
2016-06-23  8:34                   ` Zhaoyang Huang
2016-06-23  8:35                     ` Thomas Gleixner
2016-06-23  8:45                       ` Zhaoyang Huang
2016-06-23 10:16                         ` Thomas Gleixner
2016-06-25  1:09 ` [RESEND PATCH v2 1/2] power/cpuidle: enhance the precision of state select Rafael J. Wysocki
2016-06-27  1:13   ` Zhaoyang Huang

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