Subject: custom ide driver causes "Badness in smp_call_function"

From: "Bryan Althouse" <[email-masked]>
Date: Tue, 23 Aug 2005 10:07:02 -0400

Hi all,

I have added a compact flash disk onto the local bus of an rm9224 mips
processor.  We are using an FPGA as an IDE host adaptor.  Our driver works
great when the kernel is compiled for a single processor.  But if SMP
support is enabled, the kernel dies with: 
            .
Uniform Multi-Platform E-IDE driver Revision: 7.00alpha2
Ide:  Assuming 50MHz system bus speed for PIO modes; override with
idebuss=xx
Badness in smp_call_function at arch/mips/kernel/smp.c:149
Call Trace:
 [<ffffffff8010aff8>] smp_call_function+0x1f8/0x200
 [<ffffffff8032adb0>] schedule+0x950/0xa08
 [<ffffffff8010b9f8>] local_rm9k_perfcounter_irq_startup+0x0/0x68
 [<ffffffff8010baa8>] rm9k_perfcounter_irq_startup+0x48/0x68
 [<ffffffff801658b8>] probe_irq_on+0x2b8/0x2e8
 [<ffffffff80146c00>] del_singleshot_timer_sync+0x38/0x50
 [<ffffffff8028e510>] try_to_identify+0x180/0x190
 [<ffffffff80147d88>] process_timeout+0x0/0x8
 [<ffffffff8028e66c>] do_probe+0x14c/0x338
 [<ffffffff8028e57c>] do_probe+0x5c/0x338
 [<ffffffff8028ec40>] wait_hwif_ready+0x178/0x180
 [<ffffffff8028f230>] probe_hwif+0x508/0x6e8
 [<ffffffff8028eec8>] probe_hwif+0x1a0/0x6e8
 [<ffffffff8028f42c>] probe_hwif_init_with_fixup+0x1c/0xe8
 [<ffffffff8029435c>] ide_3P_init+0xb4/0x118
 [<ffffffff80294324>] ide_3P_init+0x7c/0x118
 [<ffffffff8023d100>] idr_get_new+0x18/0x50
etc. 

Does anyone know what sort of bug could cause problems with SMP, but would
work fine otherwise?  I could supply my driver code if anyone is interested.
Thanks!

Bryan

---

From: Ralf Baechle <[email-masked]>
Date: Wed, 24 Aug 2005 16:24:44 +0100

> Badness in smp_call_function at arch/mips/kernel/smp.c:149

That's just a rather drastically looking warning, btw.

> work fine otherwise?  I could supply my driver code if anyone is interested.

Your driver is probably fine.  The problem is that doing PIO may result in
cache aliases and that requires a cache flush.  Normally that's not
terribly hard to do - but in your case an SMP cacheflush is needed which
in turn requires smp_call_function to be used and that one again may
deadlock if called with interrupts disabled.  See also
include/asm-mips/mach-generic/ide.h.

So until we have a better implementation I suggest try to avoid the use
of ide_inb() etc. with interrupts disabled.

  Ralf

---

From: "Bryan Althouse" <[email-masked]>
Date: Thu, 25 Aug 2005 11:26:58 -0400

Ralf,

Thank you for your help.  
I'm doing MMIO, not PIO, but it looks like your assessment is still valid.
I've been searching for places where ide MMIO is performed with interrupts
disabled.  I got excited when I found these lines in probe_hwif() of
ide-probe.c:

   irqd = hwif->irq;
   if (irqd)
        disable_irq(hwif->irq);

I was not initializing hwif->irq in my driver, so probably the interrupts
were being disabled here, and subsequent lines were causing the SMP badness.
I added the line "hwif->irq = 0" to my driver.  Interrupts are no longer
disabled here, but still I get the SMP badness.  I'll keep looking for other
places where the interrupts might be disabled.  

Does anyone know if the mips/swarm.c driver has this problem with SMP?
Thanks!
  
Bryan

---

From: Ralf Baechle <[email-masked]>
Date: Thu, 25 Aug 2005 16:42:49 +0100

On Thu, Aug 25, 2005 at 11:26:58AM -0400, Bryan Althouse wrote:
> From:	"Bryan Althouse" <[email-masked]>
> To:	"'Ralf Baechle'" <[email-masked]>
> Cc:	<[email-masked]>
> Subject: RE: custom ide driver causes "Badness in smp_call_function"
> Date:	Thu, 25 Aug 2005 11:26:58 -0400
> Content-Type: text/plain;
> 	charset="us-ascii"
> 
> Ralf,
> 
> Thank you for your help.  

... which still is programmed io ...

> Does anyone know if the mips/swarm.c driver has this problem with SMP?

No, SB1 has sane caches.

  Ralf

---

From: "Bryan Althouse" <[email-masked]>
Date: Thu, 25 Aug 2005 17:17:48 -0400

Here are some observations...  

