linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* mips; boot fail after merge 3.9+
@ 2013-05-01  4:57 EUNBONG SONG
  2013-05-02  8:32 ` Jonas Gorski
  0 siblings, 1 reply; 13+ messages in thread
From: EUNBONG SONG @ 2013-05-01  4:57 UTC (permalink / raw)
  To: ralf, tglx, linux-mips; +Cc: linux-kernel

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=euc-kr, Size: 5185 bytes --]


Hello. 
After merge cavium board boots fail, boot log messages are as follows.
I enabled initcall_debug for debugging. 

[    0.000000] Initializing cgroup subsys cpuset
[    0.000000] Initializing cgroup subsys cpuacct
[    0.000000] Linux version 3.9.0+ (ebsong@ubi) (gcc version 4.4.1 (MontaVista Linux G++ 4.4-1302012138) ) #23 SMP PREEMPT Wed May 1 13:47:25 KST 2013
[    0.000000] octeon_bootinfo->dram_size: 4000
[    0.000000] Enabling tad1 error interrupt
[    0.000000] Enabling tad2 error interrupt
[    0.000000] Enabling tad3 error interrupt
[    0.000000] CVMSEG size: 2 cache lines (256 bytes)
[    0.000000] Cavium Networks SDK-2.1.0-p7 build 442
[    0.000000] bootconsole [early0] enabled
[    0.000000] CPU revision is: 000d9101 (Cavium Octeon II)
[    0.000000] Checking for the multiply/shift bug... no.
[    0.000000] Checking for the daddiu bug... no.
[    0.000000] Determined physical RAM map:
[    0.000000]  memory: 0000000006c00000 @ 0000000001100000 (usable)
[    0.000000]  memory: 0000000007c00000 @ 0000000008200000 (usable)
[    0.000000]  memory: 0000000011800000 @ 0000000030000000 (usable)
[    0.000000]  memory: 0000000000f85740 @ 0000000000100000 (usable)
[    0.000000] Wasting 14336 bytes for tracking 256 unused pages
[    0.000000] Initrd not found or empty - disabling initrd
[    0.000000] software IO TLB [mem 0x01b8a000-0x01bca000] (0MB) mapped at [a800000001b8a000-a800000001bc9fff]
[    0.000000] Zone ranges:
[    0.000000]   DMA32    [mem 0x00100000-0xefffffff]
[    0.000000]   Normal   empty
[    0.000000] Movable zone start for each node
[    0.000000] Early memory node ranges
[    0.000000]   node   0: [mem 0x00100000-0x01084fff]
[    0.000000]   node   0: [mem 0x01100000-0x07cfffff]
[    0.000000]   node   0: [mem 0x08200000-0x0fdfffff]
[    0.000000]   node   0: [mem 0x30000000-0x417fffff]
[    0.000000] Cavium Hotplug: Available coremask 0xffffff00
[    0.000000] Primary instruction cache 37kB, virtually tagged, 37 way, 8 sets, linesize 128 bytes.
[    0.000000] Primary data cache 32kB, 32-way, 8 sets, linesize 128 bytes.
[    0.000000] PERCPU: Embedded 11 pages/cpu @a800000001beb000 s12800 r8192 d24064 u45056
[    0.000000] Built 1 zonelists in Zone order, mobility grouping on.  Total pages: 133198
[    0.000000] Kernel command line:  root=/dev/nfs rw nfsroot=165.213.176.145:/home/ebsong/project/presmb/apc/Pkg/nfsroot/wec8500 ip=165.213.176.108:165.213.176.145:165.213.176.1:255.255.255.0::mgmt0 console=ttyS0,115200
[    0.000000] PID hash table entries: 4096 (order: 3, 32768 bytes)
[    0.000000] Dentry cache hash table entries: 131072 (order: 8, 1048576 bytes)
[    0.000000] Inode-cache hash table entries: 65536 (order: 7, 524288 bytes)
[    0.000000] Memory: 510160k/540180k available (5811k kernel code, 30020k reserved, 8933k data, 292k init, 0k highmem)
[    0.000000] Preemptible hierarchical RCU implementation.
[    0.000000] NR_IRQS:453
[    0.000000] Console: colour dummy device 80x25
[   26.701167] Calibrating delay loop (skipped) preset value.. 2400.00 BogoMIPS (lpj=12000000)
[   26.709507] pid_max: default: 32768 minimum: 301
[   26.714162] Security Framework initialized
[   26.718218] Mount-cache hash table entries: 256
[   26.723398] Initializing cgroup subsys devices
[   26.727669] Initializing cgroup subsys freezer
[   26.732131] Checking for the daddi bug... no.
[   26.736607] calling  cnmips_cu2_setup+0x0/0xc @ 1
[   26.741139] initcall cnmips_cu2_setup+0x0/0xc returned 0 after 0 usecs
[   26.747659] calling  spawn_ksoftirqd+0x0/0x34 @ 1

I found this issue after cdbedc61c8d0122ad682815936f0d11df1fe5f57. 
And i found something strange. I ran the git show for this commit.
As below "select GENERIC_IDLE_LOOP" is added for CONFIG_MIPS. 
but the latest arch/mips/Kconfig file has not this one. I have tried to find when this is gone. but i can't find.
Is there any problem with this? 


commit cdbedc61c8d0122ad682815936f0d11df1fe5f57
Author: Thomas Gleixner <tglx@linutronix.de>
Date:   Thu Mar 21 22:49:52 2013 +0100

    mips: Use generic idle loop

    Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
    Cc: Linus Torvalds <torvalds@linux-foundation.org>
    Cc: Rusty Russell <rusty@rustcorp.com.au>
    Cc: Paul McKenney <paulmck@linux.vnet.ibm.com>
    Cc: Peter Zijlstra <peterz@infradead.org>
    Reviewed-by: Cc: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com>
    Cc: Magnus Damm <magnus.damm@gmail.com>
    Cc: Ralf Baechle <ralf@linux-mips.org>
    Link: http://lkml.kernel.org/r/20130321215234.754954871@linutronix.de
    Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

diff --git a/arch/mips/Kconfig b/arch/mips/Kconfig
index 51244bf..e1a3d02 100644
--- a/arch/mips/Kconfig
+++ b/arch/mips/Kconfig
@@ -34,6 +34,7 @@ config MIPS
        select HAVE_MEMBLOCK_NODE_MAP
        select ARCH_DISCARD_MEMBLOCK
        select GENERIC_SMP_IDLE_THREAD
+       select GENERIC_IDLE_LOOP
        select BUILDTIME_EXTABLE_SORT
        select GENERIC_CLOCKEVENTS
        select GENERIC_CMOS_UPDATE
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

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

* Re: mips; boot fail after merge 3.9+
  2013-05-01  4:57 mips; boot fail after merge 3.9+ EUNBONG SONG
@ 2013-05-02  8:32 ` Jonas Gorski
  2013-05-02 10:42   ` Thomas Gleixner
  0 siblings, 1 reply; 13+ messages in thread
From: Jonas Gorski @ 2013-05-02  8:32 UTC (permalink / raw)
  To: eunb.song; +Cc: ralf, tglx, linux-mips, linux-kernel

On Wed, May 1, 2013 at 6:57 AM, EUNBONG SONG <eunb.song@samsung.com> wrote:
>
> Hello.
> After merge cavium board boots fail, boot log messages are as follows.
> I enabled initcall_debug for debugging.

I can confirm that MIPS does not seem to finish to boot after using
the generic idle loop, I have the same problem on a different platform
(bcm63xx), and bisecting showed the same commit.

(snip)

> I found this issue after cdbedc61c8d0122ad682815936f0d11df1fe5f57.
> And i found something strange. I ran the git show for this commit.
> As below "select GENERIC_IDLE_LOOP" is added for CONFIG_MIPS.
> but the latest arch/mips/Kconfig file has not this one. I have tried to find when this is gone. but i can't find.
> Is there any problem with this?

No, after all architectures were converted to use the generic idle
loop the config symbol was removed, so it's now always on. The problem
is rather that the generic idle loop does not seem to work on MIPS.
Unfortunately due to limited knowledge in this area I can't really
tell which part broke it.


Jonas

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

* Re: mips; boot fail after merge 3.9+
  2013-05-02  8:32 ` Jonas Gorski
