[PATCH 0/2] mwifiex: fix RX data stall issue on USB interface

[PATCH 0/2] mwifiex: fix RX data stall issue on USB interface

[Ganapathi Bhat]

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

[PATCH 1/2] mwifiex: schedule rx_work on RX interrupt for USB

[Ganapathi Bhat]

From: Shrenik Shikhare <shrenik@marvell.com>

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 <cluo@marvell.com>
Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>
---
 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

[PATCH 2/2] mwifiex: use more_rx_task_flag to avoid USB RX stall

[Ganapathi Bhat]

From: Shrenik Shikhare <shrenik@marvell.com>

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.

Signed-off-by: Cathy Luo <cluo@marvell.com>
Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>
---
 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

Re: [1/2] mwifiex: schedule rx_work on RX interrupt for USB

[Kalle Valo]

Ganapathi Bhat <gbhat@marvell.com> wrote:

> From: Shrenik Shikhare <shrenik@marvell.com>
> 
> 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 <cluo@marvell.com>
> Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>

Brian, did you have a chance to review these two?

-- 
https://patchwork.kernel.org/patch/10178731/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [2/2] mwifiex: use more_rx_task_flag to avoid USB RX stall

[Kalle Valo]

Ganapathi Bhat <gbhat@marvell.com> wrote:

> From: Shrenik Shikhare <shrenik@marvell.com>
> 
> 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.
> 
> Signed-off-by: Cathy Luo <cluo@marvell.com>
> Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>

I can't find any commit with id c7dbdcb2a4e1, is it correct?

-- 
https://patchwork.kernel.org/patch/10178729/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

Re: [2/2] mwifiex: use more_rx_task_flag to avoid USB RX stall

[Kalle Valo]

Kalle Valo <kvalo@codeaurora.org> writes:

> Ganapathi Bhat <gbhat@marvell.com> wrote:
>
>> From: Shrenik Shikhare <shrenik@marvell.com>
>> 
>> 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.
>> 
>> Signed-off-by: Cathy Luo <cluo@marvell.com>
>> Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>
>
> I can't find any commit with id c7dbdcb2a4e1, is it correct?

Oh, and please use Fixes line to mark the commit which broke this.

-- 
Kalle Valo

RE: [EXT] Re: [2/2] mwifiex: use more_rx_task_flag to avoid USB RX stall

[Ganapathi Bhat]

SGkgS2FsbGUsDQo+DQo+IC0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0t
LS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0tLS0NCj4gR2FuYXBhdGhpIEJoYXQgPGdiaGF0QG1h
cnZlbGwuY29tPiB3cm90ZToNCj4NCj4gPiBGcm9tOiBTaHJlbmlrIFNoaWtoYXJlIDxzaHJlbmlr
QG1hcnZlbGwuY29tPg0KPiA+DQo+ID4gVGhlcmUgaXMgYSByYWNlIGNvbmRpdGlvbiBmb3IgYWNx
dWlyaW5nIHJ4X3Byb2NfbG9jayBiZXR3ZWVuIHJ4IHdvcmtlcg0KPiA+IHRocmVhZCBhbmQgVVNC
IFJYIGRhdGEgaW50ZXJydXB0DQo+ID4gKG13aWZpZXhfdXNiX3J4X2NvbXBsZXRlKToNCj4gPg0K
PiA+IDEuIFVTQiByZWNlaXZlcyBhbiBSWCBkYXRhIGludGVycnVwdCwgcXVldWVzIHJ4X3dvcmsg
Mi4gcnhfd29yaw0KPiA+IGVtcHRpZXMgcnhfZGF0YV9xLCB0cmllcyB0byBhY3F1aXJlIHJ4X3By
b2NfbG9jayAodG8gY2xlYXINCj4gPiByeF9wcm9jZXNzaW5nIGZsYWcpIDMuIFdoaWxlICMyIGlz
IHlldCB0byBhY3F1aXJlIHJ4X3Byb2NfbG9jaywgZHJpdmVyDQo+ID4gcmVjZWl2ZXMgY29udGlu
dW91cyBSWCBkYXRhIGludGVydXB0cyhtd2lmaWV4X3VzYl9yeF9jb21wbGV0ZSkNCj4gPiAzLiBG
b3IgZWFjaCBpbnRlcnJ1cHQgYXQgIzMsIGRyaXZlciBhY3F1aXJlcyByeF9wcm9jX2xvY2soaXQg
Z2V0cyB0aGUNCj4gPiBsb2NrIHNpbmNlIGl0IGlzIGluIGludGVycnVwdCBjb250ZXh0KSwgdHJp
ZXMgdG8gcXVldWUgcnhfd29yaywgYnV0DQo+ID4gZmFpbHMgdG8gZG8gc28gc2luY2UgcnhfcHJv
Y2Vzc2luZyBpcyBzdGlsbCBzZXQoIzIpIDQuIFdoZW4gcnhfcGVuZGluZw0KPiA+IGV4Y2VlZHMg
SElHSF9SWF9QRU5ESU5HLCBkcml2ZXIgc3RvcHMgc3VibWl0dGluZyBVUkJzIGJhY2sgdG8gVVNC
DQo+ID4gc3Vic3lzdGVtIGFuZCB0aHVzIGZpcm13YXJlIHN0b3BzIHVwbG9hZGluZyBSWCBkYXRh
IHRvIGRyaXZlciA1LiBOb3cNCj4gPiBmaW5hbGx5ICMyIHdpbGwgYWNxdWlyZSByeF9wcm9jX2xv
Y2ssIGJ1dCBiZWNhdXNlIG9mICM0LCB0aGVyZSBhcmUgbm8NCj4gPiBmdXJ0aGVyIHRyaWdnZXJz
IHRvIHNjaGVkdWxlIHJ4X3dvcmsgYWdhaW4NCj4gPg0KPiA+IFRoZSBhYm92ZSBzY2VuYXJpbyBv
Y2N1cnMgaW4gc29tZSBwbGF0Zm9ybXMgd2hlcmUgdGhlIFJYIHByb2Nlc3NpbmcgaXMNCj4gPiBj
b21wYXJpdGl2ZWx5IHNsb3dlci4gVGhpcyByZXN1bHRzIGluIFJYIHN0YWxsIGluIGRyaXZlciwg
Y29tbWFuZC9UWA0KPiA+IHRpbWVvdXRzIGluIGZpcm13YXJlLiBUaGUgYWJvdmUgc2NlbmFyaW8g
aXMgaW50cm9kdWNlZCBhZnRlciBjb21taXQNCj4gPiBjN2RiZGNiMmE0ZTENCj4gPiAoIm13aWZp
ZXg6IHNjaGVkdWxlIHJ4X3dvcmsgb24gUlggaW50ZXJydXB0IGZvciBVU0IiKQ0KPiA+DQo+ID4g
VG8gZml4IHRoaXMgc2V0IGEgbmV3IG1vcmVfcnhfdGFza19mbGFnIHdoZW5ldmVyIFJYIGRhdGEg
Y2FsbGJhY2sgaXMNCj4gPiB0cnlpbmcgdG8gc2NoZWR1bGUgcnhfd29yayBidXQgcnhfcHJvY2Vz
c2luZyBpcyBub3QgeWV0IGNsZWFyZWQuIFRoaXMNCj4gPiB3aWxsIGxldCB0aGUgY3VycmVudCBy
eF93b3JrKHdoaWNoIHdhcyB3YWl0aW5nIGZvcg0KPiA+IHJ4X3Byb2NfbG9jaykgdG8gbG9vcGJh
Y2sgYW5kIHByb2Nlc3MgbmV3bHkgYXJyaXZlZCBSWCBwYWNrZXRzLg0KPiA+DQo+ID4gU2lnbmVk
LW9mZi1ieTogQ2F0aHkgTHVvIDxjbHVvQG1hcnZlbGwuY29tPg0KPiA+IFNpZ25lZC1vZmYtYnk6
IEdhbmFwYXRoaSBCaGF0IDxnYmhhdEBtYXJ2ZWxsLmNvbT4NCj4NCj4gSSBjYW4ndCBmaW5kIGFu
eSBjb21taXQgd2l0aCBpZCBjN2RiZGNiMmE0ZTEsIGlzIGl0IGNvcnJlY3Q/DQpDb3JyZWN0LiBB
Y3R1YWxseSB0aGUgY29tbWl0IGlkIGM3ZGJkY2IyYTRlMSBpcyB0aGUgUEFUQ0ggWzEvMl0gc2Vu
dCBpbiB0aGlzIHNlcmllcy4NCj4NCj4gLS0NCj4gaHR0cHM6Ly9wYXRjaHdvcmsua2VybmVsLm9y
Zy9wYXRjaC8xMDE3ODcyOS8NCj4NCj4gaHR0cHM6Ly93aXJlbGVzcy53aWtpLmtlcm5lbC5vcmcv
ZW4vZGV2ZWxvcGVycy9kb2N1bWVudGF0aW9uL3N1Ym1pdHRpbmdwDQo+IGF0Y2hlcw0KUmVnYXJk
cywNCkdhbmFwYXRoaQ0K

RE: [EXT] Re: [2/2] mwifiex: use more_rx_task_flag to avoid USB RX stall

[Ganapathi Bhat]

Hi Kalle,

> ----------------------------------------------------------------------
> Kalle Valo <kvalo@codeaurora.org> writes:
>
> > Ganapathi Bhat <gbhat@marvell.com> wrote:
> >
> >> From: Shrenik Shikhare <shrenik@marvell.com>
> >>
> >> 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.
> >>
> >> Signed-off-by: Cathy Luo <cluo@marvell.com>
> >> Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>
> >
> > I can't find any commit with id c7dbdcb2a4e1, is it correct?
>
> Oh, and please use Fixes line to mark the commit which broke this.
Ok Sure.  I will do that and send v2.
>
> --
> Kalle Valo
Regards,
Ganapathi

