[PATCH 0/11] Patches for Toshiba Cell reference set

[PATCH 0/11] Patches for Toshiba Cell reference set

[Ishizaki Kou]

Paul-san,

This is a revised version of the patch set for Toshiba Cell reference
set (aka Celleb). It is against powerpc.git. Please apply.


Changes from our previous patch set:

[PATCH 5/11] Celleb: htab routines
* use spin_lock_irqsave() instead of spin_lock()

[PATCH 8/11] Celleb: consolidate spu management ops
* (already posted)
  fix compile error in arch/powerpc/platforms/cell/spu_priv1_mmio.h


Best regards,
Kou Ishizaki
Toshiba

Re: [PATCH 0/11] Patches for Toshiba Cell reference set

[Benjamin Herrenschmidt]

On Fri, 2007-02-02 at 16:34 +0900, Ishizaki Kou wrote:
> Paul-san,
> 
> This is a revised version of the patch set for Toshiba Cell reference
> set (aka Celleb). It is against powerpc.git. Please apply.
> 
> 
> Changes from our previous patch set:
> 
> [PATCH 5/11] Celleb: htab routines
> * use spin_lock_irqsave() instead of spin_lock()

In which circumstances was that a problem ? iSeries uses spin_lock() ...
the caller should have disable IRQs before calling any of the low level
htab routines. If not, then there is a bug that needs to be fixed.

> [PATCH 8/11] Celleb: consolidate spu management ops
> * (already posted)
>   fix compile error in arch/powerpc/platforms/cell/spu_priv1_mmio.h

Ben.

Re: [PATCH 0/11] Patches for Toshiba Cell reference set

[Ishizaki Kou]

Ben-san,

> On Fri, 2007-02-02 at 16:34 +0900, Ishizaki Kou wrote:
> > Paul-san,
> > 
> > This is a revised version of the patch set for Toshiba Cell reference
> > set (aka Celleb). It is against powerpc.git. Please apply.
> > 
> > 
> > Changes from our previous patch set:
> > 
> > [PATCH 5/11] Celleb: htab routines
> > * use spin_lock_irqsave() instead of spin_lock()
> 
> In which circumstances was that a problem ? iSeries uses spin_lock() ...
> the caller should have disable IRQs before calling any of the low level
> htab routines. If not, then there is a bug that needs to be fixed.

It can be.

We found that following phenomenon:
 - An application calls "munmap"
 - do_munmap flushes TLB (so HTAB is invalidated)
 - when exiting spin_unlock(), DEC exception occurs. (!)
 - One of DEC handler causes data storage interrupt
 - Then, the kernel reenters HTAB handler and spin_lock().


BUG: spinlock recursion on CPU#0, modprobe/1752
 lock: c000000000460578, .magic: dead4ead, .owner: modprobe/1752,
.owner_cpu: 0
Call Trace:
[C000000007FF3640] [C00000000000FB00] .show_stack+0x68/0x1b0
(unreliable)
[C000000007FF36E0] [C0000000001AEB8C] .spin_bug+0xb0/0xd4
[C000000007FF3770] [C0000000001AECE8] ._raw_spin_lock+0x60/0x17c
[C000000007FF3810] [C00000000036287C] ._spin_lock+0x10/0x24
[C000000007FF3890] [C00000000003BA70]
.beat_lpar_hpte_updatepp+0x60/0x104
[C000000007FF3940] [C0000000000299E4] .htab_call_hpte_updatepp+0x4/0x18
[C000000007FF3A40] [C00000000002980C] .hash_page+0x1b0/0x1cc
[C000000007FF3AB0] [C0000000000049C0] .do_hash_page+0x40/0x5c
--- Exception: 401 at .add_grec+0x0/0x4e4 [ipv6]
    LR = .mld_ifc_timer_expire+0x1b8/0x2ac [ipv6]