@ 2013-05-02 10:42   ` Thomas Gleixner
  2013-05-02 11:20     ` Jonas Gorski
  0 siblings, 1 reply; 13+ messages in thread
From: Thomas Gleixner @ 2013-05-02 10:42 UTC (permalink / raw)
  To: Jonas Gorski; +Cc: eunb.song, ralf, linux-mips, linux-kernel

On Thu, 2 May 2013, Jonas Gorski wrote:

> On Wed, May 1, 2013 at 6:57 AM, EUNBONG SONG <eunb.song@samsung.com> wrote:
> >
> > Hello.
> > After merge cavium board boots fail, boot log messages are as follows.
> > I enabled initcall_debug for debugging.
> 
> I can confirm that MIPS does not seem to finish to boot after using
> the generic idle loop, I have the same problem on a different platform
> (bcm63xx), and bisecting showed the same commit.
> 
> (snip)
> 
> > I found this issue after cdbedc61c8d0122ad682815936f0d11df1fe5f57.
> > And i found something strange. I ran the git show for this commit.
> > As below "select GENERIC_IDLE_LOOP" is added for CONFIG_MIPS.
> > but the latest arch/mips/Kconfig file has not this one. I have tried to find when this is gone. but i can't find.
> > Is there any problem with this?
> 
> No, after all architectures were converted to use the generic idle
> loop the config symbol was removed, so it's now always on. The problem
> is rather that the generic idle loop does not seem to work on MIPS.
> Unfortunately due to limited knowledge in this area I can't really
> tell which part broke it.

