linuxsh-dev

delayed switching

[Hideo S. <sa...@de...>]

Hi Paul,

When I measured a performance with task switching, unexpected delay is issued sometimes. I think that the cause is in idle task so that any interrupt should be blocked in order to halt exclusively.

--- arch/sh/kernel/process.c.org	Thu May 10 10:34:17 2007
+++ arch/sh/kernel/process.c	Thu May 10 10:36:07 2007
@@ -52,18 +52,24 @@ void cpu_idle(void)
 {
 	/* endless idle loop with no priority at all */
 	while (1) {
 		void (*idle)(void) = pm_idle;
 
 		if (!idle)
 			idle = default_idle;
 
-		while (!need_resched())
+		set_bl_bit();
+		for (;;) {
+			if (need_resched()) {
+				clear_bl_bit();
+				break;
+			}
 			idle();
+		}
 
 		preempt_enable_no_resched();
 		schedule();
 		preempt_disable();
 	}
 }
 
 void machine_restart(char * __unused)

Re: delayed switching

[Paul M. <le...@li...>]

Hi Saito-san,

On Thu, May 10, 2007 at 10:56:05AM +0900, Hideo Saito wrote:
> When I measured a performance with task switching, unexpected delay is
> issued sometimes. I think that the cause is in idle task so that any
> interrupt should be blocked in order to halt exclusively.
> 
How about this?

--

 arch/sh/kernel/process.c |   33 +++++++++++++++++++++++++++------
 1 file changed, 27 insertions(+), 6 deletions(-)

diff --git a/arch/sh/kernel/process.c b/arch/sh/kernel/process.c
index 6b4f574..a11e2aa 100644
--- a/arch/sh/kernel/process.c
+++ b/arch/sh/kernel/process.c
@@ -26,8 +26,6 @@
 static int hlt_counter;
 int ubc_usercnt = 0;
 
-#define HARD_IDLE_TIMEOUT (HZ / 3)
-
 void (*pm_idle)(void);
 void (*pm_power_off)(void);
 EXPORT_SYMBOL(pm_power_off);
@@ -44,16 +42,39 @@ void enable_hlt(void)
 }
 EXPORT_SYMBOL(enable_hlt);
 
+static int __init nohlt_setup(char *__unused)
+{
+	hlt_counter = 1;
+	return 1;
+}
+__setup("nohlt", nohlt_setup);
+
+static int __init hlt_setup(char *__unused)
+{
+	hlt_counter = 0;
+	return 1;
+}
+__setup("hlt", hlt_setup);
+
 void default_idle(void)
 {
-	if (!hlt_counter)
-		cpu_sleep();
-	else
-		cpu_relax();
+	if (!hlt_counter) {
+		clear_thread_flag(TIF_POLLING_NRFLAG);
+		smp_mb__after_clear_bit();
+		set_bl_bit();
+		while (!need_resched())
+			cpu_sleep();
+		clear_bl_bit();
+		set_thread_flag(TIF_POLLING_NRFLAG);
+	} else
+		while (!need_resched())
+			cpu_relax();
 }
 
 void cpu_idle(void)
 {
+	set_thread_flag(TIF_POLLING_NRFLAG);
+
 	/* endless idle loop with no priority at all */
 	while (1) {
 		void (*idle)(void) = pm_idle;

Re: delayed switching

[Paul M. <le...@li...>]

On Thu, May 10, 2007 at 02:02:15PM +0900, Hideo Saito wrote:
> I hope to place the code looking at whether needs to reschedule in the
> same function as follows.
> 
> --- arch/sh/kernel/process.c.org	Thu Apr 26 12:08:32 2007
> +++ arch/sh/kernel/process.c	Thu May 10 13:05:44 2007
> +		if (pm_idle) {
> +			while (!need_resched())
> +				pm_idle();
> +		} else if (hlt_counter) {
> +			while (!need_resched())
> +				cpu_relax();
> +		} else {
> +			clear_thread_flag(TIF_POLLING_NRFLAG);
> +			smp_mb__after_clear_bit();
> +			set_bl_bit();
> +			while (!need_resched())
> +				cpu_sleep();
> +			clear_bl_bit();
> +			set_thread_flag(TIF_POLLING_NRFLAG);
> +		}
> +			

No, consider the use case where a platform implements pm_idle() and
depends on certain conditions to be met to determine what sort of
power-save state can be entered. It's still necessary to have the logic
in default_idle() and to have that exported so it can be called back in
to from pm_idle(), should the latter not have any meaningful work it can
do at the time.

In any event, if the previous patches fixes the problems you've been
seeing, I'll queue it up. It works fine with dynamic ticks on SH7785 at
least (both with and without hlt)..

Re: delayed switching

[Hideo S. <sa...@de...>]

Hi Paul,

On Thu, 10 May 2007 14:15:26 +0900, Paul Mundt wrote:
> No, consider the use case where a platform implements pm_idle() and
> depends on certain conditions to be met to determine what sort of
> power-save state can be entered. It's still necessary to have the logic
> in default_idle() and to have that exported so it can be called back in
> to from pm_idle(), should the latter not have any meaningful work it can
> do at the time.

I see, but I think following change in default_idle() does not need.

+       } else
+               while (!need_resched())
+                       cpu_relax();

how about following change?

+       } else
                cpu_relax();

