From: Jakub Kicinski <[email protected]>
DMA to variables located on the stack is a bad idea.
For simplicity and to avoid frequent allocations create
a buffer inside the device structure. Protect this
buffer with vendor_req_mutex. Don't protect vendor
requests which don't use this buffer.
Signed-off-by: Jakub Kicinski <[email protected]>
---
drivers/net/wireless/mediatek/mt7601u/mt7601u.h | 4 +-
drivers/net/wireless/mediatek/mt7601u/usb.c | 63 ++++++++++++-------------
drivers/net/wireless/mediatek/mt7601u/usb.h | 2 +
3 files changed, 34 insertions(+), 35 deletions(-)
diff --git a/drivers/net/wireless/mediatek/mt7601u/mt7601u.h b/drivers/net/wireless/mediatek/mt7601u/mt7601u.h
index 9102be6b95cb..6bdfc1103fcc 100644
--- a/drivers/net/wireless/mediatek/mt7601u/mt7601u.h
+++ b/drivers/net/wireless/mediatek/mt7601u/mt7601u.h
@@ -146,7 +146,7 @@ enum {
* @rx_lock: protects @rx_q.
* @con_mon_lock: protects @ap_bssid, @bcn_*, @avg_rssi.
* @mutex: ensures exclusive access from mac80211 callbacks.
- * @vendor_req_mutex: ensures atomicity of vendor requests.
+ * @vendor_req_mutex: protects @vend_buf, ensures atomicity of split writes.
* @reg_atomic_mutex: ensures atomicity of indirect register accesses
* (accesses to RF and BBP).
* @hw_atomic_mutex: ensures exclusive access to HW during critical
@@ -184,6 +184,8 @@ struct mt7601u_dev {
struct mt7601u_eeprom_params *ee;
struct mutex vendor_req_mutex;
+ void *vend_buf;
+
struct mutex reg_atomic_mutex;
struct mutex hw_atomic_mutex;
diff --git a/drivers/net/wireless/mediatek/mt7601u/usb.c b/drivers/net/wireless/mediatek/mt7601u/usb.c
index 54dba4001865..416c6045ff31 100644
--- a/drivers/net/wireless/mediatek/mt7601u/usb.c
+++ b/drivers/net/wireless/mediatek/mt7601u/usb.c
@@ -92,10 +92,9 @@ void mt7601u_complete_urb(struct urb *urb)
complete(cmpl);
}
-static int
-__mt7601u_vendor_request(struct mt7601u_dev *dev, const u8 req,
- const u8 direction, const u16 val, const u16 offset,
- void *buf, const size_t buflen)
+int mt7601u_vendor_request(struct mt7601u_dev *dev, const u8 req,
+ const u8 direction, const u16 val, const u16 offset,
+ void *buf, const size_t buflen)
{
int i, ret;
struct usb_device *usb_dev = mt7601u_to_usb_dev(dev);
@@ -110,6 +109,8 @@ __mt7601u_vendor_request(struct mt7601u_dev *dev, const u8 req,
trace_mt_vend_req(dev, pipe, req, req_type, val, offset,
buf, buflen, ret);
+ if (ret == -ENODEV)
+ set_bit(MT7601U_STATE_REMOVED, &dev->state);
if (ret >= 0 || ret == -ENODEV)
return ret;
@@ -122,25 +123,6 @@ __mt7601u_vendor_request(struct mt7601u_dev *dev, const u8 req,
return ret;
}
-int
-mt7601u_vendor_request(struct mt7601u_dev *dev, const u8 req,
- const u8 direction, const u16 val, const u16 offset,
- void *buf, const size_t buflen)
-{
- int ret;
-
- mutex_lock(&dev->vendor_req_mutex);
-
- ret = __mt7601u_vendor_request(dev, req, direction, val, offset,
- buf, buflen);
- if (ret == -ENODEV)
- set_bit(MT7601U_STATE_REMOVED, &dev->state);
-
- mutex_unlock(&dev->vendor_req_mutex);
-
- return ret;
-}
-
void mt7601u_vendor_reset(struct mt7601u_dev *dev)
{
mt7601u_vendor_request(dev, MT_VEND_DEV_MODE, USB_DIR_OUT,
@@ -150,19 +132,21 @@ void mt7601u_vendor_reset(struct mt7601u_dev *dev)
u32 mt7601u_rr(struct mt7601u_dev *dev, u32 offset)
{
int ret;
- __le32 reg;
- u32 val;
+ u32 val = ~0;
WARN_ONCE(offset > USHRT_MAX, "read high off:%08x", offset);
+ mutex_lock(&dev->vendor_req_mutex);
+
ret = mt7601u_vendor_request(dev, MT_VEND_MULTI_READ, USB_DIR_IN,
- 0, offset, ®, sizeof(reg));
- val = le32_to_cpu(reg);
- if (ret > 0 && ret != sizeof(reg)) {
+ 0, offset, dev->vend_buf, MT_VEND_BUF);
+ if (ret == MT_VEND_BUF)
+ val = get_unaligned_le32(dev->vend_buf);
+ else if (ret > 0)
dev_err(dev->dev, "Error: wrong size read:%d off:%08x\n",
ret, offset);
- val = ~0;
- }
+
+ mutex_unlock(&dev->vendor_req_mutex);
trace_reg_read(dev, offset, val);
return val;
@@ -173,12 +157,17 @@ int mt7601u_vendor_single_wr(struct mt7601u_dev *dev, const u8 req,
{
int ret;
+ mutex_lock(&dev->vendor_req_mutex);
+
ret = mt7601u_vendor_request(dev, req, USB_DIR_OUT,
val & 0xffff, offset, NULL, 0);
- if (ret)
- return ret;
- return mt7601u_vendor_request(dev, req, USB_DIR_OUT,
- val >> 16, offset + 2, NULL, 0);
+ if (!ret)
+ ret = mt7601u_vendor_request(dev, req, USB_DIR_OUT,
+ val >> 16, offset + 2, NULL, 0);
+
+ mutex_unlock(&dev->vendor_req_mutex);
+
+ return ret;
}
void mt7601u_wr(struct mt7601u_dev *dev, u32 offset, u32 val)
@@ -275,6 +264,12 @@ static int mt7601u_probe(struct usb_interface *usb_intf,
usb_set_intfdata(usb_intf, dev);
+ dev->vend_buf = devm_kmalloc(dev->dev, MT_VEND_BUF, GFP_KERNEL);
+ if (!dev->vend_buf) {
+ ret = -ENOMEM;
+ goto err;
+ }
+
ret = mt7601u_assign_pipes(usb_intf, dev);
if (ret)
goto err;
diff --git a/drivers/net/wireless/mediatek/mt7601u/usb.h b/drivers/net/wireless/mediatek/mt7601u/usb.h
index 49e188fa3798..bc182022b9d6 100644
--- a/drivers/net/wireless/mediatek/mt7601u/usb.h
+++ b/drivers/net/wireless/mediatek/mt7601u/usb.h
@@ -23,6 +23,8 @@
#define MT_VEND_DEV_MODE_RESET 1
+#define MT_VEND_BUF sizeof(__le32)
+
enum mt_vendor_req {
MT_VEND_DEV_MODE = 1,
MT_VEND_WRITE = 2,
--
2.1.0
From: Jakub Kicinski <[email protected]>
mac80211 requires that rx path does not run concurrently with
tx status reporting. Add a spinlock which will ensure that.
Signed-off-by: Jakub Kicinski <[email protected]>
---
drivers/net/wireless/mediatek/mt7601u/dma.c | 2 ++
drivers/net/wireless/mediatek/mt7601u/init.c | 1 +
drivers/net/wireless/mediatek/mt7601u/mac.c | 4 ++--
drivers/net/wireless/mediatek/mt7601u/mt7601u.h | 4 +++-
drivers/net/wireless/mediatek/mt7601u/tx.c | 3 +++
5 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/mediatek/mt7601u/dma.c b/drivers/net/wireless/mediatek/mt7601u/dma.c
index 63c485443a38..57a80cfa39b1 100644
--- a/drivers/net/wireless/mediatek/mt7601u/dma.c
+++ b/drivers/net/wireless/mediatek/mt7601u/dma.c
@@ -112,7 +112,9 @@ static void mt7601u_rx_process_seg(struct mt7601u_dev *dev, u8 *data,
if (!skb)
return;
+ spin_lock(&dev->mac_lock);
ieee80211_rx(dev->hw, skb);
+ spin_unlock(&dev->mac_lock);
}
static u16 mt7601u_rx_next_seg_len(u8 *data, u32 data_len)
diff --git a/drivers/net/wireless/mediatek/mt7601u/init.c b/drivers/net/wireless/mediatek/mt7601u/init.c
index 38eb20ba6e58..26190fd33407 100644
--- a/drivers/net/wireless/mediatek/mt7601u/init.c
+++ b/drivers/net/wireless/mediatek/mt7601u/init.c
@@ -454,6 +454,7 @@ struct mt7601u_dev *mt7601u_alloc_device(struct device *pdev)
spin_lock_init(&dev->tx_lock);
spin_lock_init(&dev->rx_lock);
spin_lock_init(&dev->lock);
+ spin_lock_init(&dev->mac_lock);
spin_lock_init(&dev->con_mon_lock);
atomic_set(&dev->avg_ampdu_len, 1);
skb_queue_head_init(&dev->tx_skb_done);
diff --git a/drivers/net/wireless/mediatek/mt7601u/mac.c b/drivers/net/wireless/mediatek/mt7601u/mac.c
index e3928cfa3d63..e21c53ed09fb 100644
--- a/drivers/net/wireless/mediatek/mt7601u/mac.c
+++ b/drivers/net/wireless/mediatek/mt7601u/mac.c
@@ -182,9 +182,9 @@ void mt76_send_tx_status(struct mt7601u_dev *dev, struct mt76_tx_status *stat)
mt76_mac_fill_tx_status(dev, &info, stat);
- local_bh_disable();
+ spin_lock_bh(&dev->mac_lock);
ieee80211_tx_status_noskb(dev->hw, sta, &info);
- local_bh_enable();
+ spin_unlock_bh(&dev->mac_lock);
rcu_read_unlock();
}
diff --git a/drivers/net/wireless/mediatek/mt7601u/mt7601u.h b/drivers/net/wireless/mediatek/mt7601u/mt7601u.h
index bc5e294feb8c..428bd2f10b7b 100644
--- a/drivers/net/wireless/mediatek/mt7601u/mt7601u.h
+++ b/drivers/net/wireless/mediatek/mt7601u/mt7601u.h
@@ -141,8 +141,9 @@ enum {
/**
* struct mt7601u_dev - adapter structure
* @lock: protects @wcid->tx_rate.
+ * @mac_lock: locks out mac80211's tx status and rx paths.
* @tx_lock: protects @tx_q and changes of MT7601U_STATE_*_STATS
- flags in @state.
+ * flags in @state.
* @rx_lock: protects @rx_q.
* @con_mon_lock: protects @ap_bssid, @bcn_*, @avg_rssi.
* @mutex: ensures exclusive access from mac80211 callbacks.
@@ -177,6 +178,7 @@ struct mt7601u_dev {
struct mt76_wcid __rcu *wcid[N_WCIDS];
spinlock_t lock;
+ spinlock_t mac_lock;
const u16 *beacon_offsets;
diff --git a/drivers/net/wireless/mediatek/mt7601u/tx.c b/drivers/net/wireless/mediatek/mt7601u/tx.c
index 0be2080ceab3..a0a33dc8f6bc 100644
--- a/drivers/net/wireless/mediatek/mt7601u/tx.c
+++ b/drivers/net/wireless/mediatek/mt7601u/tx.c
@@ -116,7 +116,10 @@ void mt7601u_tx_status(struct mt7601u_dev *dev, struct sk_buff *skb)
ieee80211_tx_info_clear_status(info);
info->status.rates[0].idx = -1;
info->flags |= IEEE80211_TX_STAT_ACK;
+
+ spin_lock(&dev->mac_lock);
ieee80211_tx_status(dev->hw, skb);
+ spin_unlock(&dev->mac_lock);
}
static int mt7601u_skb_rooms(struct mt7601u_dev *dev, struct sk_buff *skb)
--
2.1.0
From: Jakub Kicinski <[email protected]>
Rx is run inside a tasklet so ieee80211_rx() should be used
instead of ieee80211_rx_ni().
Signed-off-by: Jakub Kicinski <[email protected]>
---
drivers/net/wireless/mediatek/mt7601u/dma.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/mediatek/mt7601u/dma.c b/drivers/net/wireless/mediatek/mt7601u/dma.c
index 7217da4f1543..fb183e369d92 100644
--- a/drivers/net/wireless/mediatek/mt7601u/dma.c
+++ b/drivers/net/wireless/mediatek/mt7601u/dma.c
@@ -112,7 +112,7 @@ static void mt7601u_rx_process_seg(struct mt7601u_dev *dev, u8 *data,
if (!skb)
return;
- ieee80211_rx_ni(dev->hw, skb);
+ ieee80211_rx(dev->hw, skb);
}
static u16 mt7601u_rx_next_seg_len(u8 *data, u32 data_len)
--
2.1.0
From: Jakub Kicinski <[email protected]>
mac80211 requires that rx path does not run concurrently with
tx status reporting. Since rx path is run in driver tasklet,
tx status cannot be reported directly from interrupt context
(there would be no way to lock it out).
Add tasklet for tx and move all possible code from irq handler
there.
Note: tx tasklet is needed because workqueue is queued very
rarely and that kills TCP performance.
Signed-off-by: Jakub Kicinski <[email protected]>
---
drivers/net/wireless/mediatek/mt7601u/dma.c | 30 +++++++++++++++++++++----
drivers/net/wireless/mediatek/mt7601u/init.c | 1 +
drivers/net/wireless/mediatek/mt7601u/mac.c | 4 ++++
drivers/net/wireless/mediatek/mt7601u/mt7601u.h | 2 ++
4 files changed, 33 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/mediatek/mt7601u/dma.c b/drivers/net/wireless/mediatek/mt7601u/dma.c
index fb183e369d92..63c485443a38 100644
--- a/drivers/net/wireless/mediatek/mt7601u/dma.c
+++ b/drivers/net/wireless/mediatek/mt7601u/dma.c
@@ -236,23 +236,42 @@ static void mt7601u_complete_tx(struct urb *urb)
skb = q->e[q->start].skb;
trace_mt_tx_dma_done(dev, skb);
- mt7601u_tx_status(dev, skb);
+ __skb_queue_tail(&dev->tx_skb_done, skb);
+ tasklet_schedule(&dev->tx_tasklet);
if (q->used == q->entries - q->entries / 8)
ieee80211_wake_queue(dev->hw, skb_get_queue_mapping(skb));
q->start = (q->start + 1) % q->entries;
q->used--;
+out:
+ spin_unlock_irqrestore(&dev->tx_lock, flags);
+}
- if (urb->status)
- goto out;
+static void mt7601u_tx_tasklet(unsigned long data)
+{
+ struct mt7601u_dev *dev = (struct mt7601u_dev *) data;
+ struct sk_buff_head skbs;
+ unsigned long flags;
+
+ __skb_queue_head_init(&skbs);
+
+ spin_lock_irqsave(&dev->tx_lock, flags);
set_bit(MT7601U_STATE_MORE_STATS, &dev->state);
if (!test_and_set_bit(MT7601U_STATE_READING_STATS, &dev->state))
queue_delayed_work(dev->stat_wq, &dev->stat_work,
msecs_to_jiffies(10));
-out:
+
+ skb_queue_splice_init(&dev->tx_skb_done, &skbs);
+
spin_unlock_irqrestore(&dev->tx_lock, flags);
+
+ while (!skb_queue_empty(&skbs)) {
+ struct sk_buff *skb = __skb_dequeue(&skbs);
+
+ mt7601u_tx_status(dev, skb);
+ }
}
static int mt7601u_dma_submit_tx(struct mt7601u_dev *dev,
@@ -475,6 +494,7 @@ int mt7601u_dma_init(struct mt7601u_dev *dev)
{
int ret = -ENOMEM;
+ tasklet_init(&dev->tx_tasklet, mt7601u_tx_tasklet, (unsigned long) dev);
tasklet_init(&dev->rx_tasklet, mt7601u_rx_tasklet, (unsigned long) dev);
ret = mt7601u_alloc_tx(dev);
@@ -502,4 +522,6 @@ void mt7601u_dma_cleanup(struct mt7601u_dev *dev)
mt7601u_free_rx(dev);
mt7601u_free_tx(dev);
+
+ tasklet_kill(&dev->tx_tasklet);
}
diff --git a/drivers/net/wireless/mediatek/mt7601u/init.c b/drivers/net/wireless/mediatek/mt7601u/init.c
index df3dd56199a7..38eb20ba6e58 100644
--- a/drivers/net/wireless/mediatek/mt7601u/init.c
+++ b/drivers/net/wireless/mediatek/mt7601u/init.c
@@ -456,6 +456,7 @@ struct mt7601u_dev *mt7601u_alloc_device(struct device *pdev)
spin_lock_init(&dev->lock);
spin_lock_init(&dev->con_mon_lock);
atomic_set(&dev->avg_ampdu_len, 1);
+ skb_queue_head_init(&dev->tx_skb_done);
dev->stat_wq = alloc_workqueue("mt7601u", WQ_UNBOUND, 0);
if (!dev->stat_wq) {
diff --git a/drivers/net/wireless/mediatek/mt7601u/mac.c b/drivers/net/wireless/mediatek/mt7601u/mac.c
index 7514bce1ac91..e3928cfa3d63 100644
--- a/drivers/net/wireless/mediatek/mt7601u/mac.c
+++ b/drivers/net/wireless/mediatek/mt7601u/mac.c
@@ -181,7 +181,11 @@ void mt76_send_tx_status(struct mt7601u_dev *dev, struct mt76_tx_status *stat)
}
mt76_mac_fill_tx_status(dev, &info, stat);
+
+ local_bh_disable();
ieee80211_tx_status_noskb(dev->hw, sta, &info);
+ local_bh_enable();
+
rcu_read_unlock();
}
diff --git a/drivers/net/wireless/mediatek/mt7601u/mt7601u.h b/drivers/net/wireless/mediatek/mt7601u/mt7601u.h
index 6bdfc1103fcc..bc5e294feb8c 100644
--- a/drivers/net/wireless/mediatek/mt7601u/mt7601u.h
+++ b/drivers/net/wireless/mediatek/mt7601u/mt7601u.h
@@ -199,7 +199,9 @@ struct mt7601u_dev {
/* TX */
spinlock_t tx_lock;
+ struct tasklet_struct tx_tasklet;
struct mt7601u_tx_queue *tx_q;
+ struct sk_buff_head tx_skb_done;
atomic_t avg_ampdu_len;
--
2.1.0
> From: Jakub Kicinski <[email protected]>
>
> DMA to variables located on the stack is a bad idea.
> For simplicity and to avoid frequent allocations create
> a buffer inside the device structure. Protect this
> buffer with vendor_req_mutex. Don't protect vendor
> requests which don't use this buffer.
>
> Signed-off-by: Jakub Kicinski <[email protected]>
Thanks, 4 patches applied to wireless-drivers-next.git:
bed429e1ae8b mt7601u: fix dma from stack address
d9517c0a5d74 mt7601u: use correct ieee80211_rx variant
4513493d188d mt7601u: fix tx status reporting contexts
78623bfb6f4c mt7601u: lock out rx path and tx status reporting
Kalle Valo