Skip to content

Commit 39d501d

Browse files
Stanislaw Gruszkanbd168
Stanislaw Gruszka
authored and
nbd168
committed
mt76usb: fix tx/rx stop
Disabling tasklets on stopping rx/tx is wrong. If blocked tasklet is scheduled and we remove device we will get 100% cpu usage: PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ COMMAND 9 root 20 0 0 0 0 R 93.8 0.0 1:47.19 ksoftirqd/0 by infinite loop in tasklet_action_common() and eventuall crash on next mt76usb module load: [ 2068.591964] RIP: 0010:tasklet_action_common.isra.17+0x66/0x100 [ 2068.591966] Code: 41 89 f5 eb 25 f0 48 0f ba 33 00 0f 83 b1 00 00 00 48 8b 7a 20 48 8b 42 18 e8 56 a3 b5 00 f0 80 23 fd 48 89 ea 48 85 ed 74 53 <48> 8b 2a 48 8d 5a 08 f0 48 0f ba 6a 08 01 72 0b 8b 42 10 85 c0 74 [ 2068.591968] RSP: 0018:ffff98758c34be58 EFLAGS: 00010206 [ 2068.591969] RAX: ffff98758e6966d0 RBX: ffff98756e69aef8 RCX: 0000000000000006 [ 2068.591970] RDX: 01060a053d060305 RSI: 0000000000000006 RDI: ffff98758e6966d0 [ 2068.591971] RBP: 01060a053d060305 R08: 0000000000000000 R09: 00000000000203c0 [ 2068.591971] R10: 000003ff65b34f08 R11: 0000000000000001 R12: ffff98758e6966d0 [ 2068.591972] R13: 0000000000000006 R14: 0000000000000040 R15: 0000000000000006 [ 2068.591974] FS: 0000000000000000(0000) GS:ffff98758e680000(0000) knlGS:0000000000000000 [ 2068.591975] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 2068.591975] CR2: 00002c5f73a6cc20 CR3: 00000002f920a001 CR4: 00000000003606e0 [ 2068.591977] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 2068.591978] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 [ 2068.591978] Call Trace: [ 2068.591985] __do_softirq+0xe3/0x30a [ 2068.591989] ? sort_range+0x20/0x20 [ 2068.591990] run_ksoftirqd+0x26/0x40 [ 2068.591992] smpboot_thread_fn+0xc5/0x160 [ 2068.591995] kthread+0x112/0x130 [ 2068.591997] ? kthread_create_on_node+0x40/0x40 [ 2068.591998] ret_from_fork+0x35/0x40 [ 2068.591999] Modules linked in: ccm arc4 fuse rfcomm cmac bnep sunrpc snd_hda_codec_hdmi snd_soc_skl snd_soc_core snd_soc_acpi_intel_match snd_hda_codec_realtek snd_soc_acpi snd_hda_codec_generic snd_soc_skl_ipc snd_soc_sst_ipc snd_soc_sst_dsp snd_hda_ext_core iTCO_wdt snd_hda_intel intel_rapl iTCO_vendor_support x86_pkg_temp_thermal intel_powerclamp btusb mei_wdt coretemp btrtl snd_hda_codec btbcm btintel intel_cstate snd_hwdep intel_uncore uvcvideo snd_hda_core videobuf2_vmalloc videobuf2_memops intel_rapl_perf wmi_bmof videobuf2_v4l2 intel_wmi_thunderbolt snd_seq bluetooth joydev videobuf2_common snd_seq_device snd_pcm videodev media i2c_i801 snd_timer idma64 ecdh_generic intel_lpss_pci intel_lpss mei_me mei ucsi_acpi typec_ucsi processor_thermal_device intel_soc_dts_iosf intel_pch_thermal typec thinkpad_acpi wmi snd soundcore rfkill int3403_thermal int340x_thermal_zone int3400_thermal acpi_thermal_rel acpi_pad pcc_cpufreq uas usb_storage crc32c_intel i915 i2c_algo_bit nvme serio_raw [ 2068.592033] drm_kms_helper e1000e nvme_core drm video ipv6 [last unloaded: cfg80211] Fortunate thing is that this not happen frequently, as scheduling tasklet on blocked state is very exceptional, though might happen. Due to different RX/TX tasklet processing fix is different for those. For RX we have to assure rx_tasklet do fail to resubmit buffers by poisoning urb's and kill the tasklet. For TX we need to handle all stop cases properly (suspend, module unload, device removal). Signed-off-by: Stanislaw Gruszka <[email protected]> Signed-off-by: Felix Fietkau <[email protected]>
1 parent 1fb869a commit 39d501d

