| Message ID | 20191104134540.19199-1-markus.theil@tu-ilmenau.de (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Felix Fietkau |
| Headers | show |
| Series | mt76: disable softirqs while calling ieee80211_rx_napi | expand |
On 2019-11-04 14:45, Markus Theil wrote: > mac80211 assumes ieee80211_rx_napi to be called with disabled softirqs. > > ieee80211_rx_napi in mac80211.c can be called from aggregation reordering work queue > or from mt76_rx_poll_complete. mt76_rx_poll_complete does currently not disable softirq > processing. > > This patch fixes this by disabling softirqs before calling ieee80211_rx_napi. > It should be no problem to disable them twice, if mt76_aggr_reorder_work calls ieee80211_rx_napi > and has already called local_bh_disable, as local_bh_disable/local_bh_enable are reentrant. > > I became aware of this issue by the following dmesg output: > NOHZ: local_softirq_pending 08 > > Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de> I believe this fix is incomplete. If we run with softirq enabled at this point, it also implies that we've taken the RCU lock with softirq enabled, which we really shouldn't do. I believe this should be fixed by changing rcu_read_lock/unlock to the _bh variant in mt76_dma_rx_poll(). I will send a patch for that. Thanks, - Felix
diff --git a/drivers/net/wireless/mediatek/mt76/mac80211.c b/drivers/net/wireless/mediatek/mt76/mac80211.c index 1a2c143b34d0..43c050660fc7 100644 --- a/drivers/net/wireless/mediatek/mt76/mac80211.c +++ b/drivers/net/wireless/mediatek/mt76/mac80211.c @@ -628,7 +628,7 @@ void mt76_rx_complete(struct mt76_dev *dev, struct sk_buff_head *frames, struct ieee80211_sta *sta; struct sk_buff *skb; - spin_lock(&dev->rx_lock); + spin_lock_bh(&dev->rx_lock); while ((skb = __skb_dequeue(frames)) != NULL) { if (mt76_check_ccmp_pn(skb)) { dev_kfree_skb(skb); @@ -638,7 +638,7 @@ void mt76_rx_complete(struct mt76_dev *dev, struct sk_buff_head *frames, sta = mt76_rx_convert(skb); ieee80211_rx_napi(dev->hw, sta, skb, napi); } - spin_unlock(&dev->rx_lock); + spin_unlock_bh(&dev->rx_lock); } void mt76_rx_poll_complete(struct mt76_dev *dev, enum mt76_rxq_id q,
mac80211 assumes ieee80211_rx_napi to be called with disabled softirqs. ieee80211_rx_napi in mac80211.c can be called from aggregation reordering work queue or from mt76_rx_poll_complete. mt76_rx_poll_complete does currently not disable softirq processing. This patch fixes this by disabling softirqs before calling ieee80211_rx_napi. It should be no problem to disable them twice, if mt76_aggr_reorder_work calls ieee80211_rx_napi and has already called local_bh_disable, as local_bh_disable/local_bh_enable are reentrant. I became aware of this issue by the following dmesg output: NOHZ: local_softirq_pending 08 Signed-off-by: Markus Theil <markus.theil@tu-ilmenau.de> --- drivers/net/wireless/mediatek/mt76/mac80211.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) -- 2.20.1