Does the patch below fix your issue ?

Thanks,

	tglx

diff --git a/kernel/cpu/idle.c b/kernel/cpu/idle.c
index 8b86c0c..a8972fe 100644
--- a/kernel/cpu/idle.c
+++ b/kernel/cpu/idle.c
@@ -70,8 +70,10 @@ static void cpu_idle_loop(void)
 			check_pgt_cache();
 			rmb();
 
-			if (cpu_is_offline(smp_processor_id()))
+			if (cpu_is_offline(smp_processor_id())) {
 				arch_cpu_idle_dead();
+				continue;
+			}
 
 			local_irq_disable();
 			arch_cpu_idle_enter();

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

* Re: mips; boot fail after merge 3.9+
  2013-05-02 10:42   ` Thomas Gleixner
@ 2013-05-02 11:20     ` Jonas Gorski
  2013-05-02 14:33       ` [PATCH] MIPS: Enable interrupts in arch_cpu_idle() Thomas Gleixner
  0 siblings, 1 reply; 13+ messages in thread
From: Jonas Gorski @ 2013-05-02 11:20 UTC (permalink / raw)
  To: Thomas Gleixner; +Cc: eunb.song, ralf, linux-mips, linux-kernel

On Thu, May 2, 2013 at 12:42 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
> Does the patch below fix your issue ?

Does not work for me either. I don't even have SMP enabled, so this
codepath isn't taken for me at all.


Jonas

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

* [PATCH] MIPS: Enable interrupts in arch_cpu_idle()
  2013-05-02 11:20     ` Jonas Gorski
@ 2013-05-02 14:33       ` Thomas Gleixner
  2013-05-02 14:58         ` Ralf Baechle
  0 siblings, 1 reply; 13+ messages in thread
From: Thomas Gleixner @ 2013-05-02 14:33 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Jonas Gorski, eunb.song, ralf, linux-mips, linux-kernel

commit cdbedc61c8 (mips: Use generic idle loop) broke MIPS as I did
not realize that MIPS wants to invoke the wait instructions with
interrupts enabled. Don't ask why that works correctly; Ralf suggested
to get thoroughly drunk before even thinking about it. Looking sober
at commit c65a5480 ([MIPS] Fix potential latency problem due to
non-atomic cpu_wait) is not recommended.