File tree

7 files changed

+66
-42
lines changed

7 files changed

+66
-42
lines changed

‎drivers/net/wireless/mediatek/mt76/mac80211.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,7 @@ void mt76_rx(struct mt76_dev *dev, enum mt76_rxq_id q, struct sk_buff *skb)
390390
}
391391
EXPORT_SYMBOL_GPL(mt76_rx);
392392

393-
static bool mt76_has_tx_pending(struct mt76_dev *dev)
393+
bool mt76_has_tx_pending(struct mt76_dev *dev)
394394
{
395395
struct mt76_queue *q;
396396
int i;
@@ -403,6 +403,7 @@ static bool mt76_has_tx_pending(struct mt76_dev *dev)
403403

404404
return false;
405405
}
406+
EXPORT_SYMBOL_GPL(mt76_has_tx_pending);
406407

407408
void mt76_set_channel(struct mt76_dev *dev)
408409
{

‎drivers/net/wireless/mediatek/mt76/mt76.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -696,6 +696,7 @@ void mt76_release_buffered_frames(struct ieee80211_hw *hw,
696696
u16 tids, int nframes,
697697
enum ieee80211_frame_release_type reason,
698698
bool more_data);
699+
bool mt76_has_tx_pending(struct mt76_dev *dev);
699700
void mt76_set_channel(struct mt76_dev *dev);
700701
int mt76_get_survey(struct ieee80211_hw *hw, int idx,
701702
struct survey_info *survey);
@@ -790,10 +791,10 @@ int mt76u_vendor_request(struct mt76_dev *dev, u8 req,
790791
void mt76u_single_wr(struct mt76_dev *dev, const u8 req,
791792
const u16 offset, const u32 val);
792793
int mt76u_init(struct mt76_dev *dev, struct usb_interface *intf);
793-
int mt76u_submit_rx_buffers(struct mt76_dev *dev);
794794
int mt76u_alloc_queues(struct mt76_dev *dev);
795-
void mt76u_stop_queues(struct mt76_dev *dev);
796-
void mt76u_stop_stat_wk(struct mt76_dev *dev);
795+
void mt76u_stop_tx(struct mt76_dev *dev);
796+
void mt76u_stop_rx(struct mt76_dev *dev);
797+
int mt76u_resume_rx(struct mt76_dev *dev);
797798
void mt76u_queues_deinit(struct mt76_dev *dev);
798799

799800
struct sk_buff *

‎drivers/net/wireless/mediatek/mt76/mt76x0/usb.c

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,7 @@ static void mt76x0u_mac_stop(struct mt76x02_dev *dev)
8686
clear_bit(MT76_STATE_RUNNING, &dev->mt76.state);
8787
cancel_delayed_work_sync(&dev->cal_work);
8888
cancel_delayed_work_sync(&dev->mt76.mac_work);
89-
mt76u_stop_stat_wk(&dev->mt76);
89+
mt76u_stop_tx(&dev->mt76);
9090
mt76x02u_exit_beacon_config(dev);
9191

9292
if (test_bit(MT76_REMOVED, &dev->mt76.state))
@@ -313,7 +313,7 @@ static int __maybe_unused mt76x0_suspend(struct usb_interface *usb_intf,
313313
{
314314
struct mt76x02_dev *dev = usb_get_intfdata(usb_intf);
315315

316-
mt76u_stop_queues(&dev->mt76);
316+
mt76u_stop_rx(&dev->mt76);
317317
clear_bit(MT76_STATE_MCU_RUNNING, &dev->mt76.state);
318318
mt76x0_chip_onoff(dev, false, false);
319319

@@ -323,16 +323,12 @@ static int __maybe_unused mt76x0_suspend(struct usb_interface *usb_intf,
323323
static int __maybe_unused mt76x0_resume(struct usb_interface *usb_intf)
324324
{
325325
struct mt76x02_dev *dev = usb_get_intfdata(usb_intf);
326-
struct mt76_usb *usb = &dev->mt76.usb;
327326
int ret;
328327

329-
ret = mt76u_submit_rx_buffers(&dev->mt76);
328+
ret = mt76u_resume_rx(&dev->mt76);
330329
if (ret < 0)
331330
goto err;
332331

333-
tasklet_enable(&usb->rx_tasklet);
334-
tasklet_enable(&dev->mt76.tx_tasklet);
335-
336332
ret = mt76x0u_init_hardware(dev);
337333
if (ret)
338334
goto err;

‎drivers/net/wireless/mediatek/mt76/mt76x2/usb.c

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -107,24 +107,20 @@ static int __maybe_unused mt76x2u_suspend(struct usb_interface *intf,
107107
{
108108
struct mt76x02_dev *dev = usb_get_intfdata(intf);
109109

110-
mt76u_stop_queues(&dev->mt76);
110+
mt76u_stop_rx(&dev->mt76);
111111

112112
return 0;
113113
}
114114

115115
static int __maybe_unused mt76x2u_resume(struct usb_interface *intf)
116116
{
117117
struct mt76x02_dev *dev = usb_get_intfdata(intf);
118-
struct mt76_usb *usb = &dev->mt76.usb;
119118
int err;
120119

121-
err = mt76u_submit_rx_buffers(&dev->mt76);
120+
err = mt76u_resume_rx(&dev->mt76);
122121
if (err < 0)
123122
goto err;
124123

125-
tasklet_enable(&usb->rx_tasklet);
126-
tasklet_enable(&dev->mt76.tx_tasklet);
127-
128124
err = mt76x2u_init_hardware(dev);
129125
if (err < 0)
130126
goto err;

‎drivers/net/wireless/mediatek/mt76/mt76x2/usb_init.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,6 @@ int mt76x2u_register_device(struct mt76x02_dev *dev)
244244

245245
void mt76x2u_stop_hw(struct mt76x02_dev *dev)
246246
{
247-
mt76u_stop_stat_wk(&dev->mt76);
248247
cancel_delayed_work_sync(&dev->cal_work);
249248
cancel_delayed_work_sync(&dev->mt76.mac_work);
250249
mt76x2u_mac_stop(dev);

‎drivers/net/wireless/mediatek/mt76/mt76x2/usb_main.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ static void mt76x2u_stop(struct ieee80211_hw *hw)
4242

4343
mutex_lock(&dev->mt76.mutex);
4444
clear_bit(MT76_STATE_RUNNING, &dev->mt76.state);
45+
mt76u_stop_tx(&dev->mt76);
4546
mt76x2u_stop_hw(dev);
4647
mutex_unlock(&dev->mt76.mutex);
4748
}

‎drivers/net/wireless/mediatek/mt76/usb.c

Lines changed: 54 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -540,7 +540,7 @@ static void mt76u_rx_tasklet(unsigned long data)
540540
rcu_read_unlock();
541541
}
542542

543-
int mt76u_submit_rx_buffers(struct mt76_dev *dev)
543+
static int mt76u_submit_rx_buffers(struct mt76_dev *dev)
544544
{
545545
struct mt76_queue *q = &dev->q_rx[MT_RXQ_MAIN];
546546
unsigned long flags;
@@ -558,7 +558,6 @@ int mt76u_submit_rx_buffers(struct mt76_dev *dev)
558558

559559
return err;
560560
}
561-
EXPORT_SYMBOL_GPL(mt76u_submit_rx_buffers);
562561

563562
static int mt76u_alloc_rx(struct mt76_dev *dev)
564563
{
@@ -605,14 +604,29 @@ static void mt76u_free_rx(struct mt76_dev *dev)
605604
memset(&q->rx_page, 0, sizeof(q->rx_page));
606605
}
607606

608-
static void mt76u_stop_rx(struct mt76_dev *dev)
607+
void mt76u_stop_rx(struct mt76_dev *dev)
609608
{
610609
struct mt76_queue *q = &dev->q_rx[MT_RXQ_MAIN];
611610
int i;
612611

613612
for (i = 0; i < q->ndesc; i++)
614-
usb_kill_urb(q->entry[i].urb);
613+
usb_poison_urb(q->entry[i].urb);
614+
615+
tasklet_kill(&dev->usb.rx_tasklet);
616+
}
617+
EXPORT_SYMBOL_GPL(mt76u_stop_rx);
618+
619+
int mt76u_resume_rx(struct mt76_dev *dev)
620+
{
621+
struct mt76_queue *q = &dev->q_rx[MT_RXQ_MAIN];
622+
int i;
623+
624+
for (i = 0; i < q->ndesc; i++)
625+
usb_unpoison_urb(q->entry[i].urb);
626+
627+
return mt76u_submit_rx_buffers(dev);
615628
}
629+
EXPORT_SYMBOL_GPL(mt76u_resume_rx);
616630

617631
static void mt76u_tx_tasklet(unsigned long data)
618632
{
@@ -834,38 +848,54 @@ static void mt76u_free_tx(struct mt76_dev *dev)
834848
}
835849
}
836850

837-
static void mt76u_stop_tx(struct mt76_dev *dev)
851+
void mt76u_stop_tx(struct mt76_dev *dev)
838852
{
853+
struct mt76_queue_entry entry;
839854
struct mt76_queue *q;
840-
int i, j;
855+
int i, j, ret;
841856

842-
for (i = 0; i < IEEE80211_NUM_ACS; i++) {
843-
q = dev->q_tx[i].q;
844-
for (j = 0; j < q->ndesc; j++)
845-
usb_kill_urb(q->entry[j].urb);
846-
}
847-
}
857+
ret = wait_event_timeout(dev->tx_wait, !mt76_has_tx_pending(dev), HZ/5);
858+
if (!ret) {
859+
dev_err(dev->dev, "timed out waiting for pending tx\n");
848860

849-
void mt76u_stop_queues(struct mt76_dev *dev)
850-
{
851-
tasklet_disable(&dev->usb.rx_tasklet);
852-
tasklet_disable(&dev->tx_tasklet);
861+
for (i = 0; i < IEEE80211_NUM_ACS; i++) {
862+
q = dev->q_tx[i].q;
863+
for (j = 0; j < q->ndesc; j++)
864+
usb_kill_urb(q->entry[j].urb);
865+
}
853866

854-
mt76u_stop_rx(dev);
855-
mt76u_stop_tx(dev);
856-
}
857-
EXPORT_SYMBOL_GPL(mt76u_stop_queues);
867+
tasklet_kill(&dev->tx_tasklet);
868+
869+
/* On device removal we maight queue skb's, but mt76u_tx_kick()
870+
* will fail to submit urb, cleanup those skb's manually.
871+
*/
872+
for (i = 0; i < IEEE80211_NUM_ACS; i++) {
873+
q = dev->q_tx[i].q;
874+
875+
/* Assure we are in sync with killed tasklet. */
876+
spin_lock_bh(&q->lock);
877+
while (q->queued) {
878+
entry = q->entry[q->head];
879+
q->head = (q->head + 1) % q->ndesc;
880+
q->queued--;
881+
882+
dev->drv->tx_complete_skb(dev, i, &entry);
883+
}
884+
spin_unlock_bh(&q->lock);
885+
}
886+
}
858887

859-
void mt76u_stop_stat_wk(struct mt76_dev *dev)
860-
{
861888
cancel_delayed_work_sync(&dev->usb.stat_work);
862889
clear_bit(MT76_READING_STATS, &dev->state);
890+
891+
mt76_tx_status_check(dev, NULL, true);
863892
}
864-
EXPORT_SYMBOL_GPL(mt76u_stop_stat_wk);
893+
EXPORT_SYMBOL_GPL(mt76u_stop_tx);
865894

866895
void mt76u_queues_deinit(struct mt76_dev *dev)
867896
{
868-
mt76u_stop_queues(dev);
897+
mt76u_stop_rx(dev);
898+
mt76u_stop_tx(dev);
869899

870900
mt76u_free_rx(dev);
871901
mt76u_free_tx(dev);

0 commit comments

Comments
 (0)