[patch 0/6 -rt] powerpc 2.6.20-rt8: fix boot/runtime errors/warnings for PowerPC(ppc64)

[patch 0/6 -rt] powerpc 2.6.20-rt8: fix boot/runtime errors/warnings for PowerPC(ppc64)

[Tsutomu OWA]

Hi Ingo,

  Please consider for inclusion in your rt tree.

  This series of patches fixes boot and runntime errors/warnings for
powerpc (esp. 64 bit).  This applies to linux-2.6.20, patch-2.6.20-rt8
and previous my patch set;
  http://ozlabs.org/pipermail/linuxppc-dev/2007-March/032640.html
  http://lkml.org/lkml/2007/3/6/503

  Compile and boot tested on celleb (Cell Reference set) for both
PREEMPT_RT=y and PREEMPT_NONE=y.

  CONFIG_MCOUNT, CONFIG_LATENCY_TRACE and other tracing options nor
CONFIG_GENERIC_TIME, clockevents etc are not yet ported.

  Comments and suggestions are welcome.

  Thanks in advance.
-- owa
TOSHIBA, Software Engineering Center.

[patch 1/6 -rt] powerpc 2.6.20-rt8: add preemption checks for NEED_RESCHED_DELAYED.

[Tsutomu OWA]

  To add preemption checks for the NEED_RESCHED_DELAYED flag.

Signed-off-by: Tsutomu Owa <tsutomu.owa@toshiba.co.jp>
-- owa

diff -rup linux-rt8/include/asm-powerpc/thread_info.h rt/include/asm-powerpc/thread_info.h
--- linux-rt8/include/asm-powerpc/thread_info.h	2007-02-20 14:30:40.000000000 +0900
+++ rt/include/asm-powerpc/thread_info.h	2007-02-20 15:39:25.000000000 +0900
@@ -146,7 +146,8 @@ static inline struct thread_info *curren
 #define _TIF_SYSCALL_T_OR_A	(_TIF_SYSCALL_TRACE|_TIF_SYSCALL_AUDIT|_TIF_SECCOMP)
 
 #define _TIF_USER_WORK_MASK	(_TIF_NOTIFY_RESUME | _TIF_SIGPENDING | \
-				 _TIF_NEED_RESCHED | _TIF_RESTORE_SIGMASK)
+				 _TIF_NEED_RESCHED | _TIF_RESTORE_SIGMASK | \
+				 _TIF_NEED_RESCHED_DELAYED)
 #define _TIF_PERSYSCALL_MASK	(_TIF_RESTOREALL|_TIF_NOERROR)
 
 /* Bits in local_flags */
diff -rup linux-rt8/arch/powerpc/kernel/entry_64.S rt/arch/powerpc/kernel/entry_64.S
--- linux-rt8/arch/powerpc/kernel/entry_64.S	2007-02-20 09:38:52.000000000 +0900
+++ rt/arch/powerpc/kernel/entry_64.S	2007-03-05 11:59:17.000000000 +0900
@@ -444,7 +444,8 @@ _GLOBAL(ret_from_except_lite)
 
 #ifdef CONFIG_PREEMPT
 	clrrdi	r9,r1,THREAD_SHIFT	/* current_thread_info() */
-	li	r0,_TIF_NEED_RESCHED	/* bits to check */
+	li	r0,(_TIF_NEED_RESCHED|_TIF_NEED_RESCHED_DELAYED)
+					/* bits to check */
 	ld	r3,_MSR(r1)
 	ld	r4,TI_FLAGS(r9)
 	/* Move MSR_PR bit in r3 to _TIF_SIGPENDING position in r0 */
@@ -565,7 +566,7 @@ do_work:
 	rotldi	r10,r10,16
 	mtmsrd	r10,1
 	ld	r4,TI_FLAGS(r9)
-	andi.	r0,r4,_TIF_NEED_RESCHED
+	andi.	r0,r4,(_TIF_NEED_RESCHED|_TIF_NEED_RESCHED_DELAYED)
 	bne	1b
 	b	restore
 
@@ -575,7 +576,7 @@ user_work:
 	ori	r10,r10,MSR_EE
 	mtmsrd	r10,1
 
-	andi.	r0,r4,_TIF_NEED_RESCHED
+	andi.	r0,r4,(_TIF_NEED_RESCHED|_TIF_NEED_RESCHED_DELAYED)
 	beq	1f
 	bl	.schedule
 	b	.ret_from_except_lite
