* mac80211 local_bh_enable called with IRQs disabled (was: b43legacy kernel warning) [not found] <30353c3d0806171359k6d7be389g23dfc809f288d2f1@mail.gmail.com> @ 2008-06-17 21:20 ` Michael Buesch 2008-06-18 10:02 ` Johannes Berg 0 siblings, 1 reply; 3+ messages in thread From: Michael Buesch @ 2008-06-17 21:20 UTC (permalink / raw) To: bcm43xx-dev; +Cc: David Ellingsworth, Johannes Berg, linux-wireless On Tuesday 17 June 2008 22:59:52 David Ellingsworth wrote: > I ran into this error today from a kernel I built last night based on > the latest wireless-testing branch. Correct me if I'm wrong, but it > looks to be b43legacy related. I'm a bit new to kernel debugging but > can try to provide additional information if instructions on how to do > so are provided. No, this is a mac80211 bug > -------------------------------------------------------------------------------------- > dmesg output: > -------------------------------------------------------------------------------------- > WARNING: at kernel/softirq.c:141 local_bh_enable+0x1f/0x59() > Modules linked in: ipt_MASQUERADE iptable_nat nf_nat nf_conntrack_ipv4 > nf_conntrack ip_tables x_tables radeon drm binfmt_misc ppdev lp > dm_snapshot dm_mirror dm_log dm_mod b43legacy mac80211 cfg80211 > led_class snd_ali5451 snd_ac97_codec ac97_bus snd_pcm_oss > snd_mixer_oss ati_agp ibmcam usbvideo ssb pcmcia snd_pcm snd_timer > evdev videodev v4l1_compat container battery ac button firmware_class > agpgart i2c_ali1535 snd video output parport_pc parport yenta_socket > rsrc_nonstatic pcmcia_core psmouse serio_raw i2c_ali15x3 i2c_core > soundcore snd_page_alloc pcspkr reiserfs ide_cd_mod cdrom ide_disk > alim15x3 natsemi ide_pci_generic ide_core ohci_hcd usbcore thermal > processor fan > Pid: 1371, comm: b43legacy Not tainted 2.6.26-rc6-wl #1 > [<c0113be7>] warn_on_slowpath+0x40/0x66 > [<c0111c60>] check_preempt_wakeup+0xa2/0xc0 > [<c0120d0d>] autoremove_wake_function+0xc/0x2b > [<c010fc44>] __wake_up_common+0x2d/0x52 > [<c011126e>] __wake_up+0xf/0x15 > [<c0113f89>] wake_up_klogd+0x2b/0x2d > [<c01981ed>] scnprintf+0x1f/0x2b > [<c0117477>] local_bh_enable+0x1f/0x59 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This function does not like being run with IRQs disabled. WARN_ON_ONCE(irqs_disabled()); > [<dcf44045>] __sta_info_unlink+0xa9/0x134 [mac80211] > [<dcf4453f>] sta_info_unlink+0x9/0xd [mac80211] ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This function disables interrupts > [<dcf48968>] ieee80211_associated+0x9f/0x148 [mac80211] > [<dcf4b463>] ieee80211_sta_work+0x4cb/0x64f [mac80211] > [<c0110392>] update_curr+0x3d/0x52 > [<c01104ee>] dequeue_entity+0xf/0x8d > [<c0111979>] dequeue_task_fair+0x13/0x27 > [<c010fab2>] dequeue_task+0xa/0x14 > [<c0249b01>] schedule+0x22e/0x24a > [<dcf4af98>] ieee80211_sta_work+0x0/0x64f [mac80211] > [<c011e7a7>] run_workqueue+0x63/0xca > [<c011ec96>] worker_thread+0x0/0xb7 > [<c011ed42>] worker_thread+0xac/0xb7 > [<c0120d01>] autoremove_wake_function+0x0/0x2b > [<c011ec96>] worker_thread+0x0/0xb7 > [<c0120ba6>] kthread+0x36/0x5c > [<c0120b70>] kthread+0x0/0x5c > [<c010390b>] kernel_thread_helper+0x7/0x10 I'm not sure _why_ local_bh_enable() doesn't like getting called with interrupts disabled. The code is a bit weird: 134 void local_bh_enable(void) 135 { 136 #ifdef CONFIG_TRACE_IRQFLAGS 137 unsigned long flags; 138 139 WARN_ON_ONCE(in_irq()); 140 #endif 141 WARN_ON_ONCE(irqs_disabled()); 142 143 #ifdef CONFIG_TRACE_IRQFLAGS 144 local_irq_save(flags); 145 #endif 146 /* 147 * Are softirqs going to be turned on now: 148 */ 149 if (softirq_count() == SOFTIRQ_OFFSET) 150 trace_softirqs_on((unsigned long)__builtin_return_address(0)); 151 /* 152 * Keep preemption disabled until we are done with 153 * softirq processing: 154 */ 155 sub_preempt_count(SOFTIRQ_OFFSET - 1); 156 157 if (unlikely(!in_interrupt() && local_softirq_pending())) 158 do_softirq(); 159 160 dec_preempt_count(); 161 #ifdef CONFIG_TRACE_IRQFLAGS 162 local_irq_restore(flags); 163 #endif 164 preempt_check_resched(); 165 } 166 EXPORT_SYMBOL(local_bh_enable); -- Greetings Michael. ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: mac80211 local_bh_enable called with IRQs disabled (was: b43legacy kernel warning) 2008-06-17 21:20 ` mac80211 local_bh_enable called with IRQs disabled (was: b43legacy kernel warning) Michael Buesch @ 2008-06-18 10:02 ` Johannes Berg 2008-06-18 12:30 ` David Ellingsworth 0 siblings, 1 reply; 3+ messages in thread From: Johannes Berg @ 2008-06-18 10:02 UTC (permalink / raw) To: Michael Buesch; +Cc: bcm43xx-dev, David Ellingsworth, linux-wireless On Tue, 2008-06-17 at 23:20 +0200, Michael Buesch wrote: > On Tuesday 17 June 2008 22:59:52 David Ellingsworth wrote: > > I ran into this error today from a kernel I built last night based on > > the latest wireless-testing branch. Correct me if I'm wrong, but it > > looks to be b43legacy related. I'm a bit new to kernel debugging but > > can try to provide additional information if instructions on how to do > > so are provided. > > No, this is a mac80211 bug Can you try the patch below? johannes --- everything.orig/net/mac80211/sta_info.h 2008-06-18 10:07:24.000000000 +0200 +++ everything/net/mac80211/sta_info.h 2008-06-18 11:58:32.000000000 +0200 @@ -164,6 +164,7 @@ struct sta_ampdu_mlme { * @aid: STA's unique AID (1..2007, 0 = not assigned yet), * only used in AP (and IBSS?) mode * @flags: STA flags, see &enum ieee80211_sta_info_flags + * @flaglock: spinlock for flags accesses * @ps_tx_buf: buffer of frames to transmit to this station * when it leaves power saving state * @tx_filtered: buffer of frames we already tried to transmit @@ -186,6 +187,7 @@ struct sta_info { struct rate_control_ref *rate_ctrl; void *rate_ctrl_priv; spinlock_t lock; + spinlock_t flaglock; struct ieee80211_ht_info ht_info; u64 supp_rates[IEEE80211_NUM_BANDS]; u8 addr[ETH_ALEN]; @@ -198,7 +200,10 @@ struct sta_info { */ u8 pin_status; - /* frequently updated information, locked with lock spinlock */ + /* + * frequently updated, locked with own spinlock (flaglock), + * use the accessors defined below + */ u32 flags; /* @@ -293,34 +298,41 @@ static inline enum plink_state sta_plink static inline void set_sta_flags(struct sta_info *sta, const u32 flags) { - spin_lock_bh(&sta->lock); + unsigned long irqfl; + + spin_lock_irqsave(&sta->flaglock, irqfl); sta->flags |= flags; - spin_unlock_bh(&sta->lock); + spin_unlock_irqrestore(&sta->flaglock, irqfl); } static inline void clear_sta_flags(struct sta_info *sta, const u32 flags) { - spin_lock_bh(&sta->lock); + unsigned long irqfl; + + spin_lock_irqsave(&sta->flaglock, irqfl); sta->flags &= ~flags; - spin_unlock_bh(&sta->lock); + spin_unlock_irqrestore(&sta->flaglock, irqfl); } static inline void set_and_clear_sta_flags(struct sta_info *sta, const u32 set, const u32 clear) { - spin_lock_bh(&sta->lock); + unsigned long irqfl; + + spin_lock_irqsave(&sta->flaglock, irqfl); sta->flags |= set; sta->flags &= ~clear; - spin_unlock_bh(&sta->lock); + spin_unlock_irqrestore(&sta->flaglock, irqfl); } static inline u32 test_sta_flags(struct sta_info *sta, const u32 flags) { u32 ret; + unsigned long irqfl; - spin_lock_bh(&sta->lock); + spin_lock_irqsave(&sta->flaglock, irqfl); ret = sta->flags & flags; - spin_unlock_bh(&sta->lock); + spin_unlock_irqrestore(&sta->flaglock, irqfl); return ret; } @@ -329,11 +341,12 @@ static inline u32 test_and_clear_sta_fla const u32 flags) { u32 ret; + unsigned long irqfl; - spin_lock_bh(&sta->lock); + spin_lock_irqsave(&sta->flaglock, irqfl); ret = sta->flags & flags; sta->flags &= ~flags; - spin_unlock_bh(&sta->lock); + spin_unlock_irqrestore(&sta->flaglock, irqfl); return ret; } @@ -341,10 +354,11 @@ static inline u32 test_and_clear_sta_fla static inline u32 get_sta_flags(struct sta_info *sta) { u32 ret; + unsigned long irqfl; - spin_lock_bh(&sta->lock); + spin_lock_irqsave(&sta->flaglock, irqfl); ret = sta->flags; - spin_unlock_bh(&sta->lock); + spin_unlock_irqrestore(&sta->flaglock, irqfl); return ret; } --- everything.orig/net/mac80211/sta_info.c 2008-06-18 11:56:44.000000000 +0200 +++ everything/net/mac80211/sta_info.c 2008-06-18 11:56:58.000000000 +0200 @@ -235,6 +235,7 @@ struct sta_info *sta_info_alloc(struct i return NULL; spin_lock_init(&sta->lock); + spin_lock_init(&sta->flaglock); memcpy(sta->addr, addr, ETH_ALEN); sta->local = local; ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: mac80211 local_bh_enable called with IRQs disabled (was: b43legacy kernel warning) 2008-06-18 10:02 ` Johannes Berg @ 2008-06-18 12:30 ` David Ellingsworth 0 siblings, 0 replies; 3+ messages in thread From: David Ellingsworth @ 2008-06-18 12:30 UTC (permalink / raw) To: bcm43xx-dev; +Cc: Michael Buesch, linux-wireless On Wed, Jun 18, 2008 at 6:02 AM, Johannes Berg <johannes@sipsolutions.net> wrote: > On Tue, 2008-06-17 at 23:20 +0200, Michael Buesch wrote: >> On Tuesday 17 June 2008 22:59:52 David Ellingsworth wrote: >> > I ran into this error today from a kernel I built last night based on >> > the latest wireless-testing branch. Correct me if I'm wrong, but it >> > looks to be b43legacy related. I'm a bit new to kernel debugging but >> > can try to provide additional information if instructions on how to do >> > so are provided. >> >> No, this is a mac80211 bug > > Can you try the patch below? > > johannes > > --- everything.orig/net/mac80211/sta_info.h 2008-06-18 10:07:24.000000000 +0200 > +++ everything/net/mac80211/sta_info.h 2008-06-18 11:58:32.000000000 +0200 > @@ -164,6 +164,7 @@ struct sta_ampdu_mlme { > * @aid: STA's unique AID (1..2007, 0 = not assigned yet), > * only used in AP (and IBSS?) mode > * @flags: STA flags, see &enum ieee80211_sta_info_flags > + * @flaglock: spinlock for flags accesses > * @ps_tx_buf: buffer of frames to transmit to this station > * when it leaves power saving state > * @tx_filtered: buffer of frames we already tried to transmit > @@ -186,6 +187,7 @@ struct sta_info { > struct rate_control_ref *rate_ctrl; > void *rate_ctrl_priv; > spinlock_t lock; > + spinlock_t flaglock; > struct ieee80211_ht_info ht_info; > u64 supp_rates[IEEE80211_NUM_BANDS]; > u8 addr[ETH_ALEN]; > @@ -198,7 +200,10 @@ struct sta_info { > */ > u8 pin_status; > > - /* frequently updated information, locked with lock spinlock */ > + /* > + * frequently updated, locked with own spinlock (flaglock), > + * use the accessors defined below > + */ > u32 flags; > > /* > @@ -293,34 +298,41 @@ static inline enum plink_state sta_plink > > static inline void set_sta_flags(struct sta_info *sta, const u32 flags) > { > - spin_lock_bh(&sta->lock); > + unsigned long irqfl; > + > + spin_lock_irqsave(&sta->flaglock, irqfl); > sta->flags |= flags; > - spin_unlock_bh(&sta->lock); > + spin_unlock_irqrestore(&sta->flaglock, irqfl); > } > > static inline void clear_sta_flags(struct sta_info *sta, const u32 flags) > { > - spin_lock_bh(&sta->lock); > + unsigned long irqfl; > + > + spin_lock_irqsave(&sta->flaglock, irqfl); > sta->flags &= ~flags; > - spin_unlock_bh(&sta->lock); > + spin_unlock_irqrestore(&sta->flaglock, irqfl); > } > > static inline void set_and_clear_sta_flags(struct sta_info *sta, > const u32 set, const u32 clear) > { > - spin_lock_bh(&sta->lock); > + unsigned long irqfl; > + > + spin_lock_irqsave(&sta->flaglock, irqfl); > sta->flags |= set; > sta->flags &= ~clear; > - spin_unlock_bh(&sta->lock); > + spin_unlock_irqrestore(&sta->flaglock, irqfl); > } > > static inline u32 test_sta_flags(struct sta_info *sta, const u32 flags) > { > u32 ret; > + unsigned long irqfl; > > - spin_lock_bh(&sta->lock); > + spin_lock_irqsave(&sta->flaglock, irqfl); > ret = sta->flags & flags; > - spin_unlock_bh(&sta->lock); > + spin_unlock_irqrestore(&sta->flaglock, irqfl); > > return ret; > } > @@ -329,11 +341,12 @@ static inline u32 test_and_clear_sta_fla > const u32 flags) > { > u32 ret; > + unsigned long irqfl; > > - spin_lock_bh(&sta->lock); > + spin_lock_irqsave(&sta->flaglock, irqfl); > ret = sta->flags & flags; > sta->flags &= ~flags; > - spin_unlock_bh(&sta->lock); > + spin_unlock_irqrestore(&sta->flaglock, irqfl); > > return ret; > } > @@ -341,10 +354,11 @@ static inline u32 test_and_clear_sta_fla > static inline u32 get_sta_flags(struct sta_info *sta) > { > u32 ret; > + unsigned long irqfl; > > - spin_lock_bh(&sta->lock); > + spin_lock_irqsave(&sta->flaglock, irqfl); > ret = sta->flags; > - spin_unlock_bh(&sta->lock); > + spin_unlock_irqrestore(&sta->flaglock, irqfl); > > return ret; > } > --- everything.orig/net/mac80211/sta_info.c 2008-06-18 11:56:44.000000000 +0200 > +++ everything/net/mac80211/sta_info.c 2008-06-18 11:56:58.000000000 +0200 > @@ -235,6 +235,7 @@ struct sta_info *sta_info_alloc(struct i > return NULL; > > spin_lock_init(&sta->lock); > + spin_lock_init(&sta->flaglock); > > memcpy(sta->addr, addr, ETH_ALEN); > sta->local = local; > > > Yes, this patch seems to correct the issue. I no longer receive the kernel warning. Regards, David Ellingsworth ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2008-06-18 12:30 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <30353c3d0806171359k6d7be389g23dfc809f288d2f1@mail.gmail.com>
2008-06-17 21:20 ` mac80211 local_bh_enable called with IRQs disabled (was: b43legacy kernel warning) Michael Buesch
2008-06-18 10:02 ` Johannes Berg
2008-06-18 12:30 ` David Ellingsworth
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox