Re: [PATCH 1/2] mwifiex: resubmit failed to submit RX URBs in main thread

[Kalle Valo]

Ganapathi Bhat  writes:

> From: James Cao 
>
> Current driver has 6 Rx data URBs. Once any packet received
> kernel calls our callback, in which the same URB will be
> resubmitted after Rx indication. In URB submission function a new
> skb will be allocated since the previous one is passed to upper
> layer (freed later). Since the skb is from a special pool (not
> regular memory), skb allocation may fail when kernel holds a lot
> of Rx packets on some low resource platforms.

The special pool being GFP_ATOMIC allocations or what?

> The URB will not be resubmitted in this no free skb case. If driver
> fails to resubmit all 6 URBs, Rx will stop. To cover this scenario
> check and resubmit Rx URBs in main thread.
>
> Signed-off-by: James Cao 
> Signed-off-by: Cathy Luo 
> Signed-off-by: Ganapathi Bhat 

[...]

> @@ -278,6 +279,16 @@ int mwifiex_main_process(struct mwifiex_adapter *adapter)
>   break;
>   }
>  
> + /* Try to resubmit RX URB if sunmission failed earlier */
> + if (!atomic_read(>rx_pending) &&
> + adapter->iface_type == MWIFIEX_USB) {
> + usb_card = adapter->card;
> + if (atomic_read(_card->rx_data_urb_pending) <
> + MWIFIEX_RX_DATA_URB &&
> + adapter->if_ops.submit_rem_rx_urbs)
> + adapter->if_ops.submit_rem_rx_urbs(adapter);
> + }

To me this just feels wrong. Normally the proceduce is to drop the frame
if allocations fail, not try to reallocate. I need more convincing that
this really is the right approach.

-- 
Kalle Valo

[PATCH 1/2] mwifiex: resubmit failed to submit RX URBs in main thread

[Ganapathi Bhat]

From: James Cao 

Current driver has 6 Rx data URBs. Once any packet received
kernel calls our callback, in which the same URB will be
resubmitted after Rx indication. In URB submission function a new
skb will be allocated since the previous one is passed to upper
layer (freed later). Since the skb is from a special pool (not
regular memory), skb allocation may fail when kernel holds a lot
of Rx packets on some low resource platforms. The URB will not be
resubmitted in this no free skb case. If driver fails to resubmit
all 6 URBs, Rx will stop. To cover this scenario check and
resubmit Rx URBs in main thread.

Signed-off-by: James Cao 
Signed-off-by: Cathy Luo 
Signed-off-by: Ganapathi Bhat 
---
 drivers/net/wireless/marvell/mwifiex/main.c | 11 +++
 1 file changed, 11 insertions(+)

diff --git a/drivers/net/wireless/marvell/mwifiex/main.c 
b/drivers/net/wireless/marvell/mwifiex/main.c
index ee40b73..c78014b 100644
--- a/drivers/net/wireless/marvell/mwifiex/main.c
+++ b/drivers/net/wireless/marvell/mwifiex/main.c
@@ -247,6 +247,7 @@ int mwifiex_main_process(struct mwifiex_adapter *adapter)
 {
int ret = 0;
unsigned long flags;
+   struct usb_card_rec *usb_card;
 
spin_lock_irqsave(>main_proc_lock, flags);
 
@@ -278,6 +279,16 @@ int mwifiex_main_process(struct mwifiex_adapter *adapter)
break;
}
 
+   /* Try to resubmit RX URB if sunmission failed earlier */
+   if (!atomic_read(>rx_pending) &&
+   adapter->iface_type == MWIFIEX_USB) {
+   usb_card = adapter->card;
+   if (atomic_read(_card->rx_data_urb_pending) <
+   MWIFIEX_RX_DATA_URB &&
+   adapter->if_ops.submit_rem_rx_urbs)
+   adapter->if_ops.submit_rem_rx_urbs(adapter);
+   }
+
/* Handle pending interrupt if any */
if (adapter->int_status) {
if (adapter->hs_activated)
-- 
1.9.1