Message ID | 1504122674-3379-2-git-send-email-gbhat@marvell.com (mailing list archive) |
---|---|
State | Changes Requested |
Delegated to: | Kalle Valo |
Headers | show |
Ganapathi Bhat <gbhat@marvell.com> writes: > From: James Cao <jcao@marvell.com> > > 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 <jcao@marvell.com> > Signed-off-by: Cathy Luo <cluo@marvell.com> > Signed-off-by: Ganapathi Bhat <gbhat@marvell.com> [...] > @@ -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(&adapter->rx_pending) && > + adapter->iface_type == MWIFIEX_USB) { > + usb_card = adapter->card; > + if (atomic_read(&usb_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.
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(&adapter->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(&adapter->rx_pending) && + adapter->iface_type == MWIFIEX_USB) { + usb_card = adapter->card; + if (atomic_read(&usb_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)