> In any event, if the previous patches fixes the problems you've been
> seeing, I'll queue it up. It works fine with dynamic ticks on SH7785 at
> least (both with and without hlt)..

Yes, my testing is also done successfully.

Re: delayed switching

[Paul M. <le...@li...>]

Hi Saito-san,

On Fri, May 11, 2007 at 11:07:26AM +0900, Hideo Saito wrote:
> On Thu, 10 May 2007 14:15:26 +0900, Paul Mundt wrote:
> > No, consider the use case where a platform implements pm_idle() and
> > depends on certain conditions to be met to determine what sort of
> > power-save state can be entered. It's still necessary to have the logic
> > in default_idle() and to have that exported so it can be called back in
> > to from pm_idle(), should the latter not have any meaningful work it can
> > do at the time.
> 
> I see, but I think following change in default_idle() does not need.
> 
> +       } else
> +               while (!need_resched())
> +                       cpu_relax();
> 
> how about following change?
> 
> +       } else
>                 cpu_relax();
> 
That's fine as well. I shoved the while (!need_resched()) in there so we
wouldn't have to retest hlt_counter in the common case. hlt/nohlt is
something that's specified at boot, or by the board setup code, it's not
something that warrants a lot of retesting once we're booted.

Re: delayed switching

[Hideo S. <sa...@de...>]

Hi Paul,

On Thu, 10 May 2007 12:11:57 +0900, Paul Mundt wrote:
> How about this?

I hope to place the code looking at whether needs to reschedule in the same function as follows.

--- arch/sh/kernel/process.c.org	Thu Apr 26 12:08:32 2007
+++ arch/sh/kernel/process.c	Thu May 10 13:05:44 2007
@@ -17,54 +17,68 @@
 #include <linux/kexec.h>
 #include <asm/uaccess.h>
 #include <asm/mmu_context.h>
 #include <asm/ubc.h>
 
 static int hlt_counter;
 int ubc_usercnt = 0;
 
-#define HARD_IDLE_TIMEOUT (HZ / 3)
-
 void (*pm_idle)(void);
 void (*pm_power_off)(void);
 EXPORT_SYMBOL(pm_power_off);
 
 void disable_hlt(void)
 {
 	hlt_counter++;
 }
 EXPORT_SYMBOL(disable_hlt);
 
 void enable_hlt(void)
 {
 	hlt_counter--;
 }
 EXPORT_SYMBOL(enable_hlt);
 
-void default_idle(void)
+static int __init nohlt_setup(char *__unused)
+{
+	hlt_counter = 1;
+	return 1;
+}
+__setup("nohlt", nohlt_setup);
+
+static int __init hlt_setup(char *__unused)
 {
-	if (!hlt_counter)
-		cpu_sleep();
-	else
-		cpu_relax();
+	hlt_counter = 0;
+	return 1;
 }
+__setup("hlt", hlt_setup);
 
 void cpu_idle(void)
 {
+	set_thread_flag(TIF_POLLING_NRFLAG);
+
 	/* endless idle loop with no priority at all */
 	while (1) {
-		void (*idle)(void) = pm_idle;
-
-		if (!idle)
-			idle = default_idle;
-
-		while (!need_resched())
-			idle();
-
+		if (pm_idle) {
+			while (!need_resched())
+				pm_idle();
+		} else if (hlt_counter) {
+			while (!need_resched())
+				cpu_relax();
+		} else {
+			clear_thread_flag(TIF_POLLING_NRFLAG);
+			smp_mb__after_clear_bit();
+			set_bl_bit();
+			while (!need_resched())
+				cpu_sleep();
+			clear_bl_bit();
+			set_thread_flag(TIF_POLLING_NRFLAG);
+		}
+			
 		preempt_enable_no_resched();
 		schedule();
 		preempt_disable();
 	}
 }
 
 void machine_restart(char * __unused)
 {