If I change the line "hwif->irq = 0" in my driver to "hwif->irq = 5", my SMP
kernel no longer experiences SMP badness.  Instead, I get many lines like
"hda: lost interrupt", and the drive is not usable.  If I compile the kernel
without SMP, the drive works properly as before.  I tried irq = 5 because I
noticed that /proc/interrupts indicated that ide0 was being probed at 5.
With the SMP kernel, /proc/interrupts shows a count of 0 for ide0.  My
non-SMP kernel shows a count that increments when the drive is being used
(as expected).

Bryan

---

From: Ralf Baechle <[email-masked]>
Date: Fri, 26 Aug 2005 15:10:47 +0100

> non-SMP kernel shows a count that increments when the drive is being used

Try this patch below and let me know.  I would also like to ask those
people who used to suffer from aliases with IDE PIO to try this patch.

  Ralf

Index: include/asm-mips/mach-generic/ide.h
===================================================================
RCS file: /home/cvs/linux/include/asm-mips/mach-generic/ide.h,v
retrieving revision 1.9
diff -u -r1.9 ide.h
--- include/asm-mips/mach-generic/ide.h	19 Apr 2005 12:26:59 -0000	1.9
+++ include/asm-mips/mach-generic/ide.h	26 Aug 2005 14:04:38 -0000
@@ -19,6 +19,7 @@
 #include <linux/pci.h>
 #include <linux/stddef.h>
 #include <asm/processor.h>
+#include <asm/cacheflush.h>
 
 #ifndef MAX_HWIFS
 # ifdef CONFIG_BLK_DEV_IDEPCI
@@ -105,12 +106,14 @@
 
 /* MIPS port and memory-mapped I/O string operations.  */
 
-static inline void __ide_flush_dcache_range(unsigned long addr, unsigned long size)
+static inline void __ide_flush_dcache_range(unsigned long addr,
+	unsigned long size)
 {
-	if (cpu_has_dc_aliases) {
-		unsigned long end = addr + size;
-		for (; addr < end; addr += PAGE_SIZE)
-			flush_dcache_page(virt_to_page(addr));
+	unsigned long end = addr + size;
+
+	while (addr < end) {
+		SetPageDcacheDirty(virt_to_page(addr));
+		addr += PAGE_SIZE;
 	}
 }

---

From: "Bryan Althouse" <[email-masked]>
Date: Fri, 26 Aug 2005 10:58:40 -0400

Ralf,

The patch doesn't seem to make any difference. :(

Bryan

---

From: Alan Cox <[email-masked]>
Date: Fri, 26 Aug 2005 16:47:24 +0100

> The patch doesn't seem to make any difference. :(

Assuming your hardware is sane another approach might be to force
drive->unmask = 1. That will mean that PIO mode is running with
interrupts enabled which should avoid the problem.

Add a .fixup handler to your driver (assuming you are using a recent
2.6.x) and in the handler do something like this:

+void ide_unmask_interrupts(ide_hwif_t *hwif)
+{
+       int i;
+       for (i = 0; i < 2; i++) {
+               ide_drive_t *drive = &hwif->drives[i];
+               if(drive->present)
+                       drive->unmask = 1;
+       }
+}

hopefully that will be early enough.

---

From: Ralf Baechle <[email-masked]>
Date: Fri, 26 Aug 2005 17:28:32 +0100

> The patch doesn't seem to make any difference. :(

To clarify, the patch wasn't meant to resolve your interrupt problems but
the SMP cacheflush issues only.

  Ralf

---

From: "Bryan Althouse" <[email-masked]>
Date: Fri, 26 Aug 2005 12:36:26 -0400

I still get the SMP badness when hwif->irq is set to 0 in my driver.

Bryan

-----Original Message-----
From: Ralf Baechle [mailto:[email-masked]] 
Sent: Friday, August 26, 2005 12:29 PM
To: Bryan Althouse
Cc: [email-masked]
Subject: Re: custom ide driver causes "Badness in smp_call_function"

  Ralf

---

From: Ralf Baechle <[email-masked]>
Date: Fri, 26 Aug 2005 17:41:15 +0100

> I still get the SMP badness when hwif->irq is set to 0 in my driver.

Interesting.  That implies smp_call_function is called through a different
path on your system that on mine here.  Doesn't invalidate the patch,
just means some extra debugging work for you ;-)

  Ralf

---

From: "Bryan Althouse" <[email-masked]>
Date: Fri, 26 Aug 2005 14:05:43 -0400

Alan,

Thanks for your suggestion.
I'm not sure how to write a .fixup handler.  I did some Googling, but got
nowhere.  I looked through drivers/ide to see what drive->unmask was doing.
I found this in ide-io.c:
     if (drive->unmask)
          local_irq_enable();
And this in ide-taskfile.c:
     if (!drive->unmask)
          local_irq_disable();
I modified both of these files so that execution would be as if unmask = 1.
This resulted in no change of behavior.

Bryan

---

From: "Bryan Althouse" <[email-masked]>
Date: Fri, 26 Aug 2005 20:16:09 +0200

Alan,

Bryan