Enable interrupts in arch_cpu_idle() on mips to repair the issue.

Reported-and-tested-by: Jonas Gorski <jogo@openwrt.org>
Reported-by: EunBong Song <eunb.song@samsung.com>
Booze-recommended-by: Ralf Baechle <ralf@linux-mips.org>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

Index: linux-2.6/arch/mips/kernel/process.c
===================================================================
--- linux-2.6.orig/arch/mips/kernel/process.c
+++ linux-2.6/arch/mips/kernel/process.c
@@ -50,13 +50,18 @@ void arch_cpu_idle_dead(void)
 }
 #endif
 
-void arch_cpu_idle(void)
+static void smtc_idle_hook(void)
 {
 #ifdef CONFIG_MIPS_MT_SMTC
 	extern void smtc_idle_loop_hook(void);
-
 	smtc_idle_loop_hook();
 #endif
+}
+
+void arch_cpu_idle(void)
+{
+	local_irq_enable();
+	smtc_idle_hook();
 	if (cpu_wait)
 		(*cpu_wait)();
 	else

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

* Re: [PATCH] MIPS: Enable interrupts in arch_cpu_idle()
  2013-05-02 14:33       ` [PATCH] MIPS: Enable interrupts in arch_cpu_idle() Thomas Gleixner
@ 2013-05-02 14:58         ` Ralf Baechle
  2013-05-02 16:45           ` Linus Torvalds
  0 siblings, 1 reply; 13+ messages in thread
From: Ralf Baechle @ 2013-05-02 14:58 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Linus Torvalds, Jonas Gorski, eunb.song, linux-mips, linux-kernel

On Thu, May 02, 2013 at 04:33:52PM +0200, Thomas Gleixner wrote:

> commit cdbedc61c8 (mips: Use generic idle loop) broke MIPS as I did
> not realize that MIPS wants to invoke the wait instructions with
> interrupts enabled. Don't ask why that works correctly; Ralf suggested
> to get thoroughly drunk before even thinking about it. Looking sober
> at commit c65a5480 ([MIPS] Fix potential latency problem due to
> non-atomic cpu_wait) is not recommended.
> 
> Enable interrupts in arch_cpu_idle() on mips to repair the issue.
> 
> Reported-and-tested-by: Jonas Gorski <jogo@openwrt.org>
> Reported-by: EunBong Song <eunb.song@samsung.com>
> Booze-recommended-by: Ralf Baechle <ralf@linux-mips.org>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>

A very sobering commit message ;-)

The underlying issue is that the WAIT instruction on MIPS is architecturally
defined as "[...] It is implementation-dependent whether the pipeline
restarts when a non-enabled interrupt is requested.  [...]"

arch_local_irq_disable() disables interrupts by clearing the IE bit in
the CPU status register, thus disabling all interrupts.  If no a WAIT
instruction is executed it's legal for a CPU to stop the pipeline for
good.  Which obviously is pretty stupidtastik behaviour.

For a while we just used to live with the race condition resulting from
not disabling interrupts in the idle loop.  Then c65a5480 fixed this by
checking if we're returning to the WAIT instruction in the idle loop
when returning from an interrupt and iff so, rolling back the
program counter to point to the if (test_thread_flag(TIF_NEED_RESCHED))
test at the beginning of rollback_r4k_wait.

Linux knows which CPUs have the problematic behaviour and uses this special
variant of the idle loop only where needed.

  Ralf

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

* Re: [PATCH] MIPS: Enable interrupts in arch_cpu_idle()
  2013-05-02 14:58         ` Ralf Baechle
@ 2013-05-02 16:45           ` Linus Torvalds
  2013-05-02 20:48             ` [PATCH] MIPS: Enable interrupts before WAIT instruction David Daney
  0 siblings, 1 reply; 13+ messages in thread
From: Linus Torvalds @ 2013-05-02 16:45 UTC (permalink / raw)
  To: Ralf Baechle
  Cc: Thomas Gleixner, Jonas Gorski, eunb.song, linux-mips, linux-kernel

On Thu, May 2, 2013 at 7:58 AM, Ralf Baechle <ralf@linux-mips.org> wrote:
>
> For a while we just used to live with the race condition resulting from
> not disabling interrupts in the idle loop.  Then c65a5480 fixed this by
> checking if we're returning to the WAIT instruction in the idle loop
> when returning from an interrupt and iff so, rolling back the
> program counter to point to the if (test_thread_flag(TIF_NEED_RESCHED))
> test at the beginning of rollback_r4k_wait.

Umm. That sounds buggy. What if there was an interrupt *between* the
two places, not right at the wait instruction?

It seriously sounds like MIPS should do this by enabling interrupts in
the *assembly* code immediately preceding the wait instruction. IOW,
you'd effectively have the same kind of "sti; halt" kind of sequence
that x86 has. Not "enable interrupts" + C compiler puts random
instructions here + "wait".

                       Linus

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

* [PATCH] MIPS: Enable interrupts before WAIT instruction.
  2013-05-02 16:45           ` Linus Torvalds
@ 2013-05-02 20:48             ` David Daney
  2013-05-02 21:04               ` Thomas Gleixner
                                 ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: David Daney @ 2013-05-02 20:48 UTC (permalink / raw)
  To: linux-mips, ralf
  Cc: Linus Torvalds, linux-kernel, David Daney, Thomas Gleixner, Jonas Gorski

From: David Daney <david.daney@cavium.com>

As noted by Thomas Gleixner:

   commit cdbedc61c8 (mips: Use generic idle loop) broke MIPS as I did
   not realize that MIPS wants to invoke the wait instructions with
   interrupts enabled.

Instead of enabling interrupts in arch_cpu_idle() as Thomas' initial
patch does, we follow Linus' suggestion of doing it in the assembly
code to prevent the compiler from rearranging things.

Signed-off-by: David Daney <david.daney@cavium.com>
Reported-by: EunBong Song <eunb.song@samsung.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Jonas Gorski <jogo@openwrt.org>
---

This is only very lightly tested, we need more testing before
declaring it the definitive fix.

 arch/mips/kernel/genex.S | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/mips/kernel/genex.S b/arch/mips/kernel/genex.S
index ecb347c..57cda9a 100644
--- a/arch/mips/kernel/genex.S
+++ b/arch/mips/kernel/genex.S
@@ -132,12 +132,13 @@ LEAF(r4k_wait)
 	.set	noreorder
 	/* start of rollback region */
 	LONG_L	t0, TI_FLAGS($28)
-	nop
 	andi	t0, _TIF_NEED_RESCHED
 	bnez	t0, 1f
 	 nop
-	nop
-	nop
+	/* Enable interrupts so WAIT will complete */
+	mfc0	t0, CP0_STATUS
+	ori	t0, ST0_IE
+	mtc0	t0, CP0_STATUS
 	.set	mips3
 	wait
 	/* end of rollback region (the region size must be power of two) */
-- 
1.7.11.7


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

* Re: [PATCH] MIPS: Enable interrupts before WAIT instruction.
  2013-05-02 20:48             ` [PATCH] MIPS: Enable interrupts before WAIT instruction David Daney
@ 2013-05-02 21:04               ` Thomas Gleixner
  2013-05-03  9:54                 ` Jonas Gorski
  2013-05-06 14:26               ` Manuel Lauss
  2013-05-22 22:32               ` Aaro Koskinen
  2 siblings, 1 reply; 13+ messages in thread
From: Thomas Gleixner @ 2013-05-02 21:04 UTC (permalink / raw)
  To: David Daney
  Cc: linux-mips, ralf, Linus Torvalds, linux-kernel, David Daney,
	Jonas Gorski



On Thu, 2 May 2013, David Daney wrote:

> From: David Daney <david.daney@cavium.com>
> 
> As noted by Thomas Gleixner:
> 
>    commit cdbedc61c8 (mips: Use generic idle loop) broke MIPS as I did
>    not realize that MIPS wants to invoke the wait instructions with
>    interrupts enabled.
> 
> Instead of enabling interrupts in arch_cpu_idle() as Thomas' initial
> patch does, we follow Linus' suggestion of doing it in the assembly
> code to prevent the compiler from rearranging things.

Yeah, that looks way more sane.

 
> Signed-off-by: David Daney <david.daney@cavium.com>
> Reported-by: EunBong Song <eunb.song@samsung.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Jonas Gorski <jogo@openwrt.org>
> ---
> 
> This is only very lightly tested, we need more testing before
> declaring it the definitive fix.
> 
>  arch/mips/kernel/genex.S | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/mips/kernel/genex.S b/arch/mips/kernel/genex.S
> index ecb347c..57cda9a 100644
> --- a/arch/mips/kernel/genex.S
> +++ b/arch/mips/kernel/genex.S
> @@ -132,12 +132,13 @@ LEAF(r4k_wait)
>  	.set	noreorder
>  	/* start of rollback region */
>  	LONG_L	t0, TI_FLAGS($28)
> -	nop
>  	andi	t0, _TIF_NEED_RESCHED
>  	bnez	t0, 1f
>  	 nop
> -	nop
> -	nop
> +	/* Enable interrupts so WAIT will complete */
> +	mfc0	t0, CP0_STATUS
> +	ori	t0, ST0_IE
> +	mtc0	t0, CP0_STATUS
>  	.set	mips3
>  	wait
>  	/* end of rollback region (the region size must be power of two) */
> -- 
> 1.7.11.7
> 
> 

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

* Re: [PATCH] MIPS: Enable interrupts before WAIT instruction.
  2013-05-02 21:04               ` Thomas Gleixner
@ 2013-05-03  9:54                 ` Jonas Gorski
  0 siblings, 0 replies; 13+ messages in thread
From: Jonas Gorski @ 2013-05-03  9:54 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: David Daney, linux-mips, ralf, Linus Torvalds, linux-kernel, David Daney

On Thu, May 2, 2013 at 11:04 PM, Thomas Gleixner <tglx@linutronix.de> wrote:
>
>
> On Thu, 2 May 2013, David Daney wrote:
>
>> From: David Daney <david.daney@cavium.com>
>>
>> As noted by Thomas Gleixner:
>>
>>    commit cdbedc61c8 (mips: Use generic idle loop) broke MIPS as I did
>>    not realize that MIPS wants to invoke the wait instructions with
>>    interrupts enabled.
>>
>> Instead of enabling interrupts in arch_cpu_idle() as Thomas' initial
>> patch does, we follow Linus' suggestion of doing it in the assembly
>> code to prevent the compiler from rearranging things.
>
> Yeah, that looks way more sane.

In a first quick test I can also confirm that it seems to work (as an
alternative to the other fix).


Jonas

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

* Re: [PATCH] MIPS: Enable interrupts before WAIT instruction.
  2013-05-02 20:48             ` [PATCH] MIPS: Enable interrupts before WAIT instruction David Daney
  2013-05-02 21:04               ` Thomas Gleixner
@ 2013-05-06 14:26               ` Manuel Lauss
  2013-05-22 22:32               ` Aaro Koskinen
  2 siblings, 0 replies; 13+ messages in thread
From: Manuel Lauss @ 2013-05-06 14:26 UTC (permalink / raw)
  To: David Daney
  Cc: Linux-MIPS, Ralf Baechle, Linus Torvalds, LKML, David Daney,
	Thomas Gleixner, Jonas Gorski

Hi David,

On Thu, May 2, 2013 at 10:48 PM, David Daney <ddaney.cavm@gmail.com> wrote:
> From: David Daney <david.daney@cavium.com>
>
> As noted by Thomas Gleixner:
>
>    commit cdbedc61c8 (mips: Use generic idle loop) broke MIPS as I did
>    not realize that MIPS wants to invoke the wait instructions with
>    interrupts enabled.
>
> Instead of enabling interrupts in arch_cpu_idle() as Thomas' initial
> patch does, we follow Linus' suggestion of doing it in the assembly
> code to prevent the compiler from rearranging things.
>
> Signed-off-by: David Daney <david.daney@cavium.com>
> Reported-by: EunBong Song <eunb.song@samsung.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Jonas Gorski <jogo@openwrt.org>
> ---
>
> This is only very lightly tested, we need more testing before
> declaring it the definitive fix.
>
>  arch/mips/kernel/genex.S | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)