Re: [EXT] Re: [2/2] mwifiex: use more_rx_task_flag to avoid USB RX stall

[Kalle Valo]

Ganapathi Bhat <gbhat@marvell.com> writes:

>> > 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.
>> >
>> > Signed-off-by: Cathy Luo <cluo@marvell.com>
>> > Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>
>>
>> I can't find any commit with id c7dbdcb2a4e1, is it correct?
>
> Correct. Actually the commit id c7dbdcb2a4e1 is the PATCH [1/2] sent in this series.

Actually the commit id will be different, I just tested it to be sure:

$ git reset --hard master
HEAD is now at b69c1df47281 brcmfmac: separate firmware errors from i/o errors
$ git am -s -3 1.patch
Applying: mwifiex: schedule rx_work on RX interrupt for USB
$ git log --oneline -1 | cat
676bc4833907 mwifiex: schedule rx_work on RX interrupt for USB
$ git reset --hard master
HEAD is now at b69c1df47281 brcmfmac: separate firmware errors from i/o errors
$ git am -s -3 1.patch
Applying: mwifiex: schedule rx_work on RX interrupt for USB
$ git log --oneline -1 | cat
74c5fc1d45b4 mwifiex: schedule rx_work on RX interrupt for USB
$ 

So the date, and most likely also the commiter, is included when
calculating the hash. So you can't really refer to uncommited patches
using a commit id as the id is determined only once the maintainer
applies the patch.

-- 
Kalle Valo

RE: [EXT] Re: [2/2] mwifiex: use more_rx_task_flag to avoid USB RX stall

[Ganapathi Bhat]

Hi Kalle,

> -----Original Message-----
> From: Kalle Valo [mailto:kvalo@codeaurora.org]
> Sent: Thursday, January 25, 2018 6:29 PM
> To: Ganapathi Bhat
> Cc: linux-wireless@vger.kernel.org; Brian Norris; Cathy Luo; Xinming Hu;
> Zhiyuan Yang; James Cao; Mangesh Malusare; Shrenik Shikhare
> Subject: Re: [EXT] Re: [2/2] mwifiex: use more_rx_task_flag to avoid USB RX
> stall
>
> Ganapathi Bhat <gbhat@marvell.com> writes:
>
> >> > 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.
> >> >
> >> > Signed-off-by: Cathy Luo <cluo@marvell.com>
> >> > Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>
> >>
> >> I can't find any commit with id c7dbdcb2a4e1, is it correct?
> >
> > Correct. Actually the commit id c7dbdcb2a4e1 is the PATCH [1/2] sent in this
> series.
>
> Actually the commit id will be different, I just tested it to be sure:
>
> $ git reset --hard master
> HEAD is now at b69c1df47281 brcmfmac: separate firmware errors from i/o
> errors $ git am -s -3 1.patch
> Applying: mwifiex: schedule rx_work on RX interrupt for USB $ git log --
> oneline -1 | cat
> 676bc4833907 mwifiex: schedule rx_work on RX interrupt for USB $ git reset -
> -hard master HEAD is now at b69c1df47281 brcmfmac: separate firmware
> errors from i/o errors $ git am -s -3 1.patch
> Applying: mwifiex: schedule rx_work on RX interrupt for USB $ git log --
> oneline -1 | cat
> 74c5fc1d45b4 mwifiex: schedule rx_work on RX interrupt for USB $
>
> So the date, and most likely also the commiter, is included when calculating
> the hash. So you can't really refer to uncommited patches using a commit id
> as the id is determined only once the maintainer applies the patch.
Ok. So, what can be done in this case. Should we wait for 1st patch to be committed and then send v3 of second patch?
>
> --
> Kalle Valo
Regards,
Ganapathi

Re: [1/2] mwifiex: schedule rx_work on RX interrupt for USB

[Brian Norris]

On Thu, Jan 25, 2018 at 07:10:52AM +0000, Kalle Valo wrote:
> Ganapathi Bhat <gbhat@marvell.com> wrote:
> 
> > From: Shrenik Shikhare <shrenik@marvell.com>
> > 
> > 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 <cluo@marvell.com>
> > Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>
> 
> Brian, did you have a chance to review these two?

Not really. I don't generally make a lot of time to review the USB
driver unless it's really screwing around with the main driver, since I
don't use the USB driver. But I'll try to give it a few glances.

Re: [EXT] Re: [2/2] mwifiex: use more_rx_task_flag to avoid USB RX stall

[Brian Norris]

On Thu, Jan 25, 2018 at 09:59:04AM +0000, Ganapathi Bhat wrote:
> > I can't find any commit with id c7dbdcb2a4e1, is it correct?
> Correct. Actually the commit id c7dbdcb2a4e1 is the PATCH [1/2] sent in this series.

