Re: [PATCH, RTF/RFC] bcm43xx: Enable local IRQs while executing periodic work
On Tue, May 23, 2006 at 05:23:09PM +0200, Michael Buesch wrote: I would like to get this into wireless-2.6, but before that happens I would like to have a few regression tests by people. Please apply this and run high network traffic for 10-15 minutes. Please enable kernel spinlock lockup and recursion detection while testing. It has almost been two weeks...do we want this patch, or not? John -- John W. Linville [EMAIL PROTECTED] ___ Bcm43xx-dev mailing list Bcm43xx-dev@lists.berlios.de http://lists.berlios.de/mailman/listinfo/bcm43xx-dev
Re: [PATCH, RTF/RFC] bcm43xx: Enable local IRQs while executing periodic work
On Monday 05 June 2006 20:54, John W. Linville wrote: On Tue, May 23, 2006 at 05:23:09PM +0200, Michael Buesch wrote: I would like to get this into wireless-2.6, but before that happens I would like to have a few regression tests by people. Please apply this and run high network traffic for 10-15 minutes. Please enable kernel spinlock lockup and recursion detection while testing. It has almost been two weeks...do we want this patch, or not? The two patches [PATCH 1/2] bcm43xx: redesign locking [PATCH 2/2] bcm43xx: preemptible periodic work are the successors of it. ;) So, no, I don't want that old and broken patch any longer. But thanks that you asked. I forgot about it. -- Greetings Michael. ___ Bcm43xx-dev mailing list Bcm43xx-dev@lists.berlios.de http://lists.berlios.de/mailman/listinfo/bcm43xx-dev
Re: [PATCH, RTF/RFC] bcm43xx: Enable local IRQs while executing periodic work
Ok, I'm back with a completely rewritten version of the
preemptible periodic work patch.
For this I had to completely redesign driver locking. We have
two different locks now. One for general purpose and one
to protect against IRQ concurrency.
(Yes, the final patch will be splitted, but for now I have
a convenient all-in-one patch here)
As we don't take the IRQ lock while executing preemptible
periodic work (We don't need to, as IRQs are disabled), we
can not deadlock anymore.
I am currently stressing the patch on one of my machines and
it seems to work as expected here.
I also wrote a dscape patch and also stressed that.
(I will publish the dscape patch later, if nobody is able
to find a regression in the softmac patch ;) )
So, here we go. Preemptible Periodic Work for bcm43xx-softmac.
Please stress-test with all methods which come to your minds.
Index: wireless-dev/drivers/net/wireless/bcm43xx/bcm43xx.h
===
--- wireless-dev.orig/drivers/net/wireless/bcm43xx/bcm43xx.h2006-06-03
21:04:17.0 +0200
+++ wireless-dev/drivers/net/wireless/bcm43xx/bcm43xx.h 2006-06-03
21:15:23.0 +0200
@@ -647,9 +647,9 @@
void __iomem *mmio_addr;
unsigned int mmio_len;
- /* Do not use the lock directly. Use the bcm43xx_lock* helper
-* functions, to be MMIO-safe. */
- spinlock_t _lock;
+ /* Locking, see theory of locking text below. */
+ spinlock_t irq_lock;
+ struct mutex mutex;
/* Driver status flags. */
u32 initialized:1, /* init_board() succeed */
@@ -722,7 +722,7 @@
struct tasklet_struct isr_tasklet;
/* Periodic tasks */
- struct timer_list periodic_tasks;
+ struct work_struct periodic_work;
unsigned int periodic_state;
struct work_struct restart_work;
@@ -747,21 +747,55 @@
#endif
};
-/* bcm43xx_(un)lock() protect struct bcm43xx_private.
- * Note that _NO_ MMIO writes are allowed. If you want to
- * write to the device through MMIO in the critical section, use
- * the *_mmio lock functions.
- * MMIO read-access is allowed, though.
- */
-#define bcm43xx_lock(bcm, flags) spin_lock_irqsave((bcm)-_lock, flags)
-#define bcm43xx_unlock(bcm, flags) spin_unlock_irqrestore((bcm)-_lock,
flags)
-/* bcm43xx_(un)lock_mmio() protect struct bcm43xx_private and MMIO.
- * MMIO write-access to the device is allowed.
- * All MMIO writes are flushed on unlock, so it is guaranteed to not
- * interfere with other threads writing MMIO registers.
+
+/**** THEORY OF LOCKING ***
+ *
+ * We have two different locks in the bcm43xx driver.
+ * = bcm-mutex:General sleeping mutex. Protects struct bcm43xx_private
+ * and the device registers.
+ * = bcm-irq_lock: IRQ spinlock. Protects against IRQ handler concurrency.
+ *
+ * We have three types of helper function pairs to utilize these locks.
+ * (Always use the helper functions.)
+ * 1) bcm43xx_{un}lock_noirq():
+ * Takes bcm-mutex. Does _not_ protect against IRQ concurrency,
+ * so it is almost always unsafe, if device IRQs are enabled.
+ * So only use this, if device IRQs are masked.
+ * Locking may sleep.
+ * You can sleep within the critical section.
+ * 2) bcm43xx_{un}lock_irqonly():
+ * Takes bcm-irq_lock. Does _not_ protect against
+ * bcm43xx_lock_noirq() critical sections.
+ * Does only protect against the IRQ handler path and other
+ * irqonly() critical sections.
+ * Locking does not sleep.
+ * You must not sleep within the critical section.
+ * 3) bcm43xx_{un}lock_irqsafe():
+ * This is the cummulative lock and takes both, mutex and irq_lock.
+ * Protects against noirq() and irqonly() critical sections (and
+ * the IRQ handler path).
+ * Locking may sleep.
+ * You must not sleep within the critical section.
*/
-#define bcm43xx_lock_mmio(bcm, flags) bcm43xx_lock(bcm, flags)
-#define bcm43xx_unlock_mmio(bcm, flags)do { mmiowb();
bcm43xx_unlock(bcm, flags); } while (0)
+
+/* Lock type 1 */
+#define bcm43xx_lock_noirq(bcm)mutex_lock((bcm)-mutex)
+#define bcm43xx_unlock_noirq(bcm) mutex_unlock((bcm)-mutex)
+/* Lock type 2 */
+#define bcm43xx_lock_irqonly(bcm, flags) \
+ spin_lock_irqsave((bcm)-irq_lock, flags)
+#define bcm43xx_unlock_irqonly(bcm, flags) \
+ spin_unlock_irqrestore((bcm)-irq_lock, flags)
+/* Lock type 3 */
+#define bcm43xx_lock_irqsafe(bcm, flags) do { \
+ bcm43xx_lock_noirq(bcm);\
+ bcm43xx_lock_irqonly(bcm, flags); \
+ } while (0)
+#define bcm43xx_unlock_irqsafe(bcm, flags) do {\
+ bcm43xx_unlock_irqonly(bcm, flags); \
+ bcm43xx_unlock_noirq(bcm); \
+ } while (0)
+
static inline
struct bcm43xx_private * bcm43xx_priv(struct net_device *dev)
Index:
Re: [PATCH, RTF/RFC] bcm43xx: Enable local IRQs while executing periodic work
On Sun, Jun 04, 2006 at 09:26:47PM +0200, Michael Buesch wrote: Ok, I'm back with a completely rewritten version of the preemptible periodic work patch. Your patch applies against 2.6.17-rc5-git11, but compilation fails with: CC [M] drivers/net/wireless/bcm43xx/bcm43xx_main.o drivers/net/wireless/bcm43xx/bcm43xx_main.c: In function 'disable_irqs_sync_bottomhalf': drivers/net/wireless/bcm43xx/bcm43xx_main.c:510: error: too few arguments to function 'synchronize_irq' Jason ___ Bcm43xx-dev mailing list Bcm43xx-dev@lists.berlios.de http://lists.berlios.de/mailman/listinfo/bcm43xx-dev
Re: [PATCH, RTF/RFC] bcm43xx: Enable local IRQs while executing periodic work
I guess maybe you never compiled for smp? synchronize_irq() is just
barrier() for !CONFIG_SMP.
Did you mean this?
Jason
---
drivers/net/wireless/bcm43xx/bcm43xx_main.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: linux-2.6.17-rc5-git11/drivers/net/wireless/bcm43xx/bcm43xx_main.c
===
--- linux-2.6.17-rc5-git11.orig/drivers/net/wireless/bcm43xx/bcm43xx_main.c
+++ linux-2.6.17-rc5-git11/drivers/net/wireless/bcm43xx/bcm43xx_main.c
@@ -507,7 +507,7 @@
static void disable_irqs_sync_bottomhalf(struct bcm43xx_private *bcm)
{
- synchronize_irq();
+ synchronize_irq(bcm-irq);
tasklet_disable(bcm-isr_tasklet);
}
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
http://lists.berlios.de/mailman/listinfo/bcm43xx-dev
Re: [PATCH, RTF/RFC] bcm43xx: Enable local IRQs while executing periodic work
On Sunday 04 June 2006 23:13, Jason Lunz wrote:
I guess maybe you never compiled for smp? synchronize_irq() is just
barrier() for !CONFIG_SMP.
Did you mean this?
Yes, this is a typo.
Jason
---
drivers/net/wireless/bcm43xx/bcm43xx_main.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: linux-2.6.17-rc5-git11/drivers/net/wireless/bcm43xx/bcm43xx_main.c
===
--- linux-2.6.17-rc5-git11.orig/drivers/net/wireless/bcm43xx/bcm43xx_main.c
+++ linux-2.6.17-rc5-git11/drivers/net/wireless/bcm43xx/bcm43xx_main.c
@@ -507,7 +507,7 @@
static void disable_irqs_sync_bottomhalf(struct bcm43xx_private *bcm)
{
- synchronize_irq();
+ synchronize_irq(bcm-irq);
tasklet_disable(bcm-isr_tasklet);
}
--
Greetings Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
http://lists.berlios.de/mailman/listinfo/bcm43xx-dev
Re: [PATCH, RTF/RFC] bcm43xx: Enable local IRQs while executing periodic work
On Sunday 04 June 2006 23:20, Jason Lunz wrote: I got this after modules built. The acpi and jffs ones aren't new -- I don't know whether the bcm43xx ones are new or not: Building modules, stage 2. MODPOST WARNING: drivers/acpi/processor.o - Section mismatch: reference to .init.data: from .text between 'acpi_processor_power_init' (at offset 0x114c) and 'acpi_safe_halt' WARNING: fs/jffs2/jffs2.o - Section mismatch: reference to .init.text:jffs2_zlib_init from .text between 'jffs2_compressors_init' (at offset 0x17) and 'jffs2_free_comprbuf' WARNING: bcm43xx_lock_mmio [drivers/net/wireless/bcm43xx/bcm43xx.ko] undefined! WARNING: bcm43xx_unlock_mmio [drivers/net/wireless/bcm43xx/bcm43xx.ko] undefined! I am wondering why I did not get these warnings... You won't be able to insmod the module. -- Greetings Michael. ___ Bcm43xx-dev mailing list Bcm43xx-dev@lists.berlios.de http://lists.berlios.de/mailman/listinfo/bcm43xx-dev
Re: [PATCH, RTF/RFC] bcm43xx: Enable local IRQs while executing periodic work
On Sun, Jun 04, 2006 at 11:20:35PM +0200, Michael Buesch wrote: I am wondering why I did not get these warnings... You won't be able to insmod the module. yes, I noticed. it was my fault - i still had Stefano's rfkill button patch applied. Jason ___ Bcm43xx-dev mailing list Bcm43xx-dev@lists.berlios.de http://lists.berlios.de/mailman/listinfo/bcm43xx-dev
Re: [PATCH, RTF/RFC] bcm43xx: Enable local IRQs while executing periodic work
On Sunday 04 June 2006 23:31, Jason Lunz wrote:
On Sun, Jun 04, 2006 at 11:20:35PM +0200, Michael Buesch wrote:
I am wondering why I did not get these warnings...
You won't be able to insmod the module.
yes, I noticed.
it was my fault - i still had Stefano's rfkill button patch applied.
Ah, ok. :)
the bcm43xx_{un}lock_mmio can be replaced by bcm43xx_{un}lock_irqsafe
in that patch.
--
Greetings Michael.
___
Bcm43xx-dev mailing list
Bcm43xx-dev@lists.berlios.de
http://lists.berlios.de/mailman/listinfo/bcm43xx-dev
Re: [PATCH, RTF/RFC] bcm43xx: Enable local IRQs while executing periodic work
On Sun, Jun 04, 2006 at 09:26:47PM +0200, Michael Buesch wrote: So, here we go. Preemptible Periodic Work for bcm43xx-softmac. Please stress-test with all methods which come to your minds. I tested with the same configuration that I could always crash before (smp/preempt/debug kernel, then run iperf load tests for an hour or so). I couldn't get it to crash this time. I saw no problems at all. This was the non-shared interrupt case - /proc/interrupts looked like this when I gave up (i lost the beginning): 8: 0IO-APIC-edge rtc 9: 9185 IO-APIC-level acpi 12: 5305IO-APIC-edge i8042 14: 62265IO-APIC-edge ide0 15: 40518IO-APIC-edge ide1 16: 4 IO-APIC-level yenta, ohci1394 17: 1 IO-APIC-level yenta, rtk 18:435 IO-APIC-level ohci_hcd:usb1, NVidia nForce3 Modem 19: 36 IO-APIC-level ohci_hcd:usb2, NVidia nForce3 20: 0 IO-APIC-level ehci_hcd:usb3 21: 11233681 IO-APIC-level bcm43xx NMI: 1419 LOC:1157643 ERR: 0 MIS: 6251 21 9149 So I decided to test the shared interrupt case. When I boot this machine with 'noapic', I get a /proc/interrupts that looks like this: CPU0 0: 209671 XT-PIC timer 1: 1648 XT-PIC i8042 2: 0 XT-PIC cascade 7: 42 XT-PIC parport0 8: 0 XT-PIC rtc 9:472 XT-PIC acpi 10:575 XT-PIC yenta, ohci_hcd:usb2, ehci_hcd:usb3, NVidia nForce3 Modem, rtk 11: 904239 XT-PIC yenta, ohci1394, ohci_hcd:usb1, NVidia nForce3, bcm43xx 12:120 XT-PIC i8042 14: 54296 XT-PIC ide0 15: 6822 XT-PIC ide1 NMI:113 LOC: 209646 ERR: 0 MIS: 0 I found out which of the usb ports was on irq 11, plugged in a usb mouse, and checked that it generated interrupts. (I had to do cat /dev/input/mice /dev/null). Then I repeated the network load tests on the shared bcm43xx irq. Still no problems. looks good! send it upstream. :) Jason ___ Bcm43xx-dev mailing list Bcm43xx-dev@lists.berlios.de http://lists.berlios.de/mailman/listinfo/bcm43xx-dev
Re: [PATCH, RTF/RFC] bcm43xx: Enable local IRQs while executing periodic work
Ok, I think we must take another approach here. It is damn hard to write a working locking mechanism that has low latency for the periodic work and is still free of races. It is so hard, because we call into the driver from many different contexts. (And even if we consider dscape, and we really want to, I don't even know all the contexts for sure). This would always result in _lots_ of magic scattered all around the code and I am not really a friend of magic. ;) The other approach would be to insert voluntarily preemtion points into the heavy LO recalc code. I will come up with a new experimental patch, as soon as I finished it. Thanks again for your help in testing the stuff. -- Greetings Michael. [2007: Knorkator (Germany) - 12 points] ___ Bcm43xx-dev mailing list Bcm43xx-dev@lists.berlios.de http://lists.berlios.de/mailman/listinfo/bcm43xx-dev
Re: [PATCH, RTF/RFC] bcm43xx: Enable local IRQs while executing periodic work
On Monday 29 May 2006 22:40, you wrote: On Monday 29 May 2006 17:51, you wrote: On Mon, May 29, 2006 at 12:44:05AM +0200, Michael Buesch wrote: Ok Jason, could you please test the following patch and try to reproduce with it? This patch crashes immediately: http://gehennom.net/~lunz/bcm43xx_crash2.jpg Whoah, that's damn weird. I have no idea what's going on. Uh, well. Just after sending the mail I got an idea. :) Does SoftMAC call back into the driver from a bottom half? Maybe that could race somehow and deadlock. -- Greetings Michael. [2007: Knorkator (Germany) - 12 points] ___ Bcm43xx-dev mailing list Bcm43xx-dev@lists.berlios.de http://lists.berlios.de/mailman/listinfo/bcm43xx-dev
Re: [PATCH, RTF/RFC] bcm43xx: Enable local IRQs while executing periodic work
On Mon, May 29, 2006 at 10:40:14PM +0200, Michael Buesch wrote: Whoah, that's damn weird. I have no idea what's going on. I'm sorry. Does someone else have an idea? Otherwise I think we must drop that patch and live with the high periodic work latency. I really don't see how this is still possible to race. Even on UP(!). Jason, I added an assertion that should be able to catch periodic work vs IRQ handler races. You did not see it trigger by chance? Everything that looked slightly suspicious was in my email. Do you have a string for me to grep for? Besides that, this seems like a race of periodic work against the tasklet. But I really don't see how this is possible. I really don't know. It's definitely your patch though. Just to double-check, I compiled a 2.6.17-rc5-git5 kernel with an identical .config (SMP/PREEMPT + kernel debugging), but neither of your experimental patches. I set it up under the same iperf + disk activity load. It's up to 37 million bcm43xx interrupts right now with no trouble at all. Maybe there's something about preempt that violates one of your assumptions? I know that it's supposed to be good at triggering races. Jason ___ Bcm43xx-dev mailing list Bcm43xx-dev@lists.berlios.de http://lists.berlios.de/mailman/listinfo/bcm43xx-dev
Re: [PATCH, RTF/RFC] bcm43xx: Enable local IRQs while executing periodic work
On Monday 29 May 2006 23:25, Jason Lunz wrote: On Mon, May 29, 2006 at 10:40:14PM +0200, Michael Buesch wrote: Whoah, that's damn weird. I have no idea what's going on. I'm sorry. Does someone else have an idea? Otherwise I think we must drop that patch and live with the high periodic work latency. I really don't see how this is still possible to race. Even on UP(!). Jason, I added an assertion that should be able to catch periodic work vs IRQ handler races. You did not see it trigger by chance? Everything that looked slightly suspicious was in my email. Do you have a string for me to grep for? Every assertion failed string is interresting. Besides that, this seems like a race of periodic work against the tasklet. But I really don't see how this is possible. I really don't know. It's definitely your patch though. Just to double-check, I compiled a 2.6.17-rc5-git5 kernel with an identical .config (SMP/PREEMPT + kernel debugging), but neither of your experimental patches. I set it up under the same iperf + disk activity load. It's up to 37 million bcm43xx interrupts right now with no trouble at all. Maybe there's something about preempt that violates one of your assumptions? I know that it's supposed to be good at triggering races. yes, I think it is preemption and the softmac workqueues. So, current bcm43xx locking can't really handle that correctly. I am currently thinking about how to redesign it. Yes, we could disable bh processing and preemtion throughout the periodic work handler, but I don't think we want that, as we wanted to reduce latency. Well. bh disabling might still be needed. -- Greetings Michael. [2007: Knorkator (Germany) - 12 points] ___ Bcm43xx-dev mailing list Bcm43xx-dev@lists.berlios.de http://lists.berlios.de/mailman/listinfo/bcm43xx-dev
Re: [PATCH, RTF/RFC] bcm43xx: Enable local IRQs while executing periodic work
On Sun, May 28, 2006 at 06:29:01PM +0200, Michael Buesch wrote: Well, I would like to know it for sure. ;) Could you load the kernel which crashed and look there. It is quite important to know if it is caused by a shared IRQ or not. OK, i'm running the kernel that crashed now. it's the same: Linux opus 2.6.17-rc5-git3-suspend2 #2 SMP PREEMPT Sat May 27 14:17:15 EDT 2006 x86_64 GNU/Linux CPU0 0: 32010IO-APIC-edge timer 1:490IO-APIC-edge i8042 7: 3IO-APIC-edge parport0 8: 0IO-APIC-edge rtc 9:150 IO-APIC-level acpi 12:120IO-APIC-edge i8042 14: 5428IO-APIC-edge ide0 15:841IO-APIC-edge ide1 16: 4 IO-APIC-level yenta, ohci1394 17: 1 IO-APIC-level yenta, rtk 18: 0 IO-APIC-level ehci_hcd:usb1, NVidia nForce3 19: 0 IO-APIC-level ohci_hcd:usb2, NVidia nForce3 Modem 20: 0 IO-APIC-level ohci_hcd:usb3 21: 1559 IO-APIC-level bcm43xx NMI: 50 LOC: 31972 ERR: 0 MIS: 0 Jason ___ Bcm43xx-dev mailing list Bcm43xx-dev@lists.berlios.de http://lists.berlios.de/mailman/listinfo/bcm43xx-dev
Re: [PATCH, RTF/RFC] bcm43xx: Enable local IRQs while executing periodic work
On Sunday 28 May 2006 18:37, you wrote:
On Sun, May 28, 2006 at 06:29:01PM +0200, Michael Buesch wrote:
Well, I would like to know it for sure. ;)
Could you load the kernel which crashed and look there. It is
quite important to know if it is caused by a shared IRQ or not.
OK, i'm running the kernel that crashed now. it's the same:
Ok Jason, could you please test the following patch and try to reproduce
with it?
Jory, could you also test this patch on your machine with the shared
bcm43xx IRQ? While testing, put some load on one of the devices sharing
the IRQ with bcm43xx (I think it was eth0 for example).
Index: wireless-dev/drivers/net/wireless/bcm43xx/bcm43xx.h
===
--- wireless-dev.orig/drivers/net/wireless/bcm43xx/bcm43xx.h2006-05-28
23:57:01.0 +0200
+++ wireless-dev/drivers/net/wireless/bcm43xx/bcm43xx.h 2006-05-29
00:16:50.0 +0200
@@ -647,9 +647,7 @@
void __iomem *mmio_addr;
unsigned int mmio_len;
- /* Do not use the lock directly. Use the bcm43xx_lock* helper
-* functions, to be MMIO-safe. */
- spinlock_t _lock;
+ spinlock_t lock;
/* Driver status flags. */
u32 initialized:1, /* init_board() succeed */
@@ -704,6 +702,9 @@
/* Number of available 80211 cores. */
int nr_80211_available;
+ /* If 0, the MAC is enabled. Suspended otherwise. */
+ int mac_suspended;
+
u32 chipcommon_capabilities;
/* Reason code of the last interrupt. */
@@ -744,6 +745,7 @@
/* Debugging stuff follows. */
#ifdef CONFIG_BCM43XX_DEBUG
struct bcm43xx_dfsentry *dfsentry;
+ atomic_t periodic_work_in_progress;
#endif
};
@@ -753,8 +755,8 @@
* the *_mmio lock functions.
* MMIO read-access is allowed, though.
*/
-#define bcm43xx_lock(bcm, flags) spin_lock_irqsave((bcm)-_lock, flags)
-#define bcm43xx_unlock(bcm, flags) spin_unlock_irqrestore((bcm)-_lock,
flags)
+#define bcm43xx_lock(bcm, flags) spin_lock_irqsave((bcm)-lock, flags)
+#define bcm43xx_unlock(bcm, flags) spin_unlock_irqrestore((bcm)-lock,
flags)
/* bcm43xx_(un)lock_mmio() protect struct bcm43xx_private and MMIO.
* MMIO write-access to the device is allowed.
* All MMIO writes are flushed on unlock, so it is guaranteed to not
Index: wireless-dev/drivers/net/wireless/bcm43xx/bcm43xx_main.c
===
--- wireless-dev.orig/drivers/net/wireless/bcm43xx/bcm43xx_main.c
2006-05-28 23:57:01.0 +0200
+++ wireless-dev/drivers/net/wireless/bcm43xx/bcm43xx_main.c2006-05-29
00:41:58.0 +0200
@@ -496,20 +496,36 @@
return old_mask;
}
-/* Make sure we don't receive more data from the device. */
+/* Disable IRQs on the device and make sure no IRQ handler
+ * (or tasklet) is running on return.
+ */
static int bcm43xx_disable_interrupts_sync(struct bcm43xx_private *bcm, u32
*oldstate)
{
u32 old;
unsigned long flags;
+ unsigned int irq;
bcm43xx_lock_mmio(bcm, flags);
- if (bcm43xx_is_initializing(bcm) || bcm-shutting_down) {
+ if (unlikely(bcm43xx_is_initializing(bcm) ||
+bcm-shutting_down)) {
bcm43xx_unlock_mmio(bcm, flags);
return -EBUSY;
}
+ bcm43xx_mac_suspend(bcm);
+ irq = bcm-irq;
old = bcm43xx_interrupt_disable(bcm, BCM43xx_IRQ_ALL);
- tasklet_disable(bcm-isr_tasklet);
+ bcm43xx_read32(bcm, BCM43xx_MMIO_GEN_IRQ_MASK); /* dummy read. */
+
+ /* Unlock, so running IRQ handlers or tasklets can return
+* and don't deadlock.
+* The access of isr_tasklet after unlocking is Ok, because
+* it can not race after synchronizing IRQ.
+*/
bcm43xx_unlock_mmio(bcm, flags);
+
+ synchronize_irq(irq);
+ tasklet_disable(bcm-isr_tasklet);
+
if (oldstate)
*oldstate = old;
@@ -1868,8 +1884,9 @@
if (!bcm)
return IRQ_NONE;
- spin_lock(bcm-_lock);
-
+ /* Do the reason checking unlocked, as we could otherwise deadlock
+* with the periodic work handler.
+*/
reason = bcm43xx_read32(bcm, BCM43xx_MMIO_GEN_IRQ_REASON);
if (reason == 0x) {
/* irq not for us (shared irq) */
@@ -1880,6 +1897,9 @@
if (!reason)
goto out;
+ assert(!atomic_read(bcm-periodic_work_in_progress));
+ spin_lock(bcm-lock);
+
bcm-dma_reason[0] = bcm43xx_read32(bcm, BCM43xx_MMIO_DMA1_REASON)
0x0001dc00;
bcm-dma_reason[1] = bcm43xx_read32(bcm, BCM43xx_MMIO_DMA2_REASON)
@@ -1907,7 +1927,7 @@
out:
mmiowb();
- spin_unlock(bcm-_lock);
+ spin_unlock(bcm-lock);
return ret;
}
@@ -2271,13 +2291,17 @@
/* http://bcm-specs.sipsolutions.net/EnableMac */
void
Re: [PATCH, RTF/RFC] bcm43xx: Enable local IRQs while executing periodic work
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Michael Buesch wrote: On Sunday 28 May 2006 18:37, you wrote: On Sun, May 28, 2006 at 06:29:01PM +0200, Michael Buesch wrote: Well, I would like to know it for sure. ;) Could you load the kernel which crashed and look there. It is quite important to know if it is caused by a shared IRQ or not. OK, i'm running the kernel that crashed now. it's the same: Ok Jason, could you please test the following patch and try to reproduce with it? Jory, could you also test this patch on your machine with the shared bcm43xx IRQ? While testing, put some load on one of the devices sharing the IRQ with bcm43xx (I think it was eth0 for example). I have tested the hell out of the patch works great so far, no crash nothing of bad news to report here. I have done some intesive testing with eth0 and also done major testing with acpi as I disabled noapic on the boot options. So finall words would be to push it to 2.6.17-rc* as I am sure it will be a few more release canadites before it rolls out as final. Jory -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.3 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org iD8DBQFEelE1GDfjNg8unQIRAjAtAJ4k1CBAX1sX8li4hHzQkjdnwvovwQCdGAWm Gj7la0sFPldjftUP+PuSaPI= =yVFf -END PGP SIGNATURE- ___ Bcm43xx-dev mailing list Bcm43xx-dev@lists.berlios.de http://lists.berlios.de/mailman/listinfo/bcm43xx-dev
Re: [PATCH, RTF/RFC] bcm43xx: Enable local IRQs while executing periodic work
On Thu, May 25, 2006 at 02:02:37PM +0200, Michael Buesch wrote: Uh, don't let me dangling like this. :) Could you please try to reproduce the lockup and catch an oops? It's important, because it may be caused by bcm43xx. I will, but I've been quite busy at work and I have only a free hour or so each night when I can try things out. So it may take a few days to pin down what's going on. I'll let you know how tonight's run goes. In case I wasn't clear, I *did* look for an oops, but there was none in syslog and it locked up in X, so there wasn't one on screen either. I'll try next time with serial console. Do you have any suggestions about what config would best stress it? I went with smp+full preempt to try and simulate smp races on the single processor I have. Jason ___ Bcm43xx-dev mailing list Bcm43xx-dev@lists.berlios.de http://lists.berlios.de/mailman/listinfo/bcm43xx-dev
Re: [PATCH, RTF/RFC] bcm43xx: Enable local IRQs while executing periodic work
On Thursday 25 May 2006 16:16, Jason Lunz wrote: On Thu, May 25, 2006 at 02:02:37PM +0200, Michael Buesch wrote: Uh, don't let me dangling like this. :) Could you please try to reproduce the lockup and catch an oops? It's important, because it may be caused by bcm43xx. I will, but I've been quite busy at work and I have only a free hour or so each night when I can try things out. So it may take a few days to pin down what's going on. I'll let you know how tonight's run goes. In case I wasn't clear, I *did* look for an oops, but there was none in syslog and it locked up in X, so there wasn't one on screen either. I'll try next time with serial console. Do you have any suggestions about what config would best stress it? I went with smp+full preempt to try and simulate smp races on the single processor I have. The spinlock debugging related options under Kernel Hacking should catch all bugs. Especially the spinlock recursion detection on your UP system. ___ Bcm43xx-dev mailing list Bcm43xx-dev@lists.berlios.de http://lists.berlios.de/mailman/listinfo/bcm43xx-dev
Re: [PATCH, RTF/RFC] bcm43xx: Enable local IRQs while executing periodic work
One stupid question regarding inclusion of bcm43xx in upcoming 2.6.17 :for my 4306 to work (well configured with WEP key, etc..), after every boot, I have to issue as root :iwlist scan eth1dhclient eth1 because the WEP authentication was not totally finished with my APIIRC, there is a patch somewhere, that should make bcm43xx more 'compatible' with the way other wlan drivers are configured at boot, and that it should eventually go in 2.6.17 ?sorry for being off-topic..On 5/23/06, Michael Buesch [EMAIL PROTECTED] wrote: This patch is supposed to heavily increase overall system vitality. It reduces the time periodic work in the bcm43xx driver is run, with IRQs disabled, from several msecs to a few usecs. I am not sure, if we still want to have this in 2.6.17, as it is not a crash fix or something. But it fixes lost interrupt problems for some people.