[C000000007FF3DA0] [D000000000115DF8] .mld_ifc_timer_expire+0x188/0x2ac
[ipv6] (unreliable)
[C000000007FF3E40] [C00000000005C0AC] .run_timer_softirq+0x168/0x214
[C000000007FF3EF0] [C0000000000565A8] .__do_softirq+0xa8/0x164
[C000000007FF3F90] [C00000000002116C] .call_do_softirq+0x14/0x24
[C000000004E33430] [C00000000000C114] .do_softirq+0x68/0xac
[C000000004E334C0] [C0000000000566B8] .irq_exit+0x54/0x6c
[C000000004E33540] [C00000000001E508] .timer_interrupt+0x3e8/0x424
[C000000004E33620] [C000000000003578] decrementer_common+0xf8/0x100
--- Exception: 901 at .beat_hcall5+0x30/0x54
    LR = .beat_lpar_hpte_getword0+0x28/0x58
[C000000004E33910] [C000000000362794] ._spin_unlock_irqrestore+0x18/0x3c
(unreliable)
[C000000004E339D0] [C00000000003B624]
.beat_lpar_hpte_invalidate+0x64/0xe8
[C000000004E33A70] [C0000000000290F0] .flush_hash_page+0x74/0x88
[C000000004E33AE0] [C000000000029188] .flush_hash_range+0x84/0xb8
[C000000004E33B80] [C000000000029B18] .__flush_tlb_pending+0x78/0x9c
[C000000004E33C00] [C0000000000A6CBC] .unmap_region+0x160/0x1c0
[C000000004E33CD0] [C0000000000A7E80] .do_munmap+0x21c/0x2d4
[C000000004E33D90] [C0000000000A7F8C] .sys_munmap+0x54/0x88
[C000000004E33E30] [C000000000008634] syscall_exit+0x0/0x40
BUG: spinlock lockup on CPU#0, modprobe/1752, c000000000460578
Call Trace:
[C000000007FF36D0] [C00000000000FB00] .show_stack+0x68/0x1b0
(unreliable)
[C000000007FF3770] [C0000000001AEDC8] ._raw_spin_lock+0x140/0x17c
[C000000007FF3810] [C00000000036287C] ._spin_lock+0x10/0x24
[C000000007FF3890] [C00000000003BA70]
.beat_lpar_hpte_updatepp+0x60/0x104
[C000000007FF3940] [C0000000000299E4] .htab_call_hpte_updatepp+0x4/0x18
[C000000007FF3A40] [C00000000002980C] .hash_page+0x1b0/0x1cc
[C000000007FF3AB0] [C0000000000049C0] .do_hash_page+0x40/0x5c
--- Exception: 401 at .add_grec+0x0/0x4e4 [ipv6]
    LR = .mld_ifc_timer_expire+0x1b8/0x2ac [ipv6]
[C000000007FF3DA0] [D000000000115DF8] .mld_ifc_timer_expire+0x188/0x2ac
[ipv6] (unreliable)
[C000000007FF3E40] [C00000000005C0AC] .run_timer_softirq+0x168/0x214
[C000000007FF3EF0] [C0000000000565A8] .__do_softirq+0xa8/0x164
[C000000007FF3F90] [C00000000002116C] .call_do_softirq+0x14/0x24
[C000000004E33430] [C00000000000C114] .do_softirq+0x68/0xac
[C000000004E334C0] [C0000000000566B8] .irq_exit+0x54/0x6c
[C000000004E33540] [C00000000001E508] .timer_interrupt+0x3e8/0x424
[C000000004E33620] [C000000000003578] decrementer_common+0xf8/0x100
--- Exception: 901 at .beat_hcall5+0x30/0x54
    LR = .beat_lpar_hpte_getword0+0x28/0x58
[C000000004E33910] [C000000000362794] ._spin_unlock_irqrestore+0x18/0x3c
(unreliable)
[C000000004E339D0] [C00000000003B624]
.beat_lpar_hpte_invalidate+0x64/0xe8
[C000000004E33A70] [C0000000000290F0] .flush_hash_page+0x74/0x88
[C000000004E33AE0] [C000000000029188] .flush_hash_range+0x84/0xb8
[C000000004E33B80] [C000000000029B18] .__flush_tlb_pending+0x78/0x9c
[C000000004E33C00] [C0000000000A6CBC] .unmap_region+0x160/0x1c0
[C000000004E33CD0] [C0000000000A7E80] .do_munmap+0x21c/0x2d4
[C000000004E33D90] [C0000000000A7F8C] .sys_munmap+0x54/0x88
[C000000004E33E30] [C000000000008634] syscall_exit+0x0/0x40

