This patch series contains 2 fixes to avoid RX data stall on
observed on high through put scenarios on some slower platforms.
Shrenik Shikhare (2):
mwifiex: schedule rx_work on RX interrupt for USB
mwifiex: use more_rx_task_flag to avoid USB RX stall
drivers/net/wireless/marvell/mwifiex/main.c | 17 +++++++++++++----
drivers/net/wireless/marvell/mwifiex/main.h | 2 ++
drivers/net/wireless/marvell/mwifiex/usb.c | 2 ++
3 files changed, 17 insertions(+), 4 deletions(-)
--
1.9.1
From: Shrenik Shikhare <[email protected]>
There is a race condition for acquiring rx_proc_lock between
rx worker thread and USB RX data interrupt
(mwifiex_usb_rx_complete):
1. USB receives an RX data interrupt, queues rx_work
2. rx_work empties rx_data_q, tries to acquire rx_proc_lock (to
clear rx_processing flag)
3. While #2 is yet to acquire rx_proc_lock, driver receives
continuous RX data interupts(mwifiex_usb_rx_complete)
3. For each interrupt at #3, driver acquires rx_proc_lock(it gets
the lock since it is in interrupt context), tries to queue
rx_work, but fails to do so since rx_processing is still set(#2)
4. When rx_pending exceeds HIGH_RX_PENDING, driver stops
submitting URBs back to USB subsystem and thus firmware stops
uploading RX data to driver
5. Now finally #2 will acquire rx_proc_lock, but because of #4,
there are no further triggers to schedule rx_work again
The above scenario occurs in some platforms where the RX
processing is comparitively slower. This results in RX stall in
driver, command/TX timeouts in firmware. The above scenario is
introduced after commit c7dbdcb2a4e1
("mwifiex: schedule rx_work on RX interrupt for USB")
To fix this set a new more_rx_task_flag whenever RX data callback
is trying to schedule rx_work but rx_processing is not yet
cleared. This will let the current rx_work(which was waiting for
rx_proc_lock) to loopback and process newly arrived RX packets.
Fixes: c7dbdcb2a4e1 ("mwifiex: schedule rx_work on RX interrupt for USB")
Signed-off-by: Cathy Luo <[email protected]>
Signed-off-by: Ganapathi Bhat <[email protected]>
---
v2: added 'Fixes' label in commit message
---
drivers/net/wireless/marvell/mwifiex/main.c | 10 +++++++++-
drivers/net/wireless/marvell/mwifiex/main.h | 1 +
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c
index 6e6e1a7..ea87c7c 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.c
+++ b/drivers/net/wireless/marvell/mwifiex/main.c
@@ -163,6 +163,7 @@ void mwifiex_queue_main_work(struct mwifiex_adapter *adapter)
spin_lock_irqsave(&adapter->main_proc_lock, flags);
if (adapter->mwifiex_processing) {
adapter->more_task_flag = true;
+ adapter->more_rx_task_flag = true;
spin_unlock_irqrestore(&adapter->main_proc_lock, flags);
} else {
spin_unlock_irqrestore(&adapter->main_proc_lock, flags);
@@ -177,6 +178,7 @@ void mwifiex_queue_rx_work(struct mwifiex_adapter *adapter)
spin_lock_irqsave(&adapter->rx_proc_lock, flags);
if (adapter->rx_processing) {
+ adapter->more_rx_task_flag = true;
spin_unlock_irqrestore(&adapter->rx_proc_lock, flags);
} else {
spin_unlock_irqrestore(&adapter->rx_proc_lock, flags);
@@ -193,13 +195,14 @@ static int mwifiex_process_rx(struct mwifiex_adapter *adapter)
spin_lock_irqsave(&adapter->rx_proc_lock, flags);
if (adapter->rx_processing || adapter->rx_locked) {
+ adapter->more_rx_task_flag = true;
spin_unlock_irqrestore(&adapter->rx_proc_lock, flags);
goto exit_rx_proc;
} else {
adapter->rx_processing = true;
spin_unlock_irqrestore(&adapter->rx_proc_lock, flags);
}
-
+rx_process_start:
/* Check for Rx data */
while ((skb = skb_dequeue(&adapter->rx_data_q))) {
atomic_dec(&adapter->rx_pending);
@@ -221,6 +224,11 @@ static int mwifiex_process_rx(struct mwifiex_adapter *adapter)
}
}
spin_lock_irqsave(&adapter->rx_proc_lock, flags);
+ if (adapter->more_rx_task_flag) {
+ adapter->more_rx_task_flag = false;
+ spin_unlock_irqrestore(&adapter->rx_proc_lock, flags);
+ goto rx_process_start;
+ }
adapter->rx_processing = false;
spin_unlock_irqrestore(&adapter->rx_proc_lock, flags);
diff --git a/drivers/net/wireless/marvell/mwifiex/main.h b/drivers/net/wireless/marvell/mwifiex/main.h
index 66ba95c..242e05e 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.h
+++ b/drivers/net/wireless/marvell/mwifiex/main.h
@@ -891,6 +891,7 @@ struct mwifiex_adapter {
spinlock_t main_proc_lock;
u32 mwifiex_processing;
u8 more_task_flag;
+ u8 more_rx_task_flag;
u16 tx_buf_size;
u16 curr_tx_buf_size;
/* sdio single port rx aggregation capability */
--
1.9.1
From: Shrenik Shikhare <[email protected]>
There is race for data_received flag between main thread and
RX data interrupt(mwifiex_usb_rx_complete()):
1. USB received an RX data interrupt, set data_received flag
2. main thread checks data_received, if set queues rx_work
3. rx worker thread independently start processing rx_data_q
4. rx work exits (once rx_data_q is empty)
5. main thread resets the data_received flag(after #2)
6. Now at the corner case there will be high RX data interrupts
between #4 and #5
7. Driver stops submitting URBs to firmware, once rx_pending
exceeds HIGH_RX_PENDING
8. The flag data_received(cleared in #5) will remain unset since
there will be no interrupts from firmware to set it(after #7)
Above scenario causes RX stall in driver, which will finally
result in command/TX timeouts in firmware.
As a fix, queue rx_work directly in mwifiex_usb_rx_complete()
callback, instead in the main thread. This removes dependency
of RX processing on data_received flag.
Signed-off-by: Cathy Luo <[email protected]>
Signed-off-by: Ganapathi Bhat <[email protected]>
---
v2: same as v1
---
drivers/net/wireless/marvell/mwifiex/main.c | 7 ++++---
drivers/net/wireless/marvell/mwifiex/main.h | 1 +
drivers/net/wireless/marvell/mwifiex/usb.c | 2 ++
3 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/main.c b/drivers/net/wireless/marvell/mwifiex/main.c
index 12e7399..6e6e1a7 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.c
+++ b/drivers/net/wireless/marvell/mwifiex/main.c
@@ -171,7 +171,7 @@ void mwifiex_queue_main_work(struct mwifiex_adapter *adapter)
}
EXPORT_SYMBOL_GPL(mwifiex_queue_main_work);
-static void mwifiex_queue_rx_work(struct mwifiex_adapter *adapter)
+void mwifiex_queue_rx_work(struct mwifiex_adapter *adapter)
{
unsigned long flags;
@@ -183,6 +183,7 @@ static void mwifiex_queue_rx_work(struct mwifiex_adapter *adapter)
queue_work(adapter->rx_workqueue, &adapter->rx_work);
}
}
+EXPORT_SYMBOL_GPL(mwifiex_queue_rx_work);
static int mwifiex_process_rx(struct mwifiex_adapter *adapter)
{
@@ -283,10 +284,10 @@ int mwifiex_main_process(struct mwifiex_adapter *adapter)
mwifiex_process_hs_config(adapter);
if (adapter->if_ops.process_int_status)
adapter->if_ops.process_int_status(adapter);
+ if (adapter->rx_work_enabled && adapter->data_received)
+ mwifiex_queue_rx_work(adapter);
}
- if (adapter->rx_work_enabled && adapter->data_received)
- mwifiex_queue_rx_work(adapter);
/* Need to wake up the card ? */
if ((adapter->ps_state == PS_STATE_SLEEP) &&
diff --git a/drivers/net/wireless/marvell/mwifiex/main.h b/drivers/net/wireless/marvell/mwifiex/main.h
index 6b5539b..66ba95c 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.h
+++ b/drivers/net/wireless/marvell/mwifiex/main.h
@@ -1667,6 +1667,7 @@ u8 mwifiex_adjust_data_rate(struct mwifiex_private *priv,
void mwifiex_upload_device_dump(struct mwifiex_adapter *adapter);
void *mwifiex_alloc_dma_align_buf(int rx_len, gfp_t flags);
void mwifiex_queue_main_work(struct mwifiex_adapter *adapter);
+void mwifiex_queue_rx_work(struct mwifiex_adapter *adapter);
int mwifiex_get_wakeup_reason(struct mwifiex_private *priv, u16 action,
int cmd_type,
struct mwifiex_ds_wakeup_reason *wakeup_reason);
diff --git a/drivers/net/wireless/marvell/mwifiex/usb.c b/drivers/net/wireless/marvell/mwifiex/usb.c
index 4bc2448..d20fda1 100644
--- a/drivers/net/wireless/marvell/mwifiex/usb.c
+++ b/drivers/net/wireless/marvell/mwifiex/usb.c
@@ -144,6 +144,8 @@ static int mwifiex_usb_recv(struct mwifiex_adapter *adapter,
skb_queue_tail(&adapter->rx_data_q, skb);
adapter->data_received = true;
atomic_inc(&adapter->rx_pending);
+ if (adapter->rx_work_enabled)
+ mwifiex_queue_rx_work(adapter);
break;
default:
mwifiex_dbg(adapter, ERROR,
--
1.9.1