diff -rup linux-rt8/arch/powerpc/kernel/idle.c rt/arch/powerpc/kernel/idle.c
--- linux-rt8/arch/powerpc/kernel/idle.c	2007-02-20 14:30:38.000000000 +0900
+++ rt/arch/powerpc/kernel/idle.c	2007-02-20 15:43:04.000000000 +0900
@@ -56,7 +56,8 @@ void cpu_idle(void)
 
 	set_thread_flag(TIF_POLLING_NRFLAG);
 	while (1) {
-		while (!need_resched() && !cpu_should_die()) {
+		while (!need_resched() && !need_resched_delayed() &&
+				!cpu_should_die()) {
 			ppc64_runlatch_off();
 
 			if (ppc_md.power_save) {

[patch 2/6 -rt] powerpc 2.6.20-rt8: to convert spinlocks to raw ones.

[Tsutomu OWA]

  To convert the spinlocks into the raw onces to fix the following warnings/errors.

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - 
Badness at arch/powerpc/kernel/entry_64.S:651
Call Trace:
[C0000000006133E0] [C00000000000FAAC] show_stack+0x68/0x1b0 (unreliable)
[C000000000613480] [C0000000001EF004] .repor000001EF004] .report_bug+0x94/0xe8
[C000000000613510] [C0000000003EAD58] .program_check_exception+0x170/0x5a8
[C00000000000487C] program_check_common+0xfc/0x100
--- Exception: 700 at .exception: 700 at .enter_rtas+0xa0/0x10c
    LR = .rtas_call+0x144/0x1e8
[C0000000006138D0] [C000000000613980] 0xc000000000613980 (unreliabl0064a0d8 (unreliable)
[C000000000613AB0] [C00000000001DFA8] .rtas_call+0x144/0x1etas_call+0x144/0x1e8
	:

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - 
BUG: scheduling with irqs disabled: ash/0x00000000/462
caller is .rt_spin_lock_slowlock+0x14c/0x234
Call Trace:
[C00000001FE87580] [C00000000000FAAC] .show_stack+0x68/0x1b0 (unreliable)
[C00000001FE87620] [C0000000003E796C] .schedule+0x78/0x128
[C00000001FE876A0] [C0000000003E9378] .rt_spin_lock_slowlock+0x14c/0x234
[C00000001FE877B0] [C00000000002E3C4] .native_hpte_updatepp+0x158/0x274
[C00000001FE87850] [C00000000002C374] .htab_call_hpte_updatepp+0x4/0x18
[C00000001FE87950] [C00000000002BC74] .hash_preload+0x150/0x198
[C00000001FE87A00] [C00000000002A9E8] .update_mmu_cache+0x13c/0x1a4
[C00000001FE87A90] [C0000000000B28A0] .do_wp_page+0x7d0/0x888
[C00000001FE87B60] [C0000000000B4CB8] .__handle_mm_fault+0xee4/0x1004
[C00000001FE87C50] [C0000000003EC288] .do_page_fault+0x474/0x65c
[C00000001FE87E30] [C000000000004CFC] handle_page_fault+0x20/0x58
	:

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - 

Signed-off-by: Tsutomu Owa <tsutomu.owa@toshiba.co.jp>
-- owa

diff -rup linux-rt8/include/asm-powerpc/rtas.h rt/include/asm-powerpc/rtas.h
--- linux-rt8/include/asm-powerpc/rtas.h	2007-02-20 09:37:27.000000000 +0900
+++ rt/include/asm-powerpc/rtas.h	2007-03-05 11:49:41.000000000 +0900
@@ -58,7 +58,7 @@ struct rtas_t {
 	unsigned long entry;		/* physical address pointer */
 	unsigned long base;		/* physical address pointer */
 	unsigned long size;
-	spinlock_t lock;
+	raw_spinlock_t lock;
 	struct rtas_args args;
 	struct device_node *dev;	/* virtual address pointer */
 };
diff -rup linux-rt8/arch/powerpc/kernel/rtas.c rt/arch/powerpc/kernel/rtas.c
--- linux-rt8/arch/powerpc/kernel/rtas.c	2007-02-20 09:38:52.000000000 +0900
+++ rt/arch/powerpc/kernel/rtas.c	2007-03-05 11:50:58.000000000 +0900
@@ -36,7 +36,7 @@
 #include <asm/syscalls.h>
 
 struct rtas_t rtas = {
-	.lock = SPIN_LOCK_UNLOCKED
+	.lock = RAW_SPIN_LOCK_UNLOCKED(lock)
 };
 EXPORT_SYMBOL(rtas);
 
diff -rup linux-rt8/arch/powerpc/mm/hash_native_64.c rt/arch/powerpc/mm/hash_native_64.c
--- linux-rt8/arch/powerpc/mm/hash_native_64.c	2007-02-20 09:38:58.000000000 +0900
+++ rt/arch/powerpc/mm/hash_native_64.c	2007-03-05 15:40:12.000000000 +0900
@@ -35,7 +35,7 @@
 
 #define HPTE_LOCK_BIT 3
 
-static DEFINE_SPINLOCK(native_tlbie_lock);
+static DEFINE_RAW_SPINLOCK(native_tlbie_lock);
 
 static inline void __tlbie(unsigned long va, unsigned int psize)
 {
--- linux-rt8/arch/powerpc/kernel/irq.c	2007-02-20 14:30:38.000000000 +0900
+++ rt/arch/powerpc/kernel/irq.c	2007-03-05 18:54:34.000000000 +0900
@@ -392,7 +392,7 @@ EXPORT_SYMBOL(do_softirq);
 #ifdef CONFIG_PPC_MERGE
 
 static LIST_HEAD(irq_hosts);
-static spinlock_t irq_big_lock = SPIN_LOCK_UNLOCKED;
+static DEFINE_RAW_SPINLOCK(irq_big_lock);
 static DEFINE_PER_CPU(unsigned int, irq_radix_reader);
 static unsigned int irq_radix_writer;
 struct irq_map_entry irq_map[NR_IRQS];

[patch 3/6 -rt] powerpc 2.6.20-rt8: fix a runtime warning for smp_processor_id()

[Tsutomu OWA]

  To fix the following runtime warning.

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - 
BUG: using smp_processor_id() in preemptible [00000000] code: init/371
caller is .pgtable_free_tlb+0x2c/0x14c
Call Trace:
[C00000000FF6B770] [C00000000000FAAC] .show_stack+0x68/0x1b0 (unreliable)
[C00000000FF6B810] [C0000000001F7190] .debug_smp_processor_id+0xc8/0xf8
[C00000000FF6B8A0] [C00000000002C52C] .pgtable_free_tlb+0x2c/0x14c
[C00000000FF6B940] [C0000000000B6528] .free_pgd_range+0x234/0x3bc
[C00000000FF6BA40] [C0000000000B6AB8] .free_pgtables+0x224/0x260
[C00000000FF6BB00] [C0000000000B7FE8] .exit_mmap+0x100/0x208
[C00000000FF6BBC0] [C000000000055FB0] .mmput+0x70/0x12c
[C00000000FF6BC50] [C00000000005B728] .exit_mm+0x150/0x170
[C00000000FF6BCE0] [C00000000005D80C] .do_exit+0x28c/0x9bc
[C00000000FF6BDA0] [C00000000005DFF0] .sys_exit_group+0x0/0x8
[C00000000FF6BE30] [C000000000008634] syscall_exit+0x0/0x40
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - 

  Would it be better to just use raw_smp_processor_id() rather than tlb->cpu?

Signed-off-by: Tsutomu Owa <tsutomu.owa@toshiba.co.jp>
-- owa

diff -rup linux-rt8/arch/powerpc/mm/tlb_64.c rt/arch/powerpc/mm/tlb_64.c
--- linux-rt8/arch/powerpc/mm/tlb_64.c	2007-02-20 14:30:38.000000000 +0900
+++ rt/arch/powerpc/mm/tlb_64.c	2007-03-05 13:31:24.000000000 +0900
@@ -94,8 +94,11 @@ static void pte_free_submit(struct pte_f
 
 void pgtable_free_tlb(struct mmu_gather *tlb, pgtable_free_t pgf)
 {
-	/* This is safe since tlb_gather_mmu has disabled preemption */
-        cpumask_t local_cpumask = cpumask_of_cpu(smp_processor_id());
+	/*
+	 * This is safe since tlb_gather_mmu has disabled preemption.
+	 * tlb->cpu is set by tlb_gather_mmu as well.
+	 */
+        cpumask_t local_cpumask = cpumask_of_cpu(tlb->cpu);
 	struct pte_freelist_batch **batchp = &__get_cpu_var(pte_freelist_cur);
 
 	if (atomic_read(&tlb->mm->mm_users) < 2 ||

[RFC] [patch 4/6 -rt] powerpc 2.6.20-rt8: fix a runtime warnings for xmon

[Tsutomu OWA]

  To fix the following runtime warnings when entering xmon.

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - 
Entering xmon
BUG: using smp_processor_id() in preemptible [00000000] code: khvcd/280
caller is .xmon_core+0xb8/0x8ec
Call Trace:
[C00000000FD737C0] [C00000000000FAAC] .show_stack+0x68/0x1b0 (unreliable)
[C00000000FD73860] [C0000000001F71F0] .debug_smp_processor_id+0xc8/0xf8
[C00000000FD738F0] [C00000000004AF30] .xmon_core+0xb8/0x8ec
[C00000000FD73A80] [C00000000004B918] .xmon+0x38/0x4c
[C00000000FD73C60] [C00000000004BA8C] .sysrq_handle_xmon+0x48/0x5c
[C00000000FD73CD0] [C000000000243A68] .__handle_sysrq+0xe0/0x1b0
[C00000000FD73D70] [C000000000244974] .hvc_poll+0x18c/0x2b4
[C00000000FD73E50] [C000000000244E80] .khvcd+0x88/0x164
[C00000000FD73EE0] [C000000000075014] .kthread+0x124/0x174
[C00000000FD73F90] [C000000000023D48] .kernel_thread+0x4c/0x68
BUG: khvcd:280 task might have lost a preemption check!
Call Trace:
[C00000000FD73740] [C00000000000FAAC] .show_stack+0x68/0x1b0 (unreliable)
[C00000000FD737E0] [C000000000054920] .preempt_enable_no_resched+0x64/0x7c
[C00000000FD73860] [C0000000001F71F8] .debug_smp_processor_id+0xd0/0xf8
[C00000000FD738F0] [C00000000004AF30] .xmon_core+0xb8/0x8ec
[C00000000FD73A80] [C00000000004B918] .xmon+0x38/0x4c
[C00000000FD73C60] [C00000000004BA8C] .sysrq_handle_xmon+0x48/0x5c
[C00000000FD73CD0] [C000000000243A68] .__handle_sysrq+0xe0/0x1b0
[C00000000FD73D70] [C000000000244974] .hvc_poll+0x18c/0x2b4
[C00000000FD73E50] [C000000000244E80] .khvcd+0x88/0x164
[C00000000FD73EE0] [C000000000075014] .kthread+0x124/0x174
[C00000000FD73F90] [C000000000023D48] .kernel_thread+0x4c/0x68
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - 

  I'd appreciate if experts on xmon would give any comments/suggestions
on this.  These warnings was just found yesterday, this patch seems to
eliminate the warnings but I have not looked into it seriously.

Signed-off-by: Tsutomu Owa <tsutomu.owa@toshiba.co.jp>
-- owa

diff -rup linux-rt8/arch/powerpc/xmon/xmon.c rt/arch/powerpc/xmon/xmon.c
--- linux-rt8/arch/powerpc/xmon/xmon.c	2007-02-20 09:38:52.000000000 +0900
+++ rt/arch/powerpc/xmon/xmon.c	2007-03-05 15:29:46.000000000 +0900
@@ -342,6 +342,7 @@ static int xmon_core(struct pt_regs *reg
 
 	msr = mfmsr();
 	mtmsr(msr & ~MSR_EE);	/* disable interrupts */
+	preempt_disable();
 
 	bp = in_breakpoint_table(regs->nip, &offset);
 	if (bp != NULL) {
@@ -516,6 +517,7 @@ static int xmon_core(struct pt_regs *reg
 
 	insert_cpu_bpts();
 
+	preempt_enable();
 	mtmsr(msr);		/* restore interrupt enable */
 
 	return cmd != 'X' && cmd != EOF;

[RFC] [patch 5/6] powerpc 2.6.20-rt8: fix a boot error for handle_percpu_irq

[Tsutomu OWA]

  To fix the following boot time error by removing ack member added by
the rt patch.

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
Processor 1 found.
Brought up 2 CPUs
------------[ cut here ]------------
kernel BUG at arch/powerpc/platforms/cell/interrupt.c:86!
pu 0x1: Vector: 700 (Program Check) at [c00000000fff3c80]
    pc: c000000000033f9c: .iic_eoi+0x58/0x64
    lr: c00000000009add8: .handle_percpu_irq+0xd4/0xf4
    sp: c00000000fff3f00
   msr: 9000000000021032
  current = 0xc000000000fee040
  paca    = 0xc000000000509e80
    pid   = 0, comm = swapper
kernel BUG at arch/powerpc/platforms/cell/interrupt.c:86!
enter ? for help
[link register   ] c00000000009add8 .handle_percpu_irq+0xd4/0xf4
[c00000000fff3f00] c00000000009ada8 .handle_percpu_irq+0xa4/0xf4 (unreliable)
[c00000000fff3f90] c000000000023bb8 .call_handle_irq+0x1c/0x2c
[c000000000ff7950] c00000000000c910 .do_IRQ+0xf8/0x1b8
[c000000000ff79f0] c000000000034f34 .cbe_system_reset_exception+0x74/0xb4
[c000000000ff7a70] c000000000022610 .system_reset_exception+0x40/0xe0
[c000000000ff7af0] c000000000003378 system_reset_common+0xf8/0x100
--- Exception: 100 (System Reset) at c000000000035008 .cbe_power_save+0x94/0xb0
[c000000000ff7e70] c000000000012030 .cpu_idle+0xc8/0x144
[c000000000ff7f00] c000000000026894 .start_secondary+0x150/0x174
[c000000000ff7f90] c000000000008364 .start_secondary_prolog+0xc/0x10
1:mon>

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - 
  I found a pile of e-mail started by Sergei Shtylyov on linuxppc-dev regarding this.
    Subject: [PATCH] 2.6.18-rt7: PowerPC: fix breakage in threaded fasteoi type IRQ handlers
    From: Sergei Shtylyov <sshtylyov@ru.mvista.com>
    Date: Sun, 19 Nov 2006 22:43:34 +0300

  Though I don't quite get the conclusion, the code does not work at least
on celleb when handle_percpu_irq is applied.  Since the handle_percpu_irq calls
both .ask and .eoi and when ask is set to iic_eoi, then iic_eoi() is called twice
for one interrupt.  It hits BUG_ON(iic->eoi_ptr < 0)!

  Anthor workaround could be to add one more irq_chip structure for handle_percpu_irq
which does not have ack member...

  Any comments?

Signed-off-by: Tsutomu Owa <tsutomu.owa@toshiba.co.jp>
-- owa

diff -rup linux-rt8/arch/powerpc/platforms/cell/interrupt.c rt/arch/powerpc/platforms/cell/interrupt.c
--- linux-rt8/arch/powerpc/platforms/cell/interrupt.c	2007-02-20 14:30:38.000000000 +0900
+++ rt/arch/powerpc/platforms/cell/interrupt.c	2007-03-02 18:48:52.000000000 +0900
@@ -90,7 +90,6 @@ static struct irq_chip iic_chip = {
 	.typename = " CELL-IIC ",
 	.mask = iic_mask,
 	.unmask = iic_unmask,
-	.ack = iic_eoi,
 	.eoi = iic_eoi,
 };
 

[patch 6/6 -rt] powerpc 2.6.20-rt8: fix boot/runtime errors/warnings

[Tsutomu OWA]

  To fix the following boot time warnings by setting soft_enabled and
hard_enabled.

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - 
Freeing unused kernel memory: 248k freed
BUG: scheduling with irqs disabled: rc.sysinit/0x00000000/373
caller is user_work+0x14/0x2c
Call Trace:
[C00000001FEC3D10] [C00000000000FAA0] .show_stack+0x68/0x1b0 (unreliable)
[C00000001FEC3DB0] [C0000000003E78DC] .schedule+0x78/0x128
[C00000001FEC3E30] [C000000000008C40] user_work+0x14/0x2c
BUG: scheduling with irqs disabled: sed/0x00000000/378
caller is user_work+0x14/0x2c
Call Trace:
[C00000000FA33D10] [C00000000000FAA0] .show_stack+0x68/0x1b0 (unreliable)
[C00000000FA33DB0] [C0000000003E78DC] .schedule+0x78/0x128
[C00000000FA33E30] [C000000000008C40] user_work+0x14/0x2c

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - 

Signed-off-by: Tsutomu Owa <tsutomu.owa@toshiba.co.jp>
-- owa

--- rt/arch/powerpc/kernel/entry_64.S	2007-03-05 18:21:39.000000000 +0900
+++ rt.new/arch/powerpc/kernel/entry_64.S	2007-03-05 18:23:18.000000000 +0900
@@ -572,6 +572,11 @@ do_work:
 
 user_work:
 #endif
+	/* here we are preempting the current task */
+	li	r0,1
+	stb	r0,PACASOFTIRQEN(r13)
+	stb	r0,PACAHARDIRQEN(r13)
+
 	/* Enable interrupts */
 	ori	r10,r10,MSR_EE
 	mtmsrd	r10,1

Re: [patch 0/6 -rt] powerpc 2.6.20-rt8: fix boot/runtime errors/warnings for PowerPC(ppc64)

[Ingo Molnar]

* Tsutomu OWA <tsutomu.owa@toshiba.co.jp> wrote:

> 
> Hi Ingo,
> 
>   Please consider for inclusion in your rt tree.
> 
>   This series of patches fixes boot and runntime errors/warnings for
> powerpc (esp. 64 bit).  This applies to linux-2.6.20, patch-2.6.20-rt8
> and previous my patch set;
>   http://ozlabs.org/pipermail/linuxppc-dev/2007-March/032640.html
>   http://lkml.org/lkml/2007/3/6/503

thanks, applied.

	Ingo

Re: [RFC] [patch 4/6 -rt] powerpc 2.6.20-rt8: fix a runtime warnings for xmon

[Ingo Molnar]

* Tsutomu OWA <tsutomu.owa@toshiba.co.jp> wrote:

> @@ -342,6 +342,7 @@ static int xmon_core(struct pt_regs *reg
>  
>  	msr = mfmsr();
>  	mtmsr(msr & ~MSR_EE);	/* disable interrupts */
> +	preempt_disable();

i'm not an xmon expert, but maybe it might make more sense to first 
disable preemption, then interrupts - otherwise you could be preempted 
right after having disabled these interrupts (and be scheduled to 
another CPU, etc.). What is the difference between local_irq_save() and 
the above 'disable interrupts' sequence? If it's not the same and 
xmon_core() relied on having hardirqs disabled then it might make sense 
to do a local_irq_save() there, instead of a preempt_disable().

	Ingo

Re: [RFC] [patch 4/6 -rt] powerpc 2.6.20-rt8: fix a runtime warnings for xmon

[Benjamin Herrenschmidt]

On Wed, 2007-03-07 at 10:16 +0100, Ingo Molnar wrote:
> * Tsutomu OWA <tsutomu.owa@toshiba.co.jp> wrote:
> 
> > @@ -342,6 +342,7 @@ static int xmon_core(struct pt_regs *reg
> >  
> >  	msr = mfmsr();
> >  	mtmsr(msr & ~MSR_EE);	/* disable interrupts */
> > +	preempt_disable();
> 
> i'm not an xmon expert, but maybe it might make more sense to first 
> disable preemption, then interrupts - otherwise you could be preempted 
> right after having disabled these interrupts (and be scheduled to 
> another CPU, etc.). What is the difference between local_irq_save() and 
> the above 'disable interrupts' sequence? If it's not the same and 
> xmon_core() relied on having hardirqs disabled then it might make sense 
> to do a local_irq_save() there, instead of a preempt_disable().

powerpc 64 bits nowadays does lazy HW masking, so local_irq_disable()
will not actually switch MSR_EE off. However, xmon needs that to happen
(though we have a nicer accessor to do it, I suspect some bitrot need
fixing in there, possibly already fixed in .21)

I agree that preempt_disable() should be put before the MSR tweaking
though.

Ben.

Re: [RFC] [patch 4/6 -rt] powerpc 2.6.20-rt8: fix a runtime warnings for xmon

[Tsutomu OWA]

At Wed, 07 Mar 2007 11:10:59 +0100,
Benjamin Herrenschmidt wrote:
> 
> On Wed, 2007-03-07 at 10:16 +0100, Ingo Molnar wrote:
> > * Tsutomu OWA <tsutomu.owa@toshiba.co.jp> wrote:
> > 
> > > @@ -342,6 +342,7 @@ static int xmon_core(struct pt_regs *reg
> > >  
> > >  	msr = mfmsr();
> > >  	mtmsr(msr & ~MSR_EE);	/* disable interrupts */
> > > +	preempt_disable();
> > 
> > i'm not an xmon expert, but maybe it might make more sense to first 
> > disable preemption, then interrupts - otherwise you could be preempted 
> > right after having disabled these interrupts (and be scheduled to 
> > another CPU, etc.). What is the difference between local_irq_save() and 
> > the above 'disable interrupts' sequence? If it's not the same and 
> > xmon_core() relied on having hardirqs disabled then it might make sense 
> > to do a local_irq_save() there, instead of a preempt_disable().
> 
> powerpc 64 bits nowadays does lazy HW masking, so local_irq_disable()
> will not actually switch MSR_EE off. However, xmon needs that to happen
> (though we have a nicer accessor to do it, I suspect some bitrot need
> fixing in there, possibly already fixed in .21)
> 
> I agree that preempt_disable() should be put before the MSR tweaking
> though.

  As all of you said, I'm resending the patch here.  

  To fix the following runtime warnings when entering xmon.

- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - 
Entering xmon
BUG: using smp_processor_id() in preemptible [00000000] code: khvcd/280
caller is .xmon_core+0xb8/0x8ec
Call Trace:
[C00000000FD737C0] [C00000000000FAAC] .show_stack+0x68/0x1b0 (unreliable)
[C00000000FD73860] [C0000000001F71F0] .debug_smp_processor_id+0xc8/0xf8
[C00000000FD738F0] [C00000000004AF30] .xmon_core+0xb8/0x8ec
[C00000000FD73A80] [C00000000004B918] .xmon+0x38/0x4c
[C00000000FD73C60] [C00000000004BA8C] .sysrq_handle_xmon+0x48/0x5c
[C00000000FD73CD0] [C000000000243A68] .__handle_sysrq+0xe0/0x1b0
[C00000000FD73D70] [C000000000244974] .hvc_poll+0x18c/0x2b4
[C00000000FD73E50] [C000000000244E80] .khvcd+0x88/0x164
[C00000000FD73EE0] [C000000000075014] .kthread+0x124/0x174
[C00000000FD73F90] [C000000000023D48] .kernel_thread+0x4c/0x68
BUG: khvcd:280 task might have lost a preemption check!
Call Trace:
[C00000000FD73740] [C00000000000FAAC] .show_stack+0x68/0x1b0 (unreliable)
[C00000000FD737E0] [C000000000054920] .preempt_enable_no_resched+0x64/0x7c
[C00000000FD73860] [C0000000001F71F8] .debug_smp_processor_id+0xd0/0xf8
[C00000000FD738F0] [C00000000004AF30] .xmon_core+0xb8/0x8ec
[C00000000FD73A80] [C00000000004B918] .xmon+0x38/0x4c
[C00000000FD73C60] [C00000000004BA8C] .sysrq_handle_xmon+0x48/0x5c
[C00000000FD73CD0] [C000000000243A68] .__handle_sysrq+0xe0/0x1b0
[C00000000FD73D70] [C000000000244974] .hvc_poll+0x18c/0x2b4
[C00000000FD73E50] [C000000000244E80] .khvcd+0x88/0x164
[C00000000FD73EE0] [C000000000075014] .kthread+0x124/0x174
[C00000000FD73F90] [C000000000023D48] .kernel_thread+0x4c/0x68
- - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - 

thanks a lot!

Signed-off-by: Tsutomu Owa <tsutomu.owa@toshiba.co.jp>
-- owa

--- linux-rt8/arch/powerpc/xmon/xmon.c	2007-02-20 09:38:52.000000000 +0900
+++ rt/arch/powerpc/xmon/xmon.c	2007-03-07 19:49:38.000000000 +0900
@@ -340,6 +340,7 @@ static int xmon_core(struct pt_regs *reg
 	unsigned long timeout;
 #endif
 
+	preempt_disable();
 	msr = mfmsr();
 	mtmsr(msr & ~MSR_EE);	/* disable interrupts */
 
@@ -517,6 +518,7 @@ static int xmon_core(struct pt_regs *reg
 	insert_cpu_bpts();
 
 	mtmsr(msr);		/* restore interrupt enable */
+	preempt_enable();
 
 	return cmd != 'X' && cmd != EOF;
 }

Re: [RFC] [patch 4/6 -rt] powerpc 2.6.20-rt8: fix a runtime warnings for xmon

[Arnd Bergmann]

On Wednesday 07 March 2007, Ingo Molnar wrote:
> i'm not an xmon expert, but maybe it might make more sense to first 
> disable preemption, then interrupts - otherwise you could be preempted 
> right after having disabled these interrupts (and be scheduled to 
> another CPU, etc.). What is the difference between local_irq_save() and 
> the above 'disable interrupts' sequence? If it's not the same and 
> xmon_core() relied on having hardirqs disabled then it might make sense 
> to do a local_irq_save() there, instead of a preempt_disable().

Since relatively recently, powerpc does no longer actually disable
the hardware interrupts with local_irq_disable(), but rather sets
a per-cpu flag that will be checked if an actual interrupt comes
in as part of the critical section.

The mtmsr() sequence in xmon corresponds to hard_irq_disable()
and should probably changed to that, but then you still need
the extra preempt_disable() / preempt_enable().

I think you're right about the sequence having to be
1. preempt_disable()
2. hard_irq_disable()
3.
4. hard_irq_enable()
5. preempt_enable()

	Arnd <><

Re: [patch 0/6 -rt] powerpc 2.6.20-rt8: fix boot/runtime errors/warnings for PowerPC(ppc64)

[Sergei Shtylyov]

Tsutomu OWA wrote:
> Hi Ingo,

>   Please consider for inclusion in your rt tree.

>   This series of patches fixes boot and runntime errors/warnings for
> powerpc (esp. 64 bit).  This applies to linux-2.6.20, patch-2.6.20-rt8
> and previous my patch set;
>   http://ozlabs.org/pipermail/linuxppc-dev/2007-March/032640.html
>   http://lkml.org/lkml/2007/3/6/503

>   Compile and boot tested on celleb (Cell Reference set) for both
> PREEMPT_RT=y and PREEMPT_NONE=y.

>   CONFIG_MCOUNT, CONFIG_LATENCY_TRACE and other tracing options nor
> CONFIG_GENERIC_TIME,

    There is PowerPC genTOD patch and it's incorporated into -rt (don't know 
it works for Cell) but it breaks TOD vsyscalls. Several months ago I've posted 
patches removing them for the time being:

http://ozlabs.org/pipermail/linuxppc-dev/2006-November/027686.html

and I've heard that this issue is being worked on now by John Stultz.
    Here's also a patch implemeting read_persitent_clock():

http://www.tglx.de/projects/hrtimers/2.6.18/broken-out/gtod-persistent-clock-support-ppc.patch

> clockevents etc are not yet ported.

    Note that there *is* PowerPC clockevents driver already (don't know if it 
works for Cell) -- it just never got merged to -rt:

http://ozlabs.org/pipermail/linuxppc-dev/2006-November/027794.html
http://ozlabs.org/pipermail/linuxppc-dev/2006-November/027852.html

>   Comments and suggestions are welcome.

>   Thanks in advance.
> -- owa
> TOSHIBA, Software Engineering Center.

Re: [patch 2/6 -rt] powerpc 2.6.20-rt8: to convert spinlocks to raw ones.

[Sergei Shtylyov]

Hello.

Tsutomu OWA wrote:

> --- linux-rt8/arch/powerpc/kernel/irq.c	2007-02-20 14:30:38.000000000 +0900
> +++ rt/arch/powerpc/kernel/irq.c	2007-03-05 18:54:34.000000000 +0900
> @@ -392,7 +392,7 @@ EXPORT_SYMBOL(do_softirq);
>  #ifdef CONFIG_PPC_MERGE
>  
>  static LIST_HEAD(irq_hosts);
> -static spinlock_t irq_big_lock = SPIN_LOCK_UNLOCKED;
> +static DEFINE_RAW_SPINLOCK(irq_big_lock);
>  static DEFINE_PER_CPU(unsigned int, irq_radix_reader);
>  static unsigned int irq_radix_writer;
>  struct irq_map_entry irq_map[NR_IRQS];

    I've already sent a patch fixing this one (along with many others) a month 
ago:

http://ozlabs.org/pipermail/linuxppc-dev/2007-February/031164.html

    I wonder iof it was ever considered... :-/

WBR, Sergei

Re: [patch 2/6 -rt] powerpc 2.6.20-rt8: to convert spinlocks to raw ones.

[Benjamin Herrenschmidt]

On Wed, 2007-03-07 at 17:38 +0300, Sergei Shtylyov wrote:
> Hello.
> 
> Tsutomu OWA wrote:
> 
> > --- linux-rt8/arch/powerpc/kernel/irq.c	2007-02-20 14:30:38.000000000 +0900
> > +++ rt/arch/powerpc/kernel/irq.c	2007-03-05 18:54:34.000000000 +0900
> > @@ -392,7 +392,7 @@ EXPORT_SYMBOL(do_softirq);
> >  #ifdef CONFIG_PPC_MERGE
> >  
> >  static LIST_HEAD(irq_hosts);
> > -static spinlock_t irq_big_lock = SPIN_LOCK_UNLOCKED;
> > +static DEFINE_RAW_SPINLOCK(irq_big_lock);
> >  static DEFINE_PER_CPU(unsigned int, irq_radix_reader);
> >  static unsigned int irq_radix_writer;
> >  struct irq_map_entry irq_map[NR_IRQS];
> 
>     I've already sent a patch fixing this one (along with many others) a month 
> ago:
> 
> http://ozlabs.org/pipermail/linuxppc-dev/2007-February/031164.html
> 
>     I wonder iof it was ever considered... :-/

It was but not for 2.6.21 timeframe due to people lack of time/bandwidth
among others :-)

I intend to try to spend some time before the 2.6.22 merge window to
gather those -rt related patches that can be merged already and push
them to powerpc.git.

(Note to mingo: that means that we might end up with patches both in
your tree and being push via powerpc, I hope that's not too much of a
problem).

I can't promise I'll have time to do much, but I'd like to do it, so
find me on irc every now and then to "poke" if you don't see anything
happening...

Cheers,
Ben.

Re: [patch 2/6 -rt] powerpc 2.6.20-rt8: to convert spinlocks to raw ones.

[Sergei Shtylyov]

Hello.

Benjamin Herrenschmidt wrote:

>>>--- linux-rt8/arch/powerpc/kernel/irq.c	2007-02-20 14:30:38.000000000 +0900
>>>+++ rt/arch/powerpc/kernel/irq.c	2007-03-05 18:54:34.000000000 +0900
>>>@@ -392,7 +392,7 @@ EXPORT_SYMBOL(do_softirq);
>>> #ifdef CONFIG_PPC_MERGE
>>> 
>>> static LIST_HEAD(irq_hosts);
>>>-static spinlock_t irq_big_lock = SPIN_LOCK_UNLOCKED;
>>>+static DEFINE_RAW_SPINLOCK(irq_big_lock);
>>> static DEFINE_PER_CPU(unsigned int, irq_radix_reader);
>>> static unsigned int irq_radix_writer;
>>> struct irq_map_entry irq_map[NR_IRQS];

>>    I've already sent a patch fixing this one (along with many others) a month 
>>ago:

>>http://ozlabs.org/pipermail/linuxppc-dev/2007-February/031164.html

>>    I wonder iof it was ever considered... :-/

> It was but not for 2.6.21 timeframe due to people lack of time/bandwidth
> among others :-)

    I was asking Ingo, basically. :-)

> I intend to try to spend some time before the 2.6.22 merge window to
> gather those -rt related patches that can be merged already and push
> them to powerpc.git.

> (Note to mingo: that means that we might end up with patches both in
> your tree and being push via powerpc, I hope that's not too much of a
> problem).

    Well, this is happening every -rc1 now -- part of -rt getting merged to 
mainline.

> I can't promise I'll have time to do much, but I'd like to do it, so
> find me on irc every now and then to "poke" if you don't see anything
> happening...

    My purpose was only to get this into -rt patch for now.

> Cheers,
> Ben.

WBR, Sergei

Re: [patch 2/6 -rt] powerpc 2.6.20-rt8: to convert spinlocks to raw ones.

[Paul Mackerras]

Sergei Shtylyov writes:

>     I've already sent a patch fixing this one (along with many others) a month 
> ago:
> 
> http://ozlabs.org/pipermail/linuxppc-dev/2007-February/031164.html
> 
>     I wonder iof it was ever considered... :-/

The entire patch description was just this:

> Convert the spinlocks in the PowerPC interrupt related code into the raw ones,
> also convert the PURR and PMC related spinlocks...

which says what you did, but gives NO CLUE about why this might be a
good thing to do - what problem it fixes or what desirable outcome it
produces.  I will not apply patches with inadequate descriptions.

Paul.

Re: [patch 2/6 -rt] powerpc 2.6.20-rt8: to convert spinlocks to raw ones.

[Sergei Shtylyov]

Hello.

Paul Mackerras wrote:

>>    I've already sent a patch fixing this one (along with many others) a month 
>>ago:

>>http://ozlabs.org/pipermail/linuxppc-dev/2007-February/031164.html

>>    I wonder iof it was ever considered... :-/

> The entire patch description was just this:

>>Convert the spinlocks in the PowerPC interrupt related code into the raw ones,
>>also convert the PURR and PMC related spinlocks...

> which says what you did, but gives NO CLUE about why this might be a
> good thing to do - what problem it fixes or what desirable outcome it
> produces.  I will not apply patches with inadequate descriptions.

    As I said, this was intended for the -rt patch, hence the question was for 
Ingo. I CC'ed the list just to keep people here in a loop.

> Paul.

WBR, Sergei

Re: [patch 2/6 -rt] powerpc 2.6.20-rt8: to convert spinlocks to raw ones.

[Paul Mackerras]

Sergei Shtylyov writes:

>     As I said, this was intended for the -rt patch, hence the question was for 
> Ingo. I CC'ed the list just to keep people here in a loop.

OK, fair enough, but I still think the patch description was
inadequate.  In the -rt context, I would at least expect to see some
explanation as to why those particular locks needed to be converted.

Paul.

Re: [patch 2/6 -rt] powerpc 2.6.20-rt8: to convert spinlocks to raw ones.

[Sergei Shtylyov]

Hello.

Paul Mackerras wrote:

>>    As I said, this was intended for the -rt patch, hence the question was for 
>>Ingo. I CC'ed the list just to keep people here in a loop.

> OK, fair enough, but I still think the patch description was
> inadequate.  In the -rt context, I would at least expect to see some
> explanation as to why those particular locks needed to be converted.

    I've floowed up to my patch with such explanation. In the context of an-rt 
patch itself, it was just too clear, hence I didn't go into explanations in 
the patch itself. :-)

> Paul.

WBR, Sergei

Re: [RFC] [patch 5/6] powerpc 2.6.20-rt8: fix a boot error for handle_percpu_irq

[Sergei Shtylyov]

Hello.

Tsutomu OWA wrote:
>   To fix the following boot time error by removing ack member added by
> the rt patch.

> - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
> Processor 1 found.
> Brought up 2 CPUs
> ------------[ cut here ]------------
> kernel BUG at arch/powerpc/platforms/cell/interrupt.c:86!
> pu 0x1: Vector: 700 (Program Check) at [c00000000fff3c80]
>     pc: c000000000033f9c: .iic_eoi+0x58/0x64
>     lr: c00000000009add8: .handle_percpu_irq+0xd4/0xf4
>     sp: c00000000fff3f00
>    msr: 9000000000021032
>   current = 0xc000000000fee040
>   paca    = 0xc000000000509e80
>     pid   = 0, comm = swapper
> kernel BUG at arch/powerpc/platforms/cell/interrupt.c:86!
> enter ? for help
> [link register   ] c00000000009add8 .handle_percpu_irq+0xd4/0xf4
> [c00000000fff3f00] c00000000009ada8 .handle_percpu_irq+0xa4/0xf4 (unreliable)
> [c00000000fff3f90] c000000000023bb8 .call_handle_irq+0x1c/0x2c
> [c000000000ff7950] c00000000000c910 .do_IRQ+0xf8/0x1b8
> [c000000000ff79f0] c000000000034f34 .cbe_system_reset_exception+0x74/0xb4
> [c000000000ff7a70] c000000000022610 .system_reset_exception+0x40/0xe0
> [c000000000ff7af0] c000000000003378 system_reset_common+0xf8/0x100
> --- Exception: 100 (System Reset) at c000000000035008 .cbe_power_save+0x94/0xb0
> [c000000000ff7e70] c000000000012030 .cpu_idle+0xc8/0x144
> [c000000000ff7f00] c000000000026894 .start_secondary+0x150/0x174
> [c000000000ff7f90] c000000000008364 .start_secondary_prolog+0xc/0x10
> 1:mon>
> 
> - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - 
>   I found a pile of e-mail started by Sergei Shtylyov on linuxppc-dev regarding this.
>     Subject: [PATCH] 2.6.18-rt7: PowerPC: fix breakage in threaded fasteoi type IRQ handlers
>     From: Sergei Shtylyov <sshtylyov@ru.mvista.com>
>     Date: Sun, 19 Nov 2006 22:43:34 +0300

>   Though I don't quite get the conclusion, the code does not work at least
> on celleb when handle_percpu_irq is applied.  Since the handle_percpu_irq calls

    Hmmm, I was under impression that I was fixing fasteoi flow case... Sorry 
if it broke something but it really shouldn't even have been there at all. :-/

> both .ask and .eoi and when ask is set to iic_eoi, then iic_eoi() is called twice
> for one interrupt.  It hits BUG_ON(iic->eoi_ptr < 0)!

>   Anthor workaround could be to add one more irq_chip structure for handle_percpu_irq
> which does not have ack member...

>   Any comments?

    Well, I've told Ingo long ago that he shouldn't add that to the -rt patch 
(it's been refused from the very start but then got "restored" along with 
previously dropped genTOD patches).

> Signed-off-by: Tsutomu Owa <tsutomu.owa@toshiba.co.jp>
> -- owa
> 
> diff -rup linux-rt8/arch/powerpc/platforms/cell/interrupt.c rt/arch/powerpc/platforms/cell/interrupt.c
> --- linux-rt8/arch/powerpc/platforms/cell/interrupt.c	2007-02-20 14:30:38.000000000 +0900
> +++ rt/arch/powerpc/platforms/cell/interrupt.c	2007-03-02 18:48:52.000000000 +0900
> @@ -90,7 +90,6 @@ static struct irq_chip iic_chip = {
>  	.typename = " CELL-IIC ",
>  	.mask = iic_mask,
>  	.unmask = iic_unmask,
> -	.ack = iic_eoi,
>  	.eoi = iic_eoi,
>  };

WBR, Sergei

Re: [patch 2/6 -rt] powerpc 2.6.20-rt8: to convert spinlocks to raw ones.

[Paul Mackerras]

Sergei Shtylyov writes:

>     I've floowed up to my patch with such explanation. In the context of an-rt 
> patch itself, it was just too clear, hence I didn't go into explanations in 
> the patch itself. :-)

Well, it might be clear, to you, now, with the context in your head.
But if such a patch is to go into a git tree, and somebody comes along
in 3 years time and wants to know exactly why you made that change
(and maybe that somebody is you :), then they will need more detail -
such as how you came to the conclusion that those locks and no others
needed to be changed, for instance.

At least give some of the reasoning behind your choice of which locks
to convert, so that in future, if the patch turns out to have
introduced a bug somehow, the person debugging it can either identify
that there was a flaw in your logic, or else understand something that
you have seen that they missed.

Paul.

Re: [patch 2/6 -rt] powerpc 2.6.20-rt8: to convert spinlocks to raw ones.

[Bill Huey]

On Thu, Mar 08, 2007 at 08:30:43AM +1100, Paul Mackerras wrote:
> Sergei Shtylyov writes:
> 
> >     I've floowed up to my patch with such explanation. In the context of an-rt 
> > patch itself, it was just too clear, hence I didn't go into explanations in 
> > the patch itself. :-)
> 
> Well, it might be clear, to you, now, with the context in your head.
> But if such a patch is to go into a git tree, and somebody comes along
> in 3 years time and wants to know exactly why you made that change
> (and maybe that somebody is you :), then they will need more detail -
> such as how you came to the conclusion that those locks and no others
> needed to be changed, for instance.
> 
> At least give some of the reasoning behind your choice of which locks
> to convert, so that in future, if the patch turns out to have
> introduced a bug somehow, the person debugging it can either identify
> that there was a flaw in your logic, or else understand something that
> you have seen that they missed.

Paul,

It has to do with how locking is done in the -rt patch itself. It's probably
before the time of general maintainers since the -rt patch hasn't been fully
merged, but I agree a document needs to be written outlining what needs to
be changed to spinlocks and what locks can be emulated with the rtmutex.c/rt.c
logic. There aren't that many people that know specifically unless they've
tried to map out chunks of the Linux kernel for this purpose in the first
place. I only know because of my own parallel effort to get the kernel to be
preemptive (the old mmLinux project that I abandoned for Ingo's stuff).

Generally, things that run within interrupt contexts need to be spinlocks.
The interrupt controller is one of those things obviously, the timer interrupt
for practical reasons such as performance and other places so that locking is
outside of direct control and scope of the scheduler. Of course the scheduler's
runqueues needs to be spinlocked for the reasons above otherwise your system
is stuck with a kind chicken and the egg problem interacting with the scheduler. 

The places that need to be reverted to raw spinlocks are generally either
acquired by function calls that allocate the spinlock at a terminal of the
kernel's lock graph or isolated from other callers completely (parts of the
timer for logic for instance). It's all about the collision of various lock
(preemptive and non-preemptive) subtrees and how to avoid scheduling within
atomic violations that lead to deadlocks. The -rt patch gets arbitrary
preemption abilities by shrinking the non-preemptive sub-tree bit to the bare
essentials of what will let a system to run yet still preserve all of
the expected locking semantics of a critical section.

Otherwise everything by default is backed by a blocking rtmutex identity
to provide for correct preemptivity behavior within critical sections. That
is why these reverts are needed to restore the mathematical correctness of
the kernel's locking structures.

I hope this is helpful.

bill

Re: [patch 0/6 -rt] powerpc 2.6.20-rt8: fix boot/runtime errors/warnings for PowerPC(ppc64)

[Tsutomu OWA]

At Wed, 07 Mar 2007 17:26:50 +0300,
Sergei Shtylyov wrote:
> 
> Tsutomu OWA wrote:

> >   CONFIG_MCOUNT, CONFIG_LATENCY_TRACE and other tracing options nor
> > CONFIG_GENERIC_TIME,
> 
>     There is PowerPC genTOD patch and it's incorporated into -rt (don't know 
> it works for Cell) but it breaks TOD vsyscalls. Several months ago I've posted 
> patches removing them for the time being:

> > clockevents etc are not yet ported.
> 
>     Note that there *is* PowerPC clockevents driver already (don't know if it 
> works for Cell) -- it just never got merged to -rt:

  I should have written like "... are not yet ported by myself."

  anyway, thanks for the info.
-- owa

Re: [patch 2/6 -rt] powerpc 2.6.20-rt8: to convert spinlocks to raw ones.

[Paul Mackerras]

Bill Huey (hui) writes:

> The places that need to be reverted to raw spinlocks are generally either
> acquired by function calls that allocate the spinlock at a terminal of the
> kernel's lock graph or isolated from other callers completely (parts of the
> timer for logic for instance). It's all about the collision of various lock
> (preemptive and non-preemptive) subtrees and how to avoid scheduling within
> atomic violations that lead to deadlocks. The -rt patch gets arbitrary
> preemption abilities by shrinking the non-preemptive sub-tree bit to the bare
> essentials of what will let a system to run yet still preserve all of
> the expected locking semantics of a critical section.

Thanks; that's an interesting explanation.

It misses the point of what I was saying to Sergei, though, which was
*not* "I don't understand your patch", it was "if this patch goes into
a git tree, someone coming along in 3 years time won't understand the
patch."  In other words I was ranting about the need for a decent
description to accompany the patch itself, so it would go into the
permanent record.

Regards,
Paul.

Re: [patch 2/6 -rt] powerpc 2.6.20-rt8: to convert spinlocks to raw ones.

[Bill Huey]

On Thu, Mar 08, 2007 at 02:26:47PM +1100, Paul Mackerras wrote:
> Bill Huey (hui) writes:
> 
> > The places that need to be reverted to raw spinlocks are generally either
> > acquired by function calls that allocate the spinlock at a terminal of the
> > kernel's lock graph or isolated from other callers completely (parts of the
> > timer for logic for instance). It's all about the collision of various lock
> > (preemptive and non-preemptive) subtrees and how to avoid scheduling within
> > atomic violations that lead to deadlocks. The -rt patch gets arbitrary
> > preemption abilities by shrinking the non-preemptive sub-tree bit to the bare
> > essentials of what will let a system to run yet still preserve all of
> > the expected locking semantics of a critical section.
> 
> Thanks; that's an interesting explanation.
> 
> It misses the point of what I was saying to Sergei, though, which was
> *not* "I don't understand your patch", it was "if this patch goes into
> a git tree, someone coming along in 3 years time won't understand the
> patch."  In other words I was ranting about the need for a decent
> description to accompany the patch itself, so it would go into the
> permanent record.

Yeah, I think it's a a fear and uncertainly about the technical details about
the patch. That is why folks CC Ingo and company to get either a kind of
confirmation that this is ok along with comments. There are very few folks
that really understand the basic principals of the patch in this community
and that's not going to change any time soon. The mystery, paranoia (FUD)
and criticism surrounding it can make folks a bit shy.

I'll talk to you and Ben about it if we all get to OLS again. :)

bill

Re: [patch 1/6 -rt] powerpc 2.6.20-rt8: add preemption checks for NEED_RESCHED_DELAYED.

[Sergei Shtylyov]

Hello.

Tsutomu OWA wrote:
>   To add preemption checks for the NEED_RESCHED_DELAYED flag.

> diff -rup linux-rt8/arch/powerpc/kernel/idle.c rt/arch/powerpc/kernel/idle.c
> --- linux-rt8/arch/powerpc/kernel/idle.c	2007-02-20 14:30:38.000000000 +0900
> +++ rt/arch/powerpc/kernel/idle.c	2007-02-20 15:43:04.000000000 +0900
> @@ -56,7 +56,8 @@ void cpu_idle(void)
>  
>  	set_thread_flag(TIF_POLLING_NRFLAG);
>  	while (1) {
> -		while (!need_resched() && !cpu_should_die()) {
> +		while (!need_resched() && !need_resched_delayed() &&
> +				!cpu_should_die()) {
>  			ppc64_runlatch_off();
>  
>  			if (ppc_md.power_save) {

    Argh, I've missed this one! :-(
    But shouldn't we also add !need_resched_delayed() to another place below?

if (ppc_md.power_save)  {
[...]
	if (!need_resched() && !cpu_should_die())

WBR, Sergei

Re: [patch 1/6 -rt] powerpc 2.6.20-rt8: add preemption checks for NEED_RESCHED_DELAYED.

[Tsutomu OWA]

Hi,

At Fri, 16 Mar 2007 22:20:27 +0300,
Sergei Shtylyov wrote:
>     Argh, I've missed this one! :-(
>     But shouldn't we also add !need_resched_delayed() to another place below?
> 
> if (ppc_md.power_save)  {
> [...]
> 	if (!need_resched() && !cpu_should_die())

  Thanks for pointing it out.  Yes, it looks like needed.  
-- owa