* [PATCH] ath9k : Fix ieee80211 work while going to suspend @ 2013-03-16 16:38 Parag Warudkar 2013-03-18 19:13 ` John W. Linville 0 siblings, 1 reply; 9+ messages in thread From: Parag Warudkar @ 2013-03-16 16:38 UTC (permalink / raw) To: Luis R. Rodriguez, Jouni Malinen, Vasanthakumar Thiagarajan, John W. Linville, linux-wireless, ath9k-devel, netdev, LKML, senthilb [-- Attachment #1: Type: text/plain, Size: 6422 bytes --] During suspend below warning is seen when ath9k is active. Attached patch fixes the warning for me. Tested to work across few suspend-resume cycles. Mar 16 09:39:17 Parags-iMac kernel: [ 3993.642939] WARNING: at net/mac80211/util.c:599 ieee80211_can_queue_work.isra.7+0x32/0x40 [mac80211]() Mar 16 09:39:17 Parags-iMac kernel: [ 3993.642940] Hardware name: iMac12,1 Mar 16 09:39:17 Parags-iMac kernel: [ 3993.642941] queueing ieee80211 work while going to suspend Mar 16 09:39:17 Parags-iMac kernel: [ 3993.642972] Modules linked in: joydev hid_logitech_dj zfs(POF) bridge stp llc zunicode(POF) zavl(POF) zcommon(POF) znvpair(POF) spl(OF) zlib_deflate be2iscsi iscsi_boot_sysfs bnx2i cnic uio cxgb4i cxgb4 cxgb3i cxgb3 mdio libcxgbi ib_iser rdma_cm ib_addr iw_cm ib_cm ib_sa ib_mad ib_core iscsi_tcp libiscsi_tcp libiscsi scsi_transport_iscsi rfcomm bnep nls_utf8 hfsplus btusb uvcvideo videobuf2_vmalloc videobuf2_memops videobuf2_core videodev bluetooth media coretemp snd_hda_codec_hdmi snd_hda_codec_cirrus snd_hda_intel snd_hda_codec snd_hwdep snd_seq snd_seq_device snd_pcm snd_page_alloc snd_timer snd soundcore arc4 ath9k ath9k_common microcode ath9k_hw ath mac80211 iTCO_wdt iTCO_vendor_support cfg80211 lpc_ich mei mfd_core rfkill i2c_i801 applesmc apple_bl input_polldev vhost_net tun macvtap macvlan kvm_intel kvm binfmt_misc uinput usb_storage crc32c_intel radeon ttm i915 ghash_clmulni_intel firewire_ohci firewire_core i2c_algo_bit drm_kms_helper crc_itu_t drm tg3 ptp pps_core i2c_core video sunrpc [last unloaded: iptable_mangle] Mar 16 09:39:17 Parags-iMac kernel: [ 3993.642985] Pid: 0, comm: swapper/0 Tainted: PF O 3.8.2-206.fc18.x86_64 #1 Mar 16 09:39:17 Parags-iMac kernel: [ 3993.642986] Call Trace: Mar 16 09:39:17 Parags-iMac kernel: [ 3993.642992] <IRQ> [<ffffffff8105e61f>] warn_slowpath_common+0x7f/0xc0 Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643009] [<ffffffffa0581420>] ? ath_start_rx_poll+0x70/0x70 [ath9k] Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643011] [<ffffffff8105e716>] warn_slowpath_fmt+0x46/0x50 Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643018] [<ffffffffa045b542>] ieee80211_can_queue_work.isra.7+0x32/0x40 [mac80211] Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643024] [<ffffffffa045b5de>] ieee80211_queue_work+0x2e/0x50 [mac80211] Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643027] [<ffffffffa0581438>] ath_rx_poll+0x18/0x20 [ath9k] Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643029] [<ffffffff8106d0aa>] call_timer_fn+0x3a/0x120 Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643032] [<ffffffffa0581420>] ? ath_start_rx_poll+0x70/0x70 [ath9k] Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643034] [<ffffffff814fb240>] ? cpufreq_p4_target+0x120/0x120 Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643035] [<ffffffff8106ee3e>] run_timer_softirq+0x1fe/0x2b0 Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643037] [<ffffffff814fb240>] ? cpufreq_p4_target+0x120/0x120 Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643038] [<ffffffff81066cd0>] __do_softirq+0xd0/0x210 Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643040] [<ffffffff8101b913>] ? native_sched_clock+0x13/0x80 Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643041] [<ffffffff814fb240>] ? cpufreq_p4_target+0x120/0x120 Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643043] [<ffffffff8165985c>] call_softirq+0x1c/0x30 Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643045] [<ffffffff810162a5>] do_softirq+0x75/0xb0 Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643046] [<ffffffff81066fa5>] irq_exit+0xb5/0xc0 Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643048] [<ffffffff8165a1de>] smp_apic_timer_interrupt+0x6e/0x99 Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643049] [<ffffffff8165911d>] apic_timer_interrupt+0x6d/0x80 Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643052] <EOI> [<ffffffff810868ce>] ? __hrtimer_start_range_ns+0x1be/0x400 Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643053] [<ffffffff814fbc30>] ? cpuidle_wrap_enter+0x50/0xa0 Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643054] [<ffffffff814fbc29>] ? cpuidle_wrap_enter+0x49/0xa0 Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643056] [<ffffffff814fbc90>] cpuidle_enter_tk+0x10/0x20 Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643057] [<ffffffff814fb8c9>] cpuidle_idle_call+0xa9/0x260 Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643058] [<ffffffff8101d45f>] cpu_idle+0xaf/0x120 Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643061] [<ffffffff81634522>] rest_init+0x72/0x80 Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643063] [<ffffffff81d00c40>] start_kernel+0x3d1/0x3de Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643065] [<ffffffff81d0066e>] ? repair_env_string+0x5e/0x5e Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643066] [<ffffffff81d00356>] x86_64_start_reservations+0x131/0x135 Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643068] [<ffffffff81d0045a>] x86_64_start_kernel+0x100/0x10f Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643068] ---[ end trace 5595a7f5dd9a2949 ]--- Signed-off-by: Parag Warudkar <parag.lkml@gmail.com> diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h index a56b241..b3088a1 100644 --- a/drivers/net/wireless/ath/ath9k/ath9k.h +++ b/drivers/net/wireless/ath/ath9k/ath9k.h @@ -764,6 +764,7 @@ struct ath_softc { atomic_t wow_got_bmiss_intr; atomic_t wow_sleep_proc_intr; /* in the middle of WoW sleep ? */ u32 wow_intr_before_sleep; + bool suspending; #endif }; diff --git a/drivers/net/wireless/ath/ath9k/link.c b/drivers/net/wireless/ath/ath9k/link.c index ade3afb..fa77e19 100644 --- a/drivers/net/wireless/ath/ath9k/link.c +++ b/drivers/net/wireless/ath/ath9k/link.c @@ -158,7 +158,8 @@ void ath_start_rx_poll(struct ath_softc *sc, u8 nbeacon) { if (!AR_SREV_9300(sc->sc_ah)) return; - + if (sc->suspending) + return; if (!test_bit(SC_OP_PRIM_STA_VIF, &sc->sc_flags)) return; diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c index 6e66f9c..0cf88b7 100644 --- a/drivers/net/wireless/ath/ath9k/main.c +++ b/drivers/net/wireless/ath/ath9k/main.c @@ -2150,7 +2150,7 @@ static int ath9k_suspend(struct ieee80211_hw *hw, int ret = 0; mutex_lock(&sc->mutex); - + sc->suspending = 1; ath_cancel_work(sc); ath_stop_ani(sc); del_timer_sync(&sc->rx_poll_timer); @@ -2288,6 +2288,7 @@ static int ath9k_resume(struct ieee80211_hw *hw) ath9k_start_btcoex(sc); ath9k_ps_restore(sc); + sc->suspending = 0; mutex_unlock(&sc->mutex); return 0; [-- Attachment #2: ath9k.patch --] [-- Type: application/octet-stream, Size: 1514 bytes --] Signed-off-by: Parag Warudkar <parag.lkml@gmail.com> diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h index a56b241..b3088a1 100644 --- a/drivers/net/wireless/ath/ath9k/ath9k.h +++ b/drivers/net/wireless/ath/ath9k/ath9k.h @@ -764,6 +764,7 @@ struct ath_softc { atomic_t wow_got_bmiss_intr; atomic_t wow_sleep_proc_intr; /* in the middle of WoW sleep ? */ u32 wow_intr_before_sleep; + bool suspending; #endif }; diff --git a/drivers/net/wireless/ath/ath9k/link.c b/drivers/net/wireless/ath/ath9k/link.c index ade3afb..fa77e19 100644 --- a/drivers/net/wireless/ath/ath9k/link.c +++ b/drivers/net/wireless/ath/ath9k/link.c @@ -158,7 +158,8 @@ void ath_start_rx_poll(struct ath_softc *sc, u8 nbeacon) { if (!AR_SREV_9300(sc->sc_ah)) return; - + if (sc->suspending) + return; if (!test_bit(SC_OP_PRIM_STA_VIF, &sc->sc_flags)) return; diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c index 6e66f9c..0cf88b7 100644 --- a/drivers/net/wireless/ath/ath9k/main.c +++ b/drivers/net/wireless/ath/ath9k/main.c @@ -2150,7 +2150,7 @@ static int ath9k_suspend(struct ieee80211_hw *hw, int ret = 0; mutex_lock(&sc->mutex); - + sc->suspending = 1; ath_cancel_work(sc); ath_stop_ani(sc); del_timer_sync(&sc->rx_poll_timer); @@ -2288,6 +2288,7 @@ static int ath9k_resume(struct ieee80211_hw *hw) ath9k_start_btcoex(sc); ath9k_ps_restore(sc); + sc->suspending = 0; mutex_unlock(&sc->mutex); return 0; ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] ath9k : Fix ieee80211 work while going to suspend 2013-03-16 16:38 [PATCH] ath9k : Fix ieee80211 work while going to suspend Parag Warudkar @ 2013-03-18 19:13 ` John W. Linville 2013-03-18 21:03 ` Luis R. Rodriguez 0 siblings, 1 reply; 9+ messages in thread From: John W. Linville @ 2013-03-18 19:13 UTC (permalink / raw) To: Parag Warudkar Cc: Luis R. Rodriguez, Jouni Malinen, Vasanthakumar Thiagarajan, linux-wireless, ath9k-devel, netdev, LKML, senthilb Any comments from the ath9k folks? On Sat, Mar 16, 2013 at 12:38:14PM -0400, Parag Warudkar wrote: > During suspend below warning is seen when ath9k is active. Attached > patch fixes the warning for me. Tested to work across few > suspend-resume cycles. > > > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.642939] WARNING: at > net/mac80211/util.c:599 ieee80211_can_queue_work.isra.7+0x32/0x40 > [mac80211]() > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.642940] Hardware name: iMac12,1 > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.642941] queueing ieee80211 > work while going to suspend > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.642972] Modules linked in: > joydev hid_logitech_dj zfs(POF) bridge stp llc zunicode(POF) zavl(POF) > zcommon(POF) znvpair(POF) spl(OF) zlib_deflate be2iscsi > iscsi_boot_sysfs bnx2i cnic uio cxgb4i cxgb4 cxgb3i cxgb3 mdio > libcxgbi ib_iser rdma_cm ib_addr iw_cm ib_cm ib_sa ib_mad ib_core > iscsi_tcp libiscsi_tcp libiscsi scsi_transport_iscsi rfcomm bnep > nls_utf8 hfsplus btusb uvcvideo videobuf2_vmalloc videobuf2_memops > videobuf2_core videodev bluetooth media coretemp snd_hda_codec_hdmi > snd_hda_codec_cirrus snd_hda_intel snd_hda_codec snd_hwdep snd_seq > snd_seq_device snd_pcm snd_page_alloc snd_timer snd soundcore arc4 > ath9k ath9k_common microcode ath9k_hw ath mac80211 iTCO_wdt > iTCO_vendor_support cfg80211 lpc_ich mei mfd_core rfkill i2c_i801 > applesmc apple_bl input_polldev vhost_net tun macvtap macvlan > kvm_intel kvm binfmt_misc uinput usb_storage crc32c_intel radeon ttm > i915 ghash_clmulni_intel firewire_ohci firewire_core i2c_algo_bit > drm_kms_helper crc_itu_t drm tg3 ptp pps_core i2c_core video sunrpc > [last unloaded: iptable_mangle] > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.642985] Pid: 0, comm: > swapper/0 Tainted: PF O 3.8.2-206.fc18.x86_64 #1 > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.642986] Call Trace: > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.642992] <IRQ> > [<ffffffff8105e61f>] warn_slowpath_common+0x7f/0xc0 > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643009] > [<ffffffffa0581420>] ? ath_start_rx_poll+0x70/0x70 [ath9k] > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643011] > [<ffffffff8105e716>] warn_slowpath_fmt+0x46/0x50 > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643018] > [<ffffffffa045b542>] ieee80211_can_queue_work.isra.7+0x32/0x40 > [mac80211] > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643024] > [<ffffffffa045b5de>] ieee80211_queue_work+0x2e/0x50 [mac80211] > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643027] > [<ffffffffa0581438>] ath_rx_poll+0x18/0x20 [ath9k] > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643029] > [<ffffffff8106d0aa>] call_timer_fn+0x3a/0x120 > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643032] > [<ffffffffa0581420>] ? ath_start_rx_poll+0x70/0x70 [ath9k] > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643034] > [<ffffffff814fb240>] ? cpufreq_p4_target+0x120/0x120 > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643035] > [<ffffffff8106ee3e>] run_timer_softirq+0x1fe/0x2b0 > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643037] > [<ffffffff814fb240>] ? cpufreq_p4_target+0x120/0x120 > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643038] > [<ffffffff81066cd0>] __do_softirq+0xd0/0x210 > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643040] > [<ffffffff8101b913>] ? native_sched_clock+0x13/0x80 > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643041] > [<ffffffff814fb240>] ? cpufreq_p4_target+0x120/0x120 > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643043] > [<ffffffff8165985c>] call_softirq+0x1c/0x30 > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643045] > [<ffffffff810162a5>] do_softirq+0x75/0xb0 > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643046] > [<ffffffff81066fa5>] irq_exit+0xb5/0xc0 > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643048] > [<ffffffff8165a1de>] smp_apic_timer_interrupt+0x6e/0x99 > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643049] > [<ffffffff8165911d>] apic_timer_interrupt+0x6d/0x80 > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643052] <EOI> > [<ffffffff810868ce>] ? __hrtimer_start_range_ns+0x1be/0x400 > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643053] > [<ffffffff814fbc30>] ? cpuidle_wrap_enter+0x50/0xa0 > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643054] > [<ffffffff814fbc29>] ? cpuidle_wrap_enter+0x49/0xa0 > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643056] > [<ffffffff814fbc90>] cpuidle_enter_tk+0x10/0x20 > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643057] > [<ffffffff814fb8c9>] cpuidle_idle_call+0xa9/0x260 > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643058] > [<ffffffff8101d45f>] cpu_idle+0xaf/0x120 > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643061] > [<ffffffff81634522>] rest_init+0x72/0x80 > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643063] > [<ffffffff81d00c40>] start_kernel+0x3d1/0x3de > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643065] > [<ffffffff81d0066e>] ? repair_env_string+0x5e/0x5e > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643066] > [<ffffffff81d00356>] x86_64_start_reservations+0x131/0x135 > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643068] > [<ffffffff81d0045a>] x86_64_start_kernel+0x100/0x10f > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643068] ---[ end trace > 5595a7f5dd9a2949 ]--- > > Signed-off-by: Parag Warudkar <parag.lkml@gmail.com> > > diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h > b/drivers/net/wireless/ath/ath9k/ath9k.h > index a56b241..b3088a1 100644 > --- a/drivers/net/wireless/ath/ath9k/ath9k.h > +++ b/drivers/net/wireless/ath/ath9k/ath9k.h > @@ -764,6 +764,7 @@ struct ath_softc { > atomic_t wow_got_bmiss_intr; > atomic_t wow_sleep_proc_intr; /* in the middle of WoW sleep ? */ > u32 wow_intr_before_sleep; > + bool suspending; > #endif > }; > > diff --git a/drivers/net/wireless/ath/ath9k/link.c > b/drivers/net/wireless/ath/ath9k/link.c > index ade3afb..fa77e19 100644 > --- a/drivers/net/wireless/ath/ath9k/link.c > +++ b/drivers/net/wireless/ath/ath9k/link.c > @@ -158,7 +158,8 @@ void ath_start_rx_poll(struct ath_softc *sc, u8 nbeacon) > { > if (!AR_SREV_9300(sc->sc_ah)) > return; > - > + if (sc->suspending) > + return; > if (!test_bit(SC_OP_PRIM_STA_VIF, &sc->sc_flags)) > return; > > diff --git a/drivers/net/wireless/ath/ath9k/main.c > b/drivers/net/wireless/ath/ath9k/main.c > index 6e66f9c..0cf88b7 100644 > --- a/drivers/net/wireless/ath/ath9k/main.c > +++ b/drivers/net/wireless/ath/ath9k/main.c > @@ -2150,7 +2150,7 @@ static int ath9k_suspend(struct ieee80211_hw *hw, > int ret = 0; > > mutex_lock(&sc->mutex); > - > + sc->suspending = 1; > ath_cancel_work(sc); > ath_stop_ani(sc); > del_timer_sync(&sc->rx_poll_timer); > @@ -2288,6 +2288,7 @@ static int ath9k_resume(struct ieee80211_hw *hw) > ath9k_start_btcoex(sc); > > ath9k_ps_restore(sc); > + sc->suspending = 0; > mutex_unlock(&sc->mutex); > > return 0; -- John W. Linville Someday the world will need a hero, and you linville@tuxdriver.com might be all we have. Be ready. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ath9k : Fix ieee80211 work while going to suspend 2013-03-18 19:13 ` John W. Linville @ 2013-03-18 21:03 ` Luis R. Rodriguez 2013-03-19 1:02 ` Parag Warudkar 2013-03-21 11:42 ` Stanislaw Gruszka 0 siblings, 2 replies; 9+ messages in thread From: Luis R. Rodriguez @ 2013-03-18 21:03 UTC (permalink / raw) To: John W. Linville Cc: Parag Warudkar, Luis R. Rodriguez, Jouni Malinen, Vasanthakumar Thiagarajan, linux-wireless, ath9k-devel, netdev, LKML, senthilb On Mon, Mar 18, 2013 at 03:13:41PM -0400, John W. Linville wrote: > Any comments from the ath9k folks? > > On Sat, Mar 16, 2013 at 12:38:14PM -0400, Parag Warudkar wrote: > > During suspend below warning is seen when ath9k is active. Attached > > patch fixes the warning for me. Tested to work across few > > suspend-resume cycles. > > > > > > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.642939] WARNING: at > > net/mac80211/util.c:599 ieee80211_can_queue_work.isra.7+0x32/0x40 > > [mac80211]() > > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.642940] Hardware name: iMac12,1 > > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.642941] queueing ieee80211 > > work while going to suspend > > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.642972] Modules linked in: > > joydev hid_logitech_dj zfs(POF) bridge stp llc zunicode(POF) zavl(POF) > > zcommon(POF) znvpair(POF) spl(OF) zlib_deflate be2iscsi > > iscsi_boot_sysfs bnx2i cnic uio cxgb4i cxgb4 cxgb3i cxgb3 mdio > > libcxgbi ib_iser rdma_cm ib_addr iw_cm ib_cm ib_sa ib_mad ib_core > > iscsi_tcp libiscsi_tcp libiscsi scsi_transport_iscsi rfcomm bnep > > nls_utf8 hfsplus btusb uvcvideo videobuf2_vmalloc videobuf2_memops > > videobuf2_core videodev bluetooth media coretemp snd_hda_codec_hdmi > > snd_hda_codec_cirrus snd_hda_intel snd_hda_codec snd_hwdep snd_seq > > snd_seq_device snd_pcm snd_page_alloc snd_timer snd soundcore arc4 > > ath9k ath9k_common microcode ath9k_hw ath mac80211 iTCO_wdt > > iTCO_vendor_support cfg80211 lpc_ich mei mfd_core rfkill i2c_i801 > > applesmc apple_bl input_polldev vhost_net tun macvtap macvlan > > kvm_intel kvm binfmt_misc uinput usb_storage crc32c_intel radeon ttm > > i915 ghash_clmulni_intel firewire_ohci firewire_core i2c_algo_bit > > drm_kms_helper crc_itu_t drm tg3 ptp pps_core i2c_core video sunrpc > > [last unloaded: iptable_mangle] > > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.642985] Pid: 0, comm: > > swapper/0 Tainted: PF O 3.8.2-206.fc18.x86_64 #1 > > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.642986] Call Trace: > > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.642992] <IRQ> > > [<ffffffff8105e61f>] warn_slowpath_common+0x7f/0xc0 > > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643009] > > [<ffffffffa0581420>] ? ath_start_rx_poll+0x70/0x70 [ath9k] > > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643011] > > [<ffffffff8105e716>] warn_slowpath_fmt+0x46/0x50 > > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643018] > > [<ffffffffa045b542>] ieee80211_can_queue_work.isra.7+0x32/0x40 > > [mac80211] > > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643024] > > [<ffffffffa045b5de>] ieee80211_queue_work+0x2e/0x50 [mac80211] > > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643027] > > [<ffffffffa0581438>] ath_rx_poll+0x18/0x20 [ath9k] > > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643029] > > [<ffffffff8106d0aa>] call_timer_fn+0x3a/0x120 > > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643032] > > [<ffffffffa0581420>] ? ath_start_rx_poll+0x70/0x70 [ath9k] > > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643034] > > [<ffffffff814fb240>] ? cpufreq_p4_target+0x120/0x120 > > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643035] > > [<ffffffff8106ee3e>] run_timer_softirq+0x1fe/0x2b0 > > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643037] > > [<ffffffff814fb240>] ? cpufreq_p4_target+0x120/0x120 > > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643038] > > [<ffffffff81066cd0>] __do_softirq+0xd0/0x210 > > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643040] > > [<ffffffff8101b913>] ? native_sched_clock+0x13/0x80 > > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643041] > > [<ffffffff814fb240>] ? cpufreq_p4_target+0x120/0x120 > > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643043] > > [<ffffffff8165985c>] call_softirq+0x1c/0x30 > > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643045] > > [<ffffffff810162a5>] do_softirq+0x75/0xb0 > > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643046] > > [<ffffffff81066fa5>] irq_exit+0xb5/0xc0 > > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643048] > > [<ffffffff8165a1de>] smp_apic_timer_interrupt+0x6e/0x99 > > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643049] > > [<ffffffff8165911d>] apic_timer_interrupt+0x6d/0x80 > > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643052] <EOI> > > [<ffffffff810868ce>] ? __hrtimer_start_range_ns+0x1be/0x400 > > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643053] > > [<ffffffff814fbc30>] ? cpuidle_wrap_enter+0x50/0xa0 > > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643054] > > [<ffffffff814fbc29>] ? cpuidle_wrap_enter+0x49/0xa0 > > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643056] > > [<ffffffff814fbc90>] cpuidle_enter_tk+0x10/0x20 > > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643057] > > [<ffffffff814fb8c9>] cpuidle_idle_call+0xa9/0x260 > > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643058] > > [<ffffffff8101d45f>] cpu_idle+0xaf/0x120 > > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643061] > > [<ffffffff81634522>] rest_init+0x72/0x80 > > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643063] > > [<ffffffff81d00c40>] start_kernel+0x3d1/0x3de > > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643065] > > [<ffffffff81d0066e>] ? repair_env_string+0x5e/0x5e > > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643066] > > [<ffffffff81d00356>] x86_64_start_reservations+0x131/0x135 > > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643068] > > [<ffffffff81d0045a>] x86_64_start_kernel+0x100/0x10f > > Mar 16 09:39:17 Parags-iMac kernel: [ 3993.643068] ---[ end trace > > 5595a7f5dd9a2949 ]--- > > > > Signed-off-by: Parag Warudkar <parag.lkml@gmail.com> > > > > diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h > > b/drivers/net/wireless/ath/ath9k/ath9k.h > > index a56b241..b3088a1 100644 > > --- a/drivers/net/wireless/ath/ath9k/ath9k.h > > +++ b/drivers/net/wireless/ath/ath9k/ath9k.h > > @@ -764,6 +764,7 @@ struct ath_softc { > > atomic_t wow_got_bmiss_intr; > > atomic_t wow_sleep_proc_intr; /* in the middle of WoW sleep ? */ > > u32 wow_intr_before_sleep; > > + bool suspending; > > #endif > > }; > > > > diff --git a/drivers/net/wireless/ath/ath9k/link.c > > b/drivers/net/wireless/ath/ath9k/link.c > > index ade3afb..fa77e19 100644 > > --- a/drivers/net/wireless/ath/ath9k/link.c > > +++ b/drivers/net/wireless/ath/ath9k/link.c > > @@ -158,7 +158,8 @@ void ath_start_rx_poll(struct ath_softc *sc, u8 nbeacon) > > { > > if (!AR_SREV_9300(sc->sc_ah)) > > return; > > - > > + if (sc->suspending) > > + return; Thanks for the patch! Please note the style issue here, you should use a tab, but other than that lets review what happened. > > if (!test_bit(SC_OP_PRIM_STA_VIF, &sc->sc_flags)) > > return; Note that what this will do is call later mod_timer() for rx_poll_timer, the right thing to do then, which would be equivalent to your patch is to modify the ath_start_rx_poll() to instead use the new API mod_timer_pending() added on v2.6.30 via commit 74019224. This would not re-arm the timer if it was previously removed. commit 74019224ac34b044b44a31dd89a54e3477db4896 Author: Ingo Molnar <mingo@elte.hu> Date: Wed Feb 18 12:23:29 2009 +0100 timers: add mod_timer_pending() Impact: new timer API Based on an idea from Martin Josefsson with the help of Patrick McHardy and Stephen Hemminger: introduce the mod_timer_pending() API which is a mod_timer() offspring that is an invariant on already removed timers. (regular mod_timer() re-activates non-pending timers.) This is useful for the networking code in that it can allow unserialized mod_timer_pending() timer-forwarding calls, but a single del_timer*() will stop the timer from being reactivated again. Also while at it: - optimize the regular mod_timer() path some more, the timer-stat and a debug check was needlessly duplicated in __mod_timer(). - make the exports come straight after the function, as most other exports in timer.c already did. - eliminate __mod_timer() as an external API, change the users to mod_timer(). The regular mod_timer() code path is not impacted significantly, due to inlining optimizations and due to the simplifications. Based-on-patch-from: Stephen Hemminger <shemminger@vyatta.com> Acked-by: Stephen Hemminger <shemminger@vyatta.com> Cc: "David S. Miller" <davem@davemloft.net> Cc: Patrick McHardy <kaber@trash.net> Cc: netdev@vger.kernel.org Cc: Oleg Nesterov <oleg@redhat.com> Cc: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Ingo Molnar <mingo@elte.hu> > > > > diff --git a/drivers/net/wireless/ath/ath9k/main.c > > b/drivers/net/wireless/ath/ath9k/main.c > > index 6e66f9c..0cf88b7 100644 > > --- a/drivers/net/wireless/ath/ath9k/main.c > > +++ b/drivers/net/wireless/ath/ath9k/main.c > > @@ -2150,7 +2150,7 @@ static int ath9k_suspend(struct ieee80211_hw *hw, > > int ret = 0; > > > > mutex_lock(&sc->mutex); > > - > > + sc->suspending = 1; > > ath_cancel_work(sc); > > ath_stop_ani(sc); > > del_timer_sync(&sc->rx_poll_timer); See here we use del_timer_sync(). Can you try this instead for example: diff --git a/drivers/net/wireless/ath/ath9k/link.c b/drivers/net/wireless/ath/ath9k/link.c index ade3afb..fbd8b4c 100644 --- a/drivers/net/wireless/ath/ath9k/link.c +++ b/drivers/net/wireless/ath/ath9k/link.c @@ -162,8 +162,8 @@ void ath_start_rx_poll(struct ath_softc *sc, u8 nbeacon) if (!test_bit(SC_OP_PRIM_STA_VIF, &sc->sc_flags)) return; - mod_timer(&sc->rx_poll_timer, jiffies + msecs_to_jiffies - (nbeacon * sc->cur_beacon_conf.beacon_interval)); + mod_timer_pending(&sc->rx_poll_timer, jiffies + msecs_to_jiffies + (nbeacon * sc->cur_beacon_conf.beacon_interval)); } void ath_rx_poll(unsigned long data) Looking at this makes me think we should review all usage of mod_timer all over our 802.11 drivers, and mac80211, cfg80211 as well. Luis ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] ath9k : Fix ieee80211 work while going to suspend 2013-03-18 21:03 ` Luis R. Rodriguez @ 2013-03-19 1:02 ` Parag Warudkar 2013-03-21 11:42 ` Stanislaw Gruszka 1 sibling, 0 replies; 9+ messages in thread From: Parag Warudkar @ 2013-03-19 1:02 UTC (permalink / raw) To: Luis R. Rodriguez Cc: John W. Linville, Parag Warudkar, Jouni Malinen, Vasanthakumar Thiagarajan, linux-wireless, ath9k-devel, netdev, LKML, senthilb On Mon, 18 Mar 2013, Luis R. Rodriguez wrote: > > Note that what this will do is call later mod_timer() for > rx_poll_timer, the right thing to do then, which would > be equivalent to your patch is to modify the ath_start_rx_poll() > to instead use the new API mod_timer_pending() added on v2.6.30 > via commit 74019224. This would not re-arm the timer if it was > previously removed. Thanks for the details Luis. Converting to mod_timer_pending() seems to do the trick as well. Parag ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ath9k : Fix ieee80211 work while going to suspend 2013-03-18 21:03 ` Luis R. Rodriguez 2013-03-19 1:02 ` Parag Warudkar @ 2013-03-21 11:42 ` Stanislaw Gruszka 2013-03-21 19:33 ` Luis R. Rodriguez 1 sibling, 1 reply; 9+ messages in thread From: Stanislaw Gruszka @ 2013-03-21 11:42 UTC (permalink / raw) To: Luis R. Rodriguez Cc: John W. Linville, Parag Warudkar, Jouni Malinen, Vasanthakumar Thiagarajan, linux-wireless, ath9k-devel, netdev, LKML, senthilb On Mon, Mar 18, 2013 at 02:03:08PM -0700, Luis R. Rodriguez wrote: > > > --- a/drivers/net/wireless/ath/ath9k/link.c > > > +++ b/drivers/net/wireless/ath/ath9k/link.c > > > @@ -158,7 +158,8 @@ void ath_start_rx_poll(struct ath_softc *sc, u8 nbeacon) > > > { > > > if (!AR_SREV_9300(sc->sc_ah)) > > > return; > > > - > > > + if (sc->suspending) > > > + return; > > Thanks for the patch! Please note the style issue here, you should > use a tab, but other than that lets review what happened. > > > > if (!test_bit(SC_OP_PRIM_STA_VIF, &sc->sc_flags)) > > > return; > > Note that what this will do is call later mod_timer() for > rx_poll_timer, the right thing to do then, which would > be equivalent to your patch is to modify the ath_start_rx_poll() > to instead use the new API mod_timer_pending() added on v2.6.30 > via commit 74019224. This would not re-arm the timer if it was > previously removed. [snip] > - mod_timer(&sc->rx_poll_timer, jiffies + msecs_to_jiffies > - (nbeacon * sc->cur_beacon_conf.beacon_interval)); > + mod_timer_pending(&sc->rx_poll_timer, jiffies + msecs_to_jiffies > + (nbeacon * sc->cur_beacon_conf.beacon_interval)); But isn't this prevent to run timer in case it was not running, but we want to start it ? > Looking at this makes me think we should review all usage of > mod_timer all over our 802.11 drivers, and mac80211, cfg80211 as > well. I mac80211 we use local->suspended and local->quiesce booleans to prevent reschedule of timers when going to suspend for example. Works use ifmgd->associted to prevent reschedule when we are disassociating. I think on ath9k also some boolean variable should be used, not only for rx_poll_timer but also for other works i.e. tx_complete_work. Is possible to use SC_OP_INVALID flags, since mac80211 call ath9k_stop on suspend and ath9k_start on resume. Stanislaw ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ath9k : Fix ieee80211 work while going to suspend 2013-03-21 11:42 ` Stanislaw Gruszka @ 2013-03-21 19:33 ` Luis R. Rodriguez 2013-03-22 9:13 ` Stanislaw Gruszka 0 siblings, 1 reply; 9+ messages in thread From: Luis R. Rodriguez @ 2013-03-21 19:33 UTC (permalink / raw) To: Stanislaw Gruszka Cc: John W. Linville, Parag Warudkar, Jouni Malinen, Vasanthakumar Thiagarajan, linux-wireless, ath9k-devel, netdev, LKML, senthilb On Thu, Mar 21, 2013 at 12:42:20PM +0100, Stanislaw Gruszka wrote: > On Mon, Mar 18, 2013 at 02:03:08PM -0700, Luis R. Rodriguez wrote: > > > > --- a/drivers/net/wireless/ath/ath9k/link.c > > > > +++ b/drivers/net/wireless/ath/ath9k/link.c > > > > @@ -158,7 +158,8 @@ void ath_start_rx_poll(struct ath_softc *sc, u8 nbeacon) > > > > { > > > > if (!AR_SREV_9300(sc->sc_ah)) > > > > return; > > > > - > > > > + if (sc->suspending) > > > > + return; > > > > Thanks for the patch! Please note the style issue here, you should > > use a tab, but other than that lets review what happened. > > > > > > if (!test_bit(SC_OP_PRIM_STA_VIF, &sc->sc_flags)) > > > > return; > > > > Note that what this will do is call later mod_timer() for > > rx_poll_timer, the right thing to do then, which would > > be equivalent to your patch is to modify the ath_start_rx_poll() > > to instead use the new API mod_timer_pending() added on v2.6.30 > > via commit 74019224. This would not re-arm the timer if it was > > previously removed. > [snip] > > - mod_timer(&sc->rx_poll_timer, jiffies + msecs_to_jiffies > > - (nbeacon * sc->cur_beacon_conf.beacon_interval)); > > + mod_timer_pending(&sc->rx_poll_timer, jiffies + msecs_to_jiffies > > + (nbeacon * sc->cur_beacon_conf.beacon_interval)); > > But isn't this prevent to run timer in case it was not running, but > we want to start it ? No you're right, this would never have it run, sorry about that. But lets look at this a little closer. The issue is at suspend time, and the issue is ath9k is trying to schedule work while going to suspend. So when does this work get called? Given the trace this is hit when the timer rx_poll_timer runs, which in turn calls ath_rx_poll() to schedule hw_check_work work. The rx_poll_timer however was originally only set at the end of the routine that hw_check_work sets off but also at other entry points (ath_start_rx_poll() callers). Once ath_start_rx_poll() gets called though we can go on looping as follows: work timer work hw_check_work --> rx_poll_timer --> hw_check_work At suspend time we do this though: ath_cancel_work(sc); del_timer_sync(&sc->rx_poll_timer); So perhaps what we need is: diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c index 6e66f9c..42bb9ea 100644 --- a/drivers/net/wireless/ath/ath9k/main.c +++ b/drivers/net/wireless/ath/ath9k/main.c @@ -2151,9 +2152,9 @@ static int ath9k_suspend(struct ieee80211_hw *hw, mutex_lock(&sc->mutex); + del_timer_sync(&sc->rx_poll_timer); ath_cancel_work(sc); ath_stop_ani(sc); - del_timer_sync(&sc->rx_poll_timer); if (test_bit(SC_OP_INVALID, &sc->sc_flags)) { ath_dbg(common, ANY, "Device not present\n"); The less changes needed to fix the issue for stable the easier to backport. Technically though then we'd need all of these changes as well then: diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c index 6e66f9c..42bb9ea 100644 --- a/drivers/net/wireless/ath/ath9k/main.c +++ b/drivers/net/wireless/ath/ath9k/main.c @@ -752,8 +752,8 @@ static void ath9k_stop(struct ieee80211_hw *hw) mutex_lock(&sc->mutex); - ath_cancel_work(sc); del_timer_sync(&sc->rx_poll_timer); + ath_cancel_work(sc); if (test_bit(SC_OP_INVALID, &sc->sc_flags)) { ath_dbg(common, ANY, "Device not present\n"); @@ -1145,6 +1145,7 @@ static int ath9k_config(struct ieee80211_hw *hw, u32 changed) if (changed & IEEE80211_CONF_CHANGE_IDLE) { sc->ps_idle = !!(conf->flags & IEEE80211_CONF_IDLE); if (sc->ps_idle) { + del_timer_sync(&sc->rx_poll_timer); ath_cancel_work(sc); ath9k_stop_btcoex(sc); } else { @@ -2151,9 +2152,9 @@ static int ath9k_suspend(struct ieee80211_hw *hw, mutex_lock(&sc->mutex); + del_timer_sync(&sc->rx_poll_timer); ath_cancel_work(sc); ath_stop_ani(sc); - del_timer_sync(&sc->rx_poll_timer); if (test_bit(SC_OP_INVALID, &sc->sc_flags)) { ath_dbg(common, ANY, "Device not present\n"); But then we have the chicken and the egg problem, as the work item could fire off the timer so it would seem to be good to prevent adding new work when suspending. > > Looking at this makes me think we should review all usage of > > mod_timer all over our 802.11 drivers, and mac80211, cfg80211 as > > well. > > In mac80211 we use local->suspended and local->quiesce booleans to > prevent reschedule of timers when going to suspend for example. > Works use ifmgd->associted to prevent reschedule when we are > disassociating. > > I think on ath9k also some boolean variable should be used, not only > for rx_poll_timer but also for other works i.e. tx_complete_work. > Is possible to use SC_OP_INVALID flags, since mac80211 call ath9k_stop > on suspend and ath9k_start on resume. Indeed however ieee80211_queue_work() already does a suspend check for us, it just complains as many drivers including mac80211 were setting up work incorrectly. The warning was put in place to help us find the issues. Using SC_OP_INVALID seems fair but we could also add a routine ieee80211_queue_work_safe() that silently fails if we are quiescing or suspended and not resuming but I can see that creating very sloppy driver writing and everyone abusing it. OK how about this for stable for now: diff --git a/drivers/net/wireless/ath/ath9k/link.c b/drivers/net/wireless/ath/ath9k/link.c index 39c84ec..7fdac6c 100644 --- a/drivers/net/wireless/ath/ath9k/link.c +++ b/drivers/net/wireless/ath/ath9k/link.c @@ -170,7 +170,8 @@ void ath_rx_poll(unsigned long data) { struct ath_softc *sc = (struct ath_softc *)data; - ieee80211_queue_work(sc->hw, &sc->hw_check_work); + if (!test_bit(SC_OP_INVALID, &sc->sc_flags)) + ieee80211_queue_work(sc->hw, &sc->hw_check_work); } /* Luis ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH] ath9k : Fix ieee80211 work while going to suspend 2013-03-21 19:33 ` Luis R. Rodriguez @ 2013-03-22 9:13 ` Stanislaw Gruszka 2013-03-22 15:08 ` Luis R. Rodriguez 0 siblings, 1 reply; 9+ messages in thread From: Stanislaw Gruszka @ 2013-03-22 9:13 UTC (permalink / raw) To: Luis R. Rodriguez Cc: John W. Linville, Parag Warudkar, Jouni Malinen, Vasanthakumar Thiagarajan, linux-wireless-u79uwXL29TY76Z2rM5mHXA, ath9k-devel-juf53994utBLZpfksSYvnA, netdev-u79uwXL29TY76Z2rM5mHXA, LKML, senthilb-A+ZNKFmMK5xy9aJCnZT0Uw On Thu, Mar 21, 2013 at 12:33:31PM -0700, Luis R. Rodriguez wrote: > So when does this work get called? > > Given the trace this is hit when the timer rx_poll_timer runs, > which in turn calls ath_rx_poll() to schedule hw_check_work work. > The rx_poll_timer however was originally only set at the end of > the routine that hw_check_work sets off but also at other entry > points (ath_start_rx_poll() callers). Once ath_start_rx_poll() > gets called though we can go on looping as follows: > > work timer work > hw_check_work --> rx_poll_timer --> hw_check_work > > At suspend time we do this though: > > ath_cancel_work(sc); > del_timer_sync(&sc->rx_poll_timer); > > + del_timer_sync(&sc->rx_poll_timer); > ath_cancel_work(sc); > ath_stop_ani(sc); > - del_timer_sync(&sc->rx_poll_timer); [snip] > But then we have the chicken and the egg problem, as the work item > could fire off the timer so it would seem to be good to prevent > adding new work when suspending. Yup, this is egg and chicken problem, I think it can not be fixed by changing ordering of del_timer and cancel_work, it would be like: del_timer_sync(&sc->rx_poll_timer); /* but timer could schedule work */ ath_cancel_work(sc); /* but work could schedule timer */ del_timer_sync(&sc->rx_poll_timer); /* but timer could schedule work */ ath_cancel_work(sc); /* And so on ... */ Check is needed in work or timer callback, depending what is canceled last, to fix the problem ... > > In mac80211 we use local->suspended and local->quiesce booleans to > > prevent reschedule of timers when going to suspend for example. > > Works use ifmgd->associted to prevent reschedule when we are > > disassociating. > > > > I think on ath9k also some boolean variable should be used, not only > > for rx_poll_timer but also for other works i.e. tx_complete_work. > > Is possible to use SC_OP_INVALID flags, since mac80211 call ath9k_stop > > on suspend and ath9k_start on resume. > > Indeed however ieee80211_queue_work() already does a suspend check for > us, it just complains as many drivers including mac80211 were setting > up work incorrectly. The warning was put in place to help us find the > issues. Using SC_OP_INVALID seems fair but we could also add a routine > ieee80211_queue_work_safe() that silently fails if we are quiescing or > suspended and not resuming but I can see that creating very sloppy > driver writing and everyone abusing it. We also have to reliable cancel works on ath9k_stop() if device goes down for other reason than suspend, new mac80211 ieee80211_queue_work_safe() routine will not help with that. > OK how about this for stable for now: > > diff --git a/drivers/net/wireless/ath/ath9k/link.c b/drivers/net/wireless/ath/ath9k/link.c > index 39c84ec..7fdac6c 100644 > --- a/drivers/net/wireless/ath/ath9k/link.c > +++ b/drivers/net/wireless/ath/ath9k/link.c > @@ -170,7 +170,8 @@ void ath_rx_poll(unsigned long data) > { > struct ath_softc *sc = (struct ath_softc *)data; > > - ieee80211_queue_work(sc->hw, &sc->hw_check_work); > + if (!test_bit(SC_OP_INVALID, &sc->sc_flags)) > + ieee80211_queue_work(sc->hw, &sc->hw_check_work); > } That looks ok for me as -stable fix Reviewed-by: Stanislaw Gruszka <sgruszka-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> Stanislaw -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ath9k : Fix ieee80211 work while going to suspend 2013-03-22 9:13 ` Stanislaw Gruszka @ 2013-03-22 15:08 ` Luis R. Rodriguez 2013-03-24 16:16 ` Parag Warudkar 0 siblings, 1 reply; 9+ messages in thread From: Luis R. Rodriguez @ 2013-03-22 15:08 UTC (permalink / raw) To: Stanislaw Gruszka Cc: John W. Linville, Parag Warudkar, Jouni Malinen, Vasanthakumar Thiagarajan, linux-wireless, ath9k-devel, netdev, LKML, senthilb On Fri, Mar 22, 2013 at 10:13:42AM +0100, Stanislaw Gruszka wrote: > On Thu, Mar 21, 2013 at 12:33:31PM -0700, Luis R. Rodriguez wrote: > > OK how about this for stable for now: > > > > diff --git a/drivers/net/wireless/ath/ath9k/link.c b/drivers/net/wireless/ath/ath9k/link.c > > index 39c84ec..7fdac6c 100644 > > --- a/drivers/net/wireless/ath/ath9k/link.c > > +++ b/drivers/net/wireless/ath/ath9k/link.c > > @@ -170,7 +170,8 @@ void ath_rx_poll(unsigned long data) > > { > > struct ath_softc *sc = (struct ath_softc *)data; > > > > - ieee80211_queue_work(sc->hw, &sc->hw_check_work); > > + if (!test_bit(SC_OP_INVALID, &sc->sc_flags)) > > + ieee80211_queue_work(sc->hw, &sc->hw_check_work); > > } > > That looks ok for me as -stable fix > > Reviewed-by: Stanislaw Gruszka <sgruszka@redhat.com> Parag, can you test the above to ensure it fixes your issue ? Luis ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH] ath9k : Fix ieee80211 work while going to suspend 2013-03-22 15:08 ` Luis R. Rodriguez @ 2013-03-24 16:16 ` Parag Warudkar 0 siblings, 0 replies; 9+ messages in thread From: Parag Warudkar @ 2013-03-24 16:16 UTC (permalink / raw) To: Luis R. Rodriguez Cc: Stanislaw Gruszka, John W. Linville, Jouni Malinen, Vasanthakumar Thiagarajan, linux-wireless, ath9k-devel, netdev, LKML, senthilb On Fri, Mar 22, 2013 at 11:08 AM, Luis R. Rodriguez <rodrigue@qca.qualcomm.com> wrote: > On Fri, Mar 22, 2013 at 10:13:42AM +0100, Stanislaw Gruszka wrote: >> On Thu, Mar 21, 2013 at 12:33:31PM -0700, Luis R. Rodriguez wrote: >> > OK how about this for stable for now: >> > >> > diff --git a/drivers/net/wireless/ath/ath9k/link.c b/drivers/net/wireless/ath/ath9k/link.c >> > index 39c84ec..7fdac6c 100644 >> > --- a/drivers/net/wireless/ath/ath9k/link.c >> > +++ b/drivers/net/wireless/ath/ath9k/link.c >> > @@ -170,7 +170,8 @@ void ath_rx_poll(unsigned long data) >> > { >> > struct ath_softc *sc = (struct ath_softc *)data; >> > >> > - ieee80211_queue_work(sc->hw, &sc->hw_check_work); >> > + if (!test_bit(SC_OP_INVALID, &sc->sc_flags)) >> > + ieee80211_queue_work(sc->hw, &sc->hw_check_work); >> > } >> >> That looks ok for me as -stable fix >> >> Reviewed-by: Stanislaw Gruszka <sgruszka@redhat.com> > > Parag, can you test the above to ensure it fixes your issue ? Seems to be working ok so far. Reported-and-tested-by: Parag Warudkar <parag.lkml@gmail.com> ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2013-03-24 16:16 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2013-03-16 16:38 [PATCH] ath9k : Fix ieee80211 work while going to suspend Parag Warudkar 2013-03-18 19:13 ` John W. Linville 2013-03-18 21:03 ` Luis R. Rodriguez 2013-03-19 1:02 ` Parag Warudkar 2013-03-21 11:42 ` Stanislaw Gruszka 2013-03-21 19:33 ` Luis R. Rodriguez 2013-03-22 9:13 ` Stanislaw Gruszka 2013-03-22 15:08 ` Luis R. Rodriguez 2013-03-24 16:16 ` Parag Warudkar
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).