What? Why would you introduce a bug and only fix it in the next patch?
Does that mean you should just combine the two? Or reverse the order, if
patch 2 doesn't cause problems on its own?

Brian

Re: [PATCH 1/2] mwifiex: schedule rx_work on RX interrupt for USB

[Brian Norris]

On Mon, Jan 22, 2018 at 08:34:56PM +0530, Ganapathi Bhat wrote:
> From: Shrenik Shikhare <shrenik@marvell.com>
> 
> 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

Stop right there.

There is a flag, data_received, and as you say, you are setting it one
thread, and reading it in another thread (and later clearing it; step
#5). Where is the locking? There is none. Therefore, you have a data
race.

You are not resolving any locking problems here, so you're not really
solving the entire problem.

Brian

> 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 <cluo@marvell.com>
> Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>
> ---
>  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
> 

RE: [EXT] Re: [2/2] mwifiex: use more_rx_task_flag to avoid USB RX stall

[Ganapathi Bhat]

Hi Brian,

> -----Original Message-----
> From: Brian Norris [mailto:briannorris@chromium.org]
> Sent: Friday, January 26, 2018 12:04 AM
> To: Ganapathi Bhat
> Cc: Kalle Valo; linux-wireless@vger.kernel.org; Cathy Luo; Xinming Hu;
> Zhiyuan Yang; James Cao; Mangesh Malusare; Shrenik Shikhare
> Subject: Re: [EXT] Re: [2/2] mwifiex: use more_rx_task_flag to avoid USB RX
> stall
>
> On Thu, Jan 25, 2018 at 09:59:04AM +0000, Ganapathi Bhat wrote:
> > > I can't find any commit with id c7dbdcb2a4e1, is it correct?
> > Correct. Actually the commit id c7dbdcb2a4e1 is the PATCH [1/2] sent in this
> series.
>
> What? Why would you introduce a bug and only fix it in the next patch?
With the first patch the original issue took considerably longer time to recreate. Also it followed a different path to get recreated(shared in commit message).
> Does that mean you should just combine the two?
Let us know if that is fine to merge them. We can modify the commit message accordingly.
> Or reverse the order, if patch 2 doesn't cause problems on its own?
Patch 2 has a dependency on patch 1.
>
> Brian

Regards,
Ganapathi

RE: [EXT] Re: [2/2] mwifiex: use more_rx_task_flag to avoid USB RX stall

[Ganapathi Bhat]

Hi Brian,

> -----Original Message-----
> From: Ganapathi Bhat
> Sent: Monday, January 29, 2018 12:47 PM
> To: 'Brian Norris'
> Cc: Kalle Valo; linux-wireless@vger.kernel.org; Cathy Luo; Xinming Hu;
> Zhiyuan Yang; James Cao; Mangesh Malusare; Shrenik Shikhare
> Subject: RE: [EXT] Re: [2/2] mwifiex: use more_rx_task_flag to avoid USB RX
> stall
>
> Hi Brian,
>
> > -----Original Message-----
> > From: Brian Norris [mailto:briannorris@chromium.org]
> > Sent: Friday, January 26, 2018 12:04 AM
> > To: Ganapathi Bhat
> > Cc: Kalle Valo; linux-wireless@vger.kernel.org; Cathy Luo; Xinming Hu;
> > Zhiyuan Yang; James Cao; Mangesh Malusare; Shrenik Shikhare
> > Subject: Re: [EXT] Re: [2/2] mwifiex: use more_rx_task_flag to avoid
> > USB RX stall
> >
> > On Thu, Jan 25, 2018 at 09:59:04AM +0000, Ganapathi Bhat wrote:
> > > > I can't find any commit with id c7dbdcb2a4e1, is it correct?
> > > Correct. Actually the commit id c7dbdcb2a4e1 is the PATCH [1/2] sent
> > > in this
> > series.
> >
> > What? Why would you introduce a bug and only fix it in the next patch?
> With the first patch the original issue took considerably longer time to
> recreate. Also it followed a different path to get recreated(shared in commit
> message).
> > Does that mean you should just combine the two?
> Let us know if that is fine to merge them. We can modify the commit
> message accordingly.
> > Or reverse the order, if patch 2 doesn't cause problems on its own?
> Patch 2 has a dependency on patch 1.
One correction. There is no commit dependency between patch 1 and 2(they can be applied in any order). But the result would be same. We need both fixes. Let us know if we can combine them.
> >
> > Brian
>
> Regards,
> Ganapathi

Regards,
Ganapathi

RE: [EXT] Re: [PATCH 1/2] mwifiex: schedule rx_work on RX interrupt for USB

[Ganapathi Bhat]

Hi Brian,
> -----Original Message-----
> From: Brian Norris [mailto:briannorris@chromium.org]
> Sent: Friday, January 26, 2018 12:30 AM
> To: Ganapathi Bhat
> Cc: linux-wireless@vger.kernel.org; Cathy Luo; Xinming Hu; Zhiyuan Yang;
> James Cao; Mangesh Malusare; Shrenik Shikhare
> Subject: [EXT] Re: [PATCH 1/2] mwifiex: schedule rx_work on RX interrupt for
> USB
>
> External Email
>
> ----------------------------------------------------------------------
> On Mon, Jan 22, 2018 at 08:34:56PM +0530, Ganapathi Bhat wrote:
> > From: Shrenik Shikhare <shrenik@marvell.com>
> >
> > 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
>
> Stop right there.
>
> There is a flag, data_received, and as you say, you are setting it one thread,
> and reading it in another thread (and later clearing it; step #5). Where is the
> locking? There is none. Therefore, you have a data race.
Yes. We missed it. We will add the locking and send it in v3.
>
> You are not resolving any locking problems here, so you're not really solving
> the entire problem.
>
> Brian
>
> > 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 <cluo@marvell.com>
> > Signed-off-by: Ganapathi Bhat <gbhat@marvell.com>
> > ---
> >  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
> >
Regards,
Ganapathi

Re: [EXT] Re: [2/2] mwifiex: use more_rx_task_flag to avoid USB RX stall

[Brian Norris]

Hi,

On Sun, Jan 28, 2018 at 11:19 PM, Ganapathi Bhat <gbhat@marvell.com> wrote:
>> From: Ganapathi Bhat
>> > From: Brian Norris [mailto:briannorris@chromium.org]
>> > On Thu, Jan 25, 2018 at 09:59:04AM +0000, Ganapathi Bhat wrote:
>> > > > I can't find any commit with id c7dbdcb2a4e1, is it correct?
>> > > Correct. Actually the commit id c7dbdcb2a4e1 is the PATCH [1/2] sent
>> > > in this
>> > series.
>> >
>> > What? Why would you introduce a bug and only fix it in the next patch?
>> With the first patch the original issue took considerably longer time to
>> recreate. Also it followed a different path to get recreated(shared in commit
>> message).
>> > Does that mean you should just combine the two?
>> Let us know if that is fine to merge them. We can modify the commit
>> message accordingly.
>> > Or reverse the order, if patch 2 doesn't cause problems on its own?
>> Patch 2 has a dependency on patch 1.
> One correction. There is no commit dependency between patch 1 and 2(they can be applied in any order). But the result would be same. We need both fixes. Let us know if we can combine them.

I haven't closely looked at patch 2 yet. My only statement was that it
makes no sense to have 2 commits, with the second one labeled as
"Fixing" the first one. From your descriptions, it sounds like patch 2
should actually come first, but I'm not really sure. I only looked far
enough to say that I didn't like patch 1 as-is :)

Brian

RE: [EXT] Re: [2/2] mwifiex: use more_rx_task_flag to avoid USB RX stall

[Ganapathi Bhat]

SGkgQnJpYW4sDQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogQnJpYW4g
Tm9ycmlzIFttYWlsdG86YnJpYW5ub3JyaXNAY2hyb21pdW0ub3JnXQ0KPiBTZW50OiBUdWVzZGF5
LCBKYW51YXJ5IDMwLCAyMDE4IDM6MzggQU0NCj4gVG86IEdhbmFwYXRoaSBCaGF0DQo+IENjOiBL
YWxsZSBWYWxvOyBsaW51eC13aXJlbGVzc0B2Z2VyLmtlcm5lbC5vcmc7IENhdGh5IEx1bzsgWGlu
bWluZyBIdTsNCj4gWmhpeXVhbiBZYW5nOyBKYW1lcyBDYW87IE1hbmdlc2ggTWFsdXNhcmU7IFNo
cmVuaWsgU2hpa2hhcmUNCj4gU3ViamVjdDogUmU6IFtFWFRdIFJlOiBbMi8yXSBtd2lmaWV4OiB1
c2UgbW9yZV9yeF90YXNrX2ZsYWcgdG8gYXZvaWQgVVNCIFJYDQo+IHN0YWxsDQo+DQo+IEhpLA0K
Pg0KPiBPbiBTdW4sIEphbiAyOCwgMjAxOCBhdCAxMToxOSBQTSwgR2FuYXBhdGhpIEJoYXQgPGdi
aGF0QG1hcnZlbGwuY29tPg0KPiB3cm90ZToNCj4gPj4gRnJvbTogR2FuYXBhdGhpIEJoYXQNCj4g
Pj4gPiBGcm9tOiBCcmlhbiBOb3JyaXMgW21haWx0bzpicmlhbm5vcnJpc0BjaHJvbWl1bS5vcmdd
IE9uIFRodSwgSmFuDQo+ID4+ID4gMjUsIDIwMTggYXQgMDk6NTk6MDRBTSArMDAwMCwgR2FuYXBh
dGhpIEJoYXQgd3JvdGU6DQo+ID4+ID4gPiA+IEkgY2FuJ3QgZmluZCBhbnkgY29tbWl0IHdpdGgg
aWQgYzdkYmRjYjJhNGUxLCBpcyBpdCBjb3JyZWN0Pw0KPiA+PiA+ID4gQ29ycmVjdC4gQWN0dWFs
bHkgdGhlIGNvbW1pdCBpZCBjN2RiZGNiMmE0ZTEgaXMgdGhlIFBBVENIIFsxLzJdDQo+ID4+ID4g
PiBzZW50IGluIHRoaXMNCj4gPj4gPiBzZXJpZXMuDQo+ID4+ID4NCj4gPj4gPiBXaGF0PyBXaHkg
d291bGQgeW91IGludHJvZHVjZSBhIGJ1ZyBhbmQgb25seSBmaXggaXQgaW4gdGhlIG5leHQgcGF0
Y2g/DQo+ID4+IFdpdGggdGhlIGZpcnN0IHBhdGNoIHRoZSBvcmlnaW5hbCBpc3N1ZSB0b29rIGNv
bnNpZGVyYWJseSBsb25nZXIgdGltZQ0KPiA+PiB0byByZWNyZWF0ZS4gQWxzbyBpdCBmb2xsb3dl
ZCBhIGRpZmZlcmVudCBwYXRoIHRvIGdldA0KPiA+PiByZWNyZWF0ZWQoc2hhcmVkIGluIGNvbW1p
dCBtZXNzYWdlKS4NCj4gPj4gPiBEb2VzIHRoYXQgbWVhbiB5b3Ugc2hvdWxkIGp1c3QgY29tYmlu
ZSB0aGUgdHdvPw0KPiA+PiBMZXQgdXMga25vdyBpZiB0aGF0IGlzIGZpbmUgdG8gbWVyZ2UgdGhl
bS4gV2UgY2FuIG1vZGlmeSB0aGUgY29tbWl0DQo+ID4+IG1lc3NhZ2UgYWNjb3JkaW5nbHkuDQo+
ID4+ID4gT3IgcmV2ZXJzZSB0aGUgb3JkZXIsIGlmIHBhdGNoIDIgZG9lc24ndCBjYXVzZSBwcm9i
bGVtcyBvbiBpdHMgb3duPw0KPiA+PiBQYXRjaCAyIGhhcyBhIGRlcGVuZGVuY3kgb24gcGF0Y2gg
MS4NCj4gPiBPbmUgY29ycmVjdGlvbi4gVGhlcmUgaXMgbm8gY29tbWl0IGRlcGVuZGVuY3kgYmV0
d2VlbiBwYXRjaCAxIGFuZA0KPiAyKHRoZXkgY2FuIGJlIGFwcGxpZWQgaW4gYW55IG9yZGVyKS4g
QnV0IHRoZSByZXN1bHQgd291bGQgYmUgc2FtZS4gV2UgbmVlZA0KPiBib3RoIGZpeGVzLiBMZXQg
dXMga25vdyBpZiB3ZSBjYW4gY29tYmluZSB0aGVtLg0KPg0KPiBJIGhhdmVuJ3QgY2xvc2VseSBs
b29rZWQgYXQgcGF0Y2ggMiB5ZXQuIE15IG9ubHkgc3RhdGVtZW50IHdhcyB0aGF0IGl0IG1ha2Vz
DQo+IG5vIHNlbnNlIHRvIGhhdmUgMiBjb21taXRzLCB3aXRoIHRoZSBzZWNvbmQgb25lIGxhYmVs
ZWQgYXMgIkZpeGluZyIgdGhlIGZpcnN0DQo+IG9uZS4gRnJvbSB5b3VyIGRlc2NyaXB0aW9ucywg
aXQgc291bmRzIGxpa2UgcGF0Y2ggMiBzaG91bGQgYWN0dWFsbHkgY29tZSBmaXJzdCwNCk9rLiBJ
IHVuZGVyc3RhbmQuIEkgd2lsbCByZW9yZGVyIHRoZW0gYW5kIHNlbmQgdjMoYWxvbmcgd2l0aCBz
cGlubG9jayBjaGFuZ2UpLg0KPiBidXQgSSdtIG5vdCByZWFsbHkgc3VyZS4gSSBvbmx5IGxvb2tl
ZCBmYXIgZW5vdWdoIHRvIHNheSB0aGF0IEkgZGlkbid0IGxpa2UgcGF0Y2gNCj4gMSBhcy1pcyA6
KQ0KPg0KPiBCcmlhbg0KUmVnYXJkcywNCkdhbmFwYXRoaQ0K

RE: [EXT] Re: [2/2] mwifiex: use more_rx_task_flag to avoid USB RX stall

[Ganapathi Bhat]

SGkgQnJpYW4sDQoNCj4gLS0tLS1PcmlnaW5hbCBNZXNzYWdlLS0tLS0NCj4gRnJvbTogR2FuYXBh
dGhpIEJoYXQNCj4gU2VudDogVHVlc2RheSwgSmFudWFyeSAzMCwgMjAxOCA4OjU1IFBNDQo+IFRv
OiAnQnJpYW4gTm9ycmlzJw0KPiBDYzogS2FsbGUgVmFsbzsgbGludXgtd2lyZWxlc3NAdmdlci5r
ZXJuZWwub3JnOyBDYXRoeSBMdW87IFhpbm1pbmcgSHU7DQo+IFpoaXl1YW4gWWFuZzsgSmFtZXMg
Q2FvOyBNYW5nZXNoIE1hbHVzYXJlOyBTaHJlbmlrIFNoaWtoYXJlDQo+IFN1YmplY3Q6IFJFOiBb
RVhUXSBSZTogWzIvMl0gbXdpZmlleDogdXNlIG1vcmVfcnhfdGFza19mbGFnIHRvIGF2b2lkIFVT
QiBSWA0KPiBzdGFsbA0KPg0KPiBIaSBCcmlhbiwNCj4NCj4gPiAtLS0tLU9yaWdpbmFsIE1lc3Nh
Z2UtLS0tLQ0KPiA+IEZyb206IEJyaWFuIE5vcnJpcyBbbWFpbHRvOmJyaWFubm9ycmlzQGNocm9t
aXVtLm9yZ10NCj4gPiBTZW50OiBUdWVzZGF5LCBKYW51YXJ5IDMwLCAyMDE4IDM6MzggQU0NCj4g
PiBUbzogR2FuYXBhdGhpIEJoYXQNCj4gPiBDYzogS2FsbGUgVmFsbzsgbGludXgtd2lyZWxlc3NA
dmdlci5rZXJuZWwub3JnOyBDYXRoeSBMdW87IFhpbm1pbmcgSHU7DQo+ID4gWmhpeXVhbiBZYW5n
OyBKYW1lcyBDYW87IE1hbmdlc2ggTWFsdXNhcmU7IFNocmVuaWsgU2hpa2hhcmUNCj4gPiBTdWJq
ZWN0OiBSZTogW0VYVF0gUmU6IFsyLzJdIG13aWZpZXg6IHVzZSBtb3JlX3J4X3Rhc2tfZmxhZyB0
byBhdm9pZA0KPiA+IFVTQiBSWCBzdGFsbA0KPiA+DQo+ID4gSGksDQo+ID4NCj4gPiBPbiBTdW4s
IEphbiAyOCwgMjAxOCBhdCAxMToxOSBQTSwgR2FuYXBhdGhpIEJoYXQgPGdiaGF0QG1hcnZlbGwu
Y29tPg0KPiA+IHdyb3RlOg0KPiA+ID4+IEZyb206IEdhbmFwYXRoaSBCaGF0DQo+ID4gPj4gPiBG
cm9tOiBCcmlhbiBOb3JyaXMgW21haWx0bzpicmlhbm5vcnJpc0BjaHJvbWl1bS5vcmddIE9uIFRo
dSwgSmFuDQo+ID4gPj4gPiAyNSwgMjAxOCBhdCAwOTo1OTowNEFNICswMDAwLCBHYW5hcGF0aGkg
QmhhdCB3cm90ZToNCj4gPiA+PiA+ID4gPiBJIGNhbid0IGZpbmQgYW55IGNvbW1pdCB3aXRoIGlk
IGM3ZGJkY2IyYTRlMSwgaXMgaXQgY29ycmVjdD8NCj4gPiA+PiA+ID4gQ29ycmVjdC4gQWN0dWFs
bHkgdGhlIGNvbW1pdCBpZCBjN2RiZGNiMmE0ZTEgaXMgdGhlIFBBVENIIFsxLzJdDQo+ID4gPj4g
PiA+IHNlbnQgaW4gdGhpcw0KPiA+ID4+ID4gc2VyaWVzLg0KPiA+ID4+ID4NCj4gPiA+PiA+IFdo
YXQ/IFdoeSB3b3VsZCB5b3UgaW50cm9kdWNlIGEgYnVnIGFuZCBvbmx5IGZpeCBpdCBpbiB0aGUg
bmV4dA0KPiBwYXRjaD8NCj4gPiA+PiBXaXRoIHRoZSBmaXJzdCBwYXRjaCB0aGUgb3JpZ2luYWwg
aXNzdWUgdG9vayBjb25zaWRlcmFibHkgbG9uZ2VyDQo+ID4gPj4gdGltZSB0byByZWNyZWF0ZS4g
QWxzbyBpdCBmb2xsb3dlZCBhIGRpZmZlcmVudCBwYXRoIHRvIGdldA0KPiA+ID4+IHJlY3JlYXRl
ZChzaGFyZWQgaW4gY29tbWl0IG1lc3NhZ2UpLg0KPiA+ID4+ID4gRG9lcyB0aGF0IG1lYW4geW91
IHNob3VsZCBqdXN0IGNvbWJpbmUgdGhlIHR3bz8NCj4gPiA+PiBMZXQgdXMga25vdyBpZiB0aGF0
IGlzIGZpbmUgdG8gbWVyZ2UgdGhlbS4gV2UgY2FuIG1vZGlmeSB0aGUgY29tbWl0DQo+ID4gPj4g
bWVzc2FnZSBhY2NvcmRpbmdseS4NCj4gPiA+PiA+IE9yIHJldmVyc2UgdGhlIG9yZGVyLCBpZiBw
YXRjaCAyIGRvZXNuJ3QgY2F1c2UgcHJvYmxlbXMgb24gaXRzIG93bj8NCj4gPiA+PiBQYXRjaCAy
IGhhcyBhIGRlcGVuZGVuY3kgb24gcGF0Y2ggMS4NCj4gPiA+IE9uZSBjb3JyZWN0aW9uLiBUaGVy
ZSBpcyBubyBjb21taXQgZGVwZW5kZW5jeSBiZXR3ZWVuIHBhdGNoIDEgYW5kDQo+ID4gMih0aGV5
IGNhbiBiZSBhcHBsaWVkIGluIGFueSBvcmRlcikuIEJ1dCB0aGUgcmVzdWx0IHdvdWxkIGJlIHNh
bWUuIFdlDQo+ID4gbmVlZCBib3RoIGZpeGVzLiBMZXQgdXMga25vdyBpZiB3ZSBjYW4gY29tYmlu
ZSB0aGVtLg0KPiA+DQo+ID4gSSBoYXZlbid0IGNsb3NlbHkgbG9va2VkIGF0IHBhdGNoIDIgeWV0
LiBNeSBvbmx5IHN0YXRlbWVudCB3YXMgdGhhdCBpdA0KPiA+IG1ha2VzIG5vIHNlbnNlIHRvIGhh
dmUgMiBjb21taXRzLCB3aXRoIHRoZSBzZWNvbmQgb25lIGxhYmVsZWQgYXMNCj4gPiAiRml4aW5n
IiB0aGUgZmlyc3Qgb25lLiBGcm9tIHlvdXIgZGVzY3JpcHRpb25zLCBpdCBzb3VuZHMgbGlrZSBw
YXRjaCAyDQo+ID4gc2hvdWxkIGFjdHVhbGx5IGNvbWUgZmlyc3QsDQo+IE9rLiBJIHVuZGVyc3Rh
bmQuIEkgd2lsbCByZW9yZGVyIHRoZW0gYW5kIHNlbmQgdjMoYWxvbmcgd2l0aCBzcGlubG9jaw0K
PiBjaGFuZ2UpLg0KSSB3YXMgdHJ5aW5nIHRvIHJlb3JkZXIgdGhlIHBhdGNoIGJ1dCBmb3VuZCBw
YXRjaCAyIGlzIGluZGVlZCBkZXBlbmRlbnQgb24gcGF0Y2ggMS4gQ29uc2lkZXIgdGhlIGZpcnN0
IHBvaW50IGluIGNvbW1pdCBtZXNzYWdlIG9mIHBhdGNoIDI6DQoxLiBVU0IgcmVjZWl2ZXMgYW4g
UlggZGF0YSBpbnRlcnJ1cHQsIHF1ZXVlcyByeF93b3JrDQpUaGlzIGlzIHRoZSBjaGFuZ2UgYWRk
ZWQgaW4gcGF0Y2ggMS4gRWFybGllciBvbiByZWNlaXZlIG9mIFJYIGRhdGEgaW50ZXJydXB0LCBk
cml2ZXIgd291bGQgcXVldWUgbWFpbl93b3JrLCB3aGljaCBpbiB0dXJuIHF1ZXVlZCByeF93b3Jr
LiBCdXQgYWZ0ZXIgcGF0Y2ggMSBkcml2ZXIgdHJpZXMgdG8gIHF1ZXVlIHJ4X3dvcmsgaW4gaW50
ZXJydXB0IGNvbnRleHQuDQoNCkxldCB1cyBwbGVhc2Uga25vdyB5b3VyIHRob3VnaHRzIG9uIHRo
aXMuDQoNCj4gPiBidXQgSSdtIG5vdCByZWFsbHkgc3VyZS4gSSBvbmx5IGxvb2tlZCBmYXIgZW5v
dWdoIHRvIHNheSB0aGF0IEkgZGlkbid0DQo+ID4gbGlrZSBwYXRjaA0KPiA+IDEgYXMtaXMgOikN
Cj4gPg0KPiA+IEJyaWFuDQo+IFJlZ2FyZHMsDQo+IEdhbmFwYXRoaQ0K