Best regards,
Kou Ishizaki

Re: [PATCH 0/11] Patches for Toshiba Cell reference set

[Paul Mackerras]

Ishizaki Kou writes:

> We found that following phenomenon:
>  - An application calls "munmap"
>  - do_munmap flushes TLB (so HTAB is invalidated)
>  - when exiting spin_unlock(), DEC exception occurs. (!)
>  - One of DEC handler causes data storage interrupt

Interesting.  Did you find out exactly what was causing the DSI in the
decrementer interrupt handling?  It sounds like that is the real bug.

Paul.

Re: [PATCH 0/11] Patches for Toshiba Cell reference set

[Benjamin Herrenschmidt]

On Wed, 2007-02-07 at 08:05 +1100, Paul Mackerras wrote:
> Ishizaki Kou writes:
> 
> > We found that following phenomenon:
> >  - An application calls "munmap"
> >  - do_munmap flushes TLB (so HTAB is invalidated)
> >  - when exiting spin_unlock(), DEC exception occurs. (!)
> >  - One of DEC handler causes data storage interrupt
> 
> Interesting.  Did you find out exactly what was causing the DSI in the
> decrementer interrupt handling?  It sounds like that is the real bug.

Not necessarily... DEC -> module -> vmalloc'ed or ioremap'ed access for
example would trigger a DSI.

The Hash routines expect irqs off when called.

Ben.

Re: [PATCH 0/11] Patches for Toshiba Cell reference set

[Benjamin Herrenschmidt]

On Wed, 2007-02-07 at 10:30 +1100, Benjamin Herrenschmidt wrote:
> On Wed, 2007-02-07 at 08:05 +1100, Paul Mackerras wrote:
> > Ishizaki Kou writes:
> > 
> > > We found that following phenomenon:
> > >  - An application calls "munmap"
> > >  - do_munmap flushes TLB (so HTAB is invalidated)
> > >  - when exiting spin_unlock(), DEC exception occurs. (!)
> > >  - One of DEC handler causes data storage interrupt
> > 
> > Interesting.  Did you find out exactly what was causing the DSI in the
> > decrementer interrupt handling?  It sounds like that is the real bug.
> 
> Not necessarily... DEC -> module -> vmalloc'ed or ioremap'ed access for
> example would trigger a DSI.
> 
> The Hash routines expect irqs off when called.

Argh ! Actually no ! They expect irqs off in insert and updatepp, not in
invalidate. You'll notice that iSeries invalidate does a
local_irq_disable.

Ishiazaki-san: The proper fix is to do that too for Celleb. That is,
continue using a normal spinlock and add a local_irq_save/restore in
your invalidate callback (and only that one).

I wonder if we should "clean" that up and move that to the caller
though, and thus have a more consisent interface to the hash code.

Ben.

[PATCH 0/11] Patches for Toshiba Cell reference set

[Ishizaki Kou]

Paul-san,

This is a revised version of the patch set for Toshiba Cell reference
set (aka Celleb).


Changes from our previous patch set:

[PATCH 1/11] Celleb: interfaces to the hypervisor
* remove unused macros

[PATCH 2/11] Celleb: support interrupts
* remove unused variables
* cleanup spinlock initializing

[PATCH 3/11] Celleb: setup usb host controller in SCC
* add platform checking

[PATCH 4/11] Celleb: support iommu
* add platform checking

[PATCH 5/11] Celleb: htab routines
* add spinlock

[PATCH 6/11] Celleb: support udbg
* cleanup an unused variable

[PATCH 7/11] Celleb: hypervisor console driver
* add platform checking

[PATCH 8/11] Celleb: consolidate spu management ops
* spilit spu support patch into [8] and [9] (mostly NEW)
* consolidate spu management ops on of based platforms

[PATCH 9/11] Celleb: support spu priv1 ops
* spilit spu support patch into [8] and [9]
* chase changes of [8]

[PATCH 10/11] Celleb: basic support
* chase changes of [8]

[PATCH 11/11] Celleb: add celleb_defconfig
* nothing

Best regards,
Kou Ishizaki
Toshiba