Unfortunately this patch doesn't work for me, system just hangs at
certain points
during kernel startup.  Reverting the patch above fixes it, but with
this warning:

ehci-platform ehci-platform.0: irq 98, io mem 0x14020000
------------[ cut here ]------------
WARNING: at /home/mano/db1200/kernel/linux/kernel/cpu/idle.c:96
cpu_startup_entry+0x138/0x184()
CPU: 0 PID: 0 Comm: swapper Not tainted 3.9.0-db1235-10522-g6295a89 #2
Stack : 00000000 00000000 809b4462 00000046 80929c20 00000000 808c6504 00000000
          808c3428 80929b27 80929dc8 00000000 809b3c04 00000000
00000000 00000000
          80093348 807d1000 2cb41780 8011f458 00000000 00000000
808c4a28 8091fe24
          00000000 00000000 00000000 00000000 00000000 00000000
00000000 00000000
          00000000 00000000 00000000 00000000 00000000 00000000
00000000 8091fdb0
          ...
Call Trace:
[<8010a1fc>] show_stack+0x64/0x7c
[<8011f614>] warn_slowpath_common+0x70/0xa0
[<8011f700>] warn_slowpath_null+0x18/0x28
[<80150464>] cpu_startup_entry+0x138/0x184
[<809658f0>] start_kernel+0x360/0x378

---[ end trace 19427144468f733d ]---
ehci-platform ehci-platform.0: USB 2.0 started, EHCI 1.00
hub 1-0:1.0: USB hub found


Thanks,
      Manuel

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

* Re: [PATCH] MIPS: Enable interrupts before WAIT instruction.
  2013-05-02 20:48             ` [PATCH] MIPS: Enable interrupts before WAIT instruction David Daney
  2013-05-02 21:04               ` Thomas Gleixner
  2013-05-06 14:26               ` Manuel Lauss
@ 2013-05-22 22:32               ` Aaro Koskinen
  2013-05-22 22:36                 ` David Daney
  2 siblings, 1 reply; 13+ messages in thread
From: Aaro Koskinen @ 2013-05-22 22:32 UTC (permalink / raw)
  To: David Daney
  Cc: linux-mips, ralf, Linus Torvalds, linux-kernel, David Daney,
	Thomas Gleixner, Jonas Gorski

Hi,

On Thu, May 02, 2013 at 01:48:12PM -0700, David Daney wrote:
> From: David Daney <david.daney@cavium.com>
> 
> As noted by Thomas Gleixner:
> 
>    commit cdbedc61c8 (mips: Use generic idle loop) broke MIPS as I did
>    not realize that MIPS wants to invoke the wait instructions with
>    interrupts enabled.
> 
> Instead of enabling interrupts in arch_cpu_idle() as Thomas' initial
> patch does, we follow Linus' suggestion of doing it in the assembly
> code to prevent the compiler from rearranging things.
> 
> Signed-off-by: David Daney <david.daney@cavium.com>
> Reported-by: EunBong Song <eunb.song@samsung.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Jonas Gorski <jogo@openwrt.org>
> ---
> 
> This is only very lightly tested, we need more testing before
> declaring it the definitive fix.

I wonder what is the status of this patch? Or is there some alternative
fix?

I have Octeon+ board that hangs during 3.10-rc2 boot in spawn_ksoftirqd()
without this. Also, this patch does not apply cleanly to 3.10-rc2
anymore...

A.

>  arch/mips/kernel/genex.S | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/mips/kernel/genex.S b/arch/mips/kernel/genex.S
> index ecb347c..57cda9a 100644
> --- a/arch/mips/kernel/genex.S
> +++ b/arch/mips/kernel/genex.S
> @@ -132,12 +132,13 @@ LEAF(r4k_wait)
>  	.set	noreorder
>  	/* start of rollback region */
>  	LONG_L	t0, TI_FLAGS($28)
> -	nop
>  	andi	t0, _TIF_NEED_RESCHED
>  	bnez	t0, 1f
>  	 nop
> -	nop
> -	nop
> +	/* Enable interrupts so WAIT will complete */
> +	mfc0	t0, CP0_STATUS
> +	ori	t0, ST0_IE
> +	mtc0	t0, CP0_STATUS
>  	.set	mips3
>  	wait
>  	/* end of rollback region (the region size must be power of two) */
> -- 
> 1.7.11.7
> 
> 

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

* Re: [PATCH] MIPS: Enable interrupts before WAIT instruction.
  2013-05-22 22:32               ` Aaro Koskinen
@ 2013-05-22 22:36                 ` David Daney
  0 siblings, 0 replies; 13+ messages in thread
From: David Daney @ 2013-05-22 22:36 UTC (permalink / raw)
  To: Aaro Koskinen
  Cc: David Daney, linux-mips, ralf, Linus Torvalds, linux-kernel,
	David Daney, Thomas Gleixner, Jonas Gorski

On 05/22/2013 03:32 PM, Aaro Koskinen wrote:
> Hi,
>
> On Thu, May 02, 2013 at 01:48:12PM -0700, David Daney wrote:
>> From: David Daney <david.daney@cavium.com>
>>
>> As noted by Thomas Gleixner:
>>
>>     commit cdbedc61c8 (mips: Use generic idle loop) broke MIPS as I did
>>     not realize that MIPS wants to invoke the wait instructions with
>>     interrupts enabled.
>>
>> Instead of enabling interrupts in arch_cpu_idle() as Thomas' initial
>> patch does, we follow Linus' suggestion of doing it in the assembly
>> code to prevent the compiler from rearranging things.
>>
>> Signed-off-by: David Daney <david.daney@cavium.com>
>> Reported-by: EunBong Song <eunb.song@samsung.com>
>> Cc: Thomas Gleixner <tglx@linutronix.de>
>> Cc: Jonas Gorski <jogo@openwrt.org>
>> ---
>>
>> This is only very lightly tested, we need more testing before
>> declaring it the definitive fix.
>
> I wonder what is the status of this patch? Or is there some alternative
> fix?
>
> I have Octeon+ board that hangs during 3.10-rc2 boot in spawn_ksoftirqd()
> without this. Also, this patch does not apply cleanly to 3.10-rc2
> anymore...
>
> A.
>

Ralf has an alternate fix here:

http://git.linux-mips.org/pub/scm/ralf/upstream-linus.git

If all goes well, it will be merged soon.

David Daney




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

end of thread, other threads:[~2013-05-22 22:36 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-05-01  4:57 mips; boot fail after merge 3.9+ EUNBONG SONG
2013-05-02  8:32 ` Jonas Gorski
2013-05-02 10:42   ` Thomas Gleixner
2013-05-02 11:20     ` Jonas Gorski
2013-05-02 14:33       ` [PATCH] MIPS: Enable interrupts in arch_cpu_idle() Thomas Gleixner
2013-05-02 14:58         ` Ralf Baechle
2013-05-02 16:45           ` Linus Torvalds
2013-05-02 20:48             ` [PATCH] MIPS: Enable interrupts before WAIT instruction David Daney
2013-05-02 21:04               ` Thomas Gleixner
2013-05-03  9:54                 ` Jonas Gorski
2013-05-06 14:26               ` Manuel Lauss
2013-05-22 22:32               ` Aaro Koskinen
2013-05-22 22:36                 ` David Daney

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