Re: [Patch v2 3/5] mwifiex: fix out of memory issue observed for USB chipsets

Re: [Patch v2 3/5] mwifiex: fix out of memory issue observed for USB chipsets

[James Cameron]

On Wed, Nov 05, 2014 at 05:04:29PM +0530, Amitkumar Karwar wrote:
> On some platforms, system goes out of memory during heavy
> Rx traffic with our USB chipsets.
> 
> In case of SDIO/PCIe, after receiving 50 packets in Rx queue
> we stop processing interrupts till packets pending fall below
> low threshold i.e 20. We don't have similar logic for USB,
> so if host platform is slow, we would hit a case where firmware
> keeps on pushing packets at high speed than driver/kernel can
> process.
> 
> We will stop submitting URBs for Rx data when pending packet
> count reaches high threshold and restart them when enough
> packets are consumed to solve the problem.

Other drivers and user activity can deplete memory.  How does this
patch solve the problem when dev_alloc_skb fails?  I'm worried the
underlying issue remains; handling out of memory.

-- 
James Cameron
http://quozl.linux.org.au/

RE: [Patch v2 3/5] mwifiex: fix out of memory issue observed for USB chipsets

[Amitkumar Karwar]

Hi James,

>> We will stop submitting URBs for Rx data when pending packet count
>> reaches high threshold and restart them when enough packets are
>> consumed to solve the problem.
>
>Other drivers and user activity can deplete memory.  How does this patch
>solve the problem when dev_alloc_skb fails?  I'm worried the underlying
>issue remains; handling out of memory.
>

This patch basically avoids out of memory issue caused by our driver. It also solves the problem when dev_alloc_skb fails by other reasons.

When dev_alloc_skb is failed in mwifiex_usb_submit_rx_urb(), we skip submitting that particular URB and "ctx->skb" remains NULL. Later when pending packets are being consumed in Rx work queue, "if_ops.submit_rem_rx_urb()" call takes care of submitting the URB skipped previously.

Please let me know you have any doubts. 

Regards,
Amitkumar

[Patch v2 1/5] mwifiex: rx workqueue support for USB interface

[Amitkumar Karwar]

From: Avinash Patil <patila@marvell.com>

This patch adds RX workqueue support for USB interfaces.
Currently rx_pending is applicable for cmd/events and Rx
data in USB interface. Let's use it only for Rx data.

Signed-off-by: Avinash Patil <patila@marvell.com>
Signed-off-by: Cathy Luo <cluo@marvell.com>
Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
---
 drivers/net/wireless/mwifiex/init.c | 14 --------------
 drivers/net/wireless/mwifiex/main.c |  8 +-------
 drivers/net/wireless/mwifiex/main.h |  6 +-----
 drivers/net/wireless/mwifiex/usb.c  |  9 +++------
 4 files changed, 5 insertions(+), 32 deletions(-)

diff --git a/drivers/net/wireless/mwifiex/init.c b/drivers/net/wireless/mwifiex/init.c
index 580aa45..bd740b6 100644
--- a/drivers/net/wireless/mwifiex/init.c
+++ b/drivers/net/wireless/mwifiex/init.c
@@ -449,7 +449,6 @@ int mwifiex_init_lock_list(struct mwifiex_adapter *adapter)
 	spin_lock_init(&adapter->scan_pending_q_lock);
 	spin_lock_init(&adapter->rx_proc_lock);
 
-	skb_queue_head_init(&adapter->usb_rx_data_q);
 	skb_queue_head_init(&adapter->rx_data_q);
 
 	for (i = 0; i < adapter->priv_num; ++i) {
@@ -668,19 +667,6 @@ mwifiex_shutdown_drv(struct mwifiex_adapter *adapter)
 
 	spin_lock(&adapter->mwifiex_lock);
 
-	if (adapter->if_ops.data_complete) {
-		while ((skb = skb_dequeue(&adapter->usb_rx_data_q))) {
-			struct mwifiex_rxinfo *rx_info = MWIFIEX_SKB_RXCB(skb);
-
-			priv = adapter->priv[rx_info->bss_num];
-			if (priv)
-				priv->stats.rx_dropped++;
-
-			dev_kfree_skb_any(skb);
-			adapter->if_ops.data_complete(adapter);
-		}
-	}
-
 	mwifiex_adapter_cleanup(adapter);
 
 	spin_unlock(&adapter->mwifiex_lock);
diff --git a/drivers/net/wireless/mwifiex/main.c b/drivers/net/wireless/mwifiex/main.c
index f26420d..cb23ca3 100644
--- a/drivers/net/wireless/mwifiex/main.c
+++ b/drivers/net/wireless/mwifiex/main.c
@@ -178,7 +178,6 @@ int mwifiex_main_process(struct mwifiex_adapter *adapter)
 {
 	int ret = 0;
 	unsigned long flags;
-	struct sk_buff *skb;
 
 	spin_lock_irqsave(&adapter->main_proc_lock, flags);
 
@@ -253,11 +252,6 @@ process_start:
 			}
 		}
 
-		/* Check Rx data for USB */
-		if (adapter->iface_type == MWIFIEX_USB)
-			while ((skb = skb_dequeue(&adapter->usb_rx_data_q)))
-				mwifiex_handle_rx_packet(adapter, skb);
-
 		/* Check for event */
 		if (adapter->event_received) {
 			adapter->event_received = false;
@@ -864,7 +858,7 @@ mwifiex_add_card(void *card, struct semaphore *sem,
 	adapter->cmd_wait_q.status = 0;
 	adapter->scan_wait_q_woken = false;
 
-	if (num_possible_cpus() > 1) {
+	if ((num_possible_cpus() > 1) || adapter->iface_type == MWIFIEX_USB) {
 		adapter->rx_work_enabled = true;
 		pr_notice("rx work enabled, cpus %d\n", num_possible_cpus());
 	}
diff --git a/drivers/net/wireless/mwifiex/main.h b/drivers/net/wireless/mwifiex/main.h
index cb39319..a9eea9b 100644
--- a/drivers/net/wireless/mwifiex/main.h
+++ b/drivers/net/wireless/mwifiex/main.h
@@ -106,10 +106,7 @@ enum {
  */
 #define IS_CARD_RX_RCVD(adapter) (adapter->cmd_resp_received || \
 				adapter->event_received || \
-				((adapter->iface_type != MWIFIEX_USB) && \
-				adapter->data_received) || \
-				((adapter->iface_type == MWIFIEX_USB) && \
-				!skb_queue_empty(&adapter->usb_rx_data_q)))
+				adapter->data_received)
 
 #define MWIFIEX_TYPE_CMD				1
 #define MWIFIEX_TYPE_DATA				0
@@ -767,7 +764,6 @@ struct mwifiex_adapter {
 	spinlock_t scan_pending_q_lock;
 	/* spin lock for RX processing routine */
 	spinlock_t rx_proc_lock;
-	struct sk_buff_head usb_rx_data_q;
 	u32 scan_processing;
 	u16 region_code;
 	struct mwifiex_802_11d_domain_reg domain_reg;
diff --git a/drivers/net/wireless/mwifiex/usb.c b/drivers/net/wireless/mwifiex/usb.c
index 4371e12..00afcf6 100644
--- a/drivers/net/wireless/mwifiex/usb.c
+++ b/drivers/net/wireless/mwifiex/usb.c
@@ -125,8 +125,10 @@ static int mwifiex_usb_recv(struct mwifiex_adapter *adapter,
 			dev_err(dev, "DATA: skb->len too large\n");
 			return -1;
 		}
-		skb_queue_tail(&adapter->usb_rx_data_q, skb);
+
+		skb_queue_tail(&adapter->rx_data_q, skb);
 		adapter->data_received = true;
+		atomic_inc(&adapter->rx_pending);
 		break;
 	default:
 		dev_err(dev, "%s: unknown endport %#x\n", __func__, ep);
@@ -176,7 +178,6 @@ static void mwifiex_usb_rx_complete(struct urb *urb)
 		else
 			skb_put(skb, recv_length - skb->len);
 
-		atomic_inc(&adapter->rx_pending);
 		status = mwifiex_usb_recv(adapter, skb, context->ep);
 
 		dev_dbg(adapter->dev, "info: recv_length=%d, status=%d\n",
@@ -191,7 +192,6 @@ static void mwifiex_usb_rx_complete(struct urb *urb)
 			if (card->rx_cmd_ep == context->ep)
 				return;
 		} else {
-			atomic_dec(&adapter->rx_pending);
 			if (status == -1)
 				dev_err(adapter->dev,
 					"received data processing failed!\n");
@@ -962,7 +962,6 @@ static void mwifiex_submit_rx_urb(struct mwifiex_adapter *adapter, u8 ep)
 static int mwifiex_usb_cmd_event_complete(struct mwifiex_adapter *adapter,
 				       struct sk_buff *skb)
 {
-	atomic_dec(&adapter->rx_pending);
 	mwifiex_submit_rx_urb(adapter, MWIFIEX_USB_EP_CMD_EVENT);
 
 	return 0;
@@ -970,8 +969,6 @@ static int mwifiex_usb_cmd_event_complete(struct mwifiex_adapter *adapter,
 
 static int mwifiex_usb_data_complete(struct mwifiex_adapter *adapter)
 {
-	atomic_dec(&adapter->rx_pending);
-
 	return 0;
 }
 
-- 
1.8.1.4

[Patch v2 2/5] mwifiex: remove data_complete handler

[Amitkumar Karwar]

From: Avinash Patil <patila@marvell.com>

This patch removes redundant data complete handler.

Signed-off-by: Avinash Patil <patila@marvell.com>
Signed-off-by: Cathy Luo <cluo@marvell.com>
Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
---
 drivers/net/wireless/mwifiex/main.h | 1 -
 drivers/net/wireless/mwifiex/txrx.c | 4 ----
 drivers/net/wireless/mwifiex/usb.c  | 6 ------
 3 files changed, 11 deletions(-)

diff --git a/drivers/net/wireless/mwifiex/main.h b/drivers/net/wireless/mwifiex/main.h
index a9eea9b..df6f877 100644
--- a/drivers/net/wireless/mwifiex/main.h
+++ b/drivers/net/wireless/mwifiex/main.h
@@ -687,7 +687,6 @@ struct mwifiex_if_ops {
 	void (*cleanup_mpa_buf) (struct mwifiex_adapter *);
 	int (*cmdrsp_complete) (struct mwifiex_adapter *, struct sk_buff *);
 	int (*event_complete) (struct mwifiex_adapter *, struct sk_buff *);
-	int (*data_complete) (struct mwifiex_adapter *);
 	int (*init_fw_port) (struct mwifiex_adapter *);
 	int (*dnld_fw) (struct mwifiex_adapter *, struct mwifiex_fw_image *);
 	void (*card_reset) (struct mwifiex_adapter *);
diff --git a/drivers/net/wireless/mwifiex/txrx.c b/drivers/net/wireless/mwifiex/txrx.c
index 96a2126..a5983fc 100644
--- a/drivers/net/wireless/mwifiex/txrx.c
+++ b/drivers/net/wireless/mwifiex/txrx.c
@@ -64,10 +64,6 @@ int mwifiex_handle_rx_packet(struct mwifiex_adapter *adapter,
 	else
 		ret = mwifiex_process_sta_rx_packet(priv, skb);
 
-	/* Decrement RX pending counter for each packet */
-	if (adapter->if_ops.data_complete)
-		adapter->if_ops.data_complete(adapter);
-
 	return ret;
 }
 EXPORT_SYMBOL_GPL(mwifiex_handle_rx_packet);
diff --git a/drivers/net/wireless/mwifiex/usb.c b/drivers/net/wireless/mwifiex/usb.c
index 00afcf6..08e9ec0 100644
--- a/drivers/net/wireless/mwifiex/usb.c
+++ b/drivers/net/wireless/mwifiex/usb.c
@@ -967,11 +967,6 @@ static int mwifiex_usb_cmd_event_complete(struct mwifiex_adapter *adapter,
 	return 0;
 }
 
-static int mwifiex_usb_data_complete(struct mwifiex_adapter *adapter)
-{
-	return 0;
-}
-
 /* This function wakes up the card. */
 static int mwifiex_pm_wakeup_card(struct mwifiex_adapter *adapter)
 {
@@ -993,7 +988,6 @@ static struct mwifiex_if_ops usb_ops = {
 	.dnld_fw =		mwifiex_usb_dnld_fw,
 	.cmdrsp_complete =	mwifiex_usb_cmd_event_complete,
 	.event_complete =	mwifiex_usb_cmd_event_complete,
-	.data_complete =	mwifiex_usb_data_complete,
 	.host_to_card =		mwifiex_usb_host_to_card,
 };
 
-- 
1.8.1.4

[Patch v2 3/5] mwifiex: fix out of memory issue observed for USB chipsets

[Amitkumar Karwar]

On some platforms, system goes out of memory during heavy
Rx traffic with our USB chipsets.

In case of SDIO/PCIe, after receiving 50 packets in Rx queue
we stop processing interrupts till packets pending fall below
low threshold i.e 20. We don't have similar logic for USB,
so if host platform is slow, we would hit a case where firmware
keeps on pushing packets at high speed than driver/kernel can
process.

We will stop submitting URBs for Rx data when pending packet
count reaches high threshold and restart them when enough
packets are consumed to solve the problem.

BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=85071
Reported-by: Marek Belisko <marek.belisko@gmail.com>
Tested-by: Marek Belisko <marek.belisko@gmail.com>
Signed-off-by: Avinash Patil <patila@marvell.com>
Signed-off-by: Cathy Luo <cluo@marvell.com>
Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
---
 drivers/net/wireless/mwifiex/main.c |  2 ++
 drivers/net/wireless/mwifiex/main.h |  1 +
 drivers/net/wireless/mwifiex/usb.c  | 23 ++++++++++++++++++++++-
 3 files changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/mwifiex/main.c b/drivers/net/wireless/mwifiex/main.c
index cb23ca3..2a5a59b 100644
--- a/drivers/net/wireless/mwifiex/main.c
+++ b/drivers/net/wireless/mwifiex/main.c
@@ -146,6 +146,8 @@ static int mwifiex_process_rx(struct mwifiex_adapter *adapter)
 		atomic_dec(&adapter->rx_pending);
 		if (adapter->delay_main_work &&
 		    (atomic_read(&adapter->rx_pending) < LOW_RX_PENDING)) {
+			if (adapter->if_ops.submit_rem_rx_urbs)
+				adapter->if_ops.submit_rem_rx_urbs(adapter);
 			adapter->delay_main_work = false;
 			queue_work(adapter->workqueue, &adapter->main_work);
 		}
diff --git a/drivers/net/wireless/mwifiex/main.h b/drivers/net/wireless/mwifiex/main.h
index df6f877..5feaffb 100644
--- a/drivers/net/wireless/mwifiex/main.h
+++ b/drivers/net/wireless/mwifiex/main.h
@@ -693,6 +693,7 @@ struct mwifiex_if_ops {
 	void (*fw_dump)(struct mwifiex_adapter *);
 	int (*clean_pcie_ring) (struct mwifiex_adapter *adapter);
 	void (*iface_work)(struct work_struct *work);
+	void (*submit_rem_rx_urbs)(struct mwifiex_adapter *adapter);
 };
 
 struct mwifiex_adapter {
diff --git a/drivers/net/wireless/mwifiex/usb.c b/drivers/net/wireless/mwifiex/usb.c
index 08e9ec0..6cc8519 100644
--- a/drivers/net/wireless/mwifiex/usb.c
+++ b/drivers/net/wireless/mwifiex/usb.c
@@ -222,7 +222,13 @@ setup_for_next:
 	else
 		size = MWIFIEX_RX_DATA_BUF_SIZE;
 
-	mwifiex_usb_submit_rx_urb(context, size);
+	if (card->rx_cmd_ep == context->ep) {
+		mwifiex_usb_submit_rx_urb(context, size);
+	} else {
+		context->skb = NULL;
+		if (atomic_read(&adapter->rx_pending) <= HIGH_RX_PENDING)
+			mwifiex_usb_submit_rx_urb(context, size);
+	}
 
 	return;
 }
@@ -978,6 +984,20 @@ static int mwifiex_pm_wakeup_card(struct mwifiex_adapter *adapter)
 	return 0;
 }
 
+static void mwifiex_usb_submit_rem_rx_urbs(struct mwifiex_adapter *adapter)
+{
+	struct usb_card_rec *card = (struct usb_card_rec *)adapter->card;
+	int i;
+	struct urb_context *ctx;
+
+	for (i = 0; i < MWIFIEX_RX_DATA_URB; i++) {
+		if (card->rx_data_list[i].skb)
+			continue;
+		ctx = &card->rx_data_list[i];
+		mwifiex_usb_submit_rx_urb(ctx, MWIFIEX_RX_DATA_BUF_SIZE);
+	}
+}
+
 static struct mwifiex_if_ops usb_ops = {
 	.register_dev =		mwifiex_register_dev,
 	.unregister_dev =	mwifiex_unregister_dev,
@@ -989,6 +1009,7 @@ static struct mwifiex_if_ops usb_ops = {
 	.cmdrsp_complete =	mwifiex_usb_cmd_event_complete,
 	.event_complete =	mwifiex_usb_cmd_event_complete,
 	.host_to_card =		mwifiex_usb_host_to_card,
+	.submit_rem_rx_urbs =	mwifiex_usb_submit_rem_rx_urbs,
 };
 
 /* This function initializes the USB driver module.
-- 
1.8.1.4

[Patch v2 4/5] mwifiex: do not setup AMPDU/AMSDU with broadcast receiver

[Amitkumar Karwar]

From: Avinash Patil <patila@marvell.com>

It is observed that device sometimes sends BA setup requests for
broadcast mac address.
This patch adds a check to avoid checking availability of
AMPDU/AMSDU streams for broadcast mac address.

Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
Signed-off-by: Avinash Patil <patila@marvell.com>
---
 drivers/net/wireless/mwifiex/11n.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/net/wireless/mwifiex/11n.h b/drivers/net/wireless/mwifiex/11n.h
index 2ee268b..f275675 100644
--- a/drivers/net/wireless/mwifiex/11n.h
+++ b/drivers/net/wireless/mwifiex/11n.h
@@ -84,6 +84,8 @@ mwifiex_is_amsdu_in_ampdu_allowed(struct mwifiex_private *priv,
 {
 	struct mwifiex_tx_ba_stream_tbl *tx_tbl;
 
+	if (is_broadcast_ether_addr(ptr->ra))
+		return false;
 	tx_tbl = mwifiex_get_ba_tbl(priv, tid, ptr->ra);
 	if (tx_tbl)
 		return tx_tbl->amsdu;
@@ -96,6 +98,8 @@ static inline u8
 mwifiex_is_ampdu_allowed(struct mwifiex_private *priv,
 			 struct mwifiex_ra_list_tbl *ptr, int tid)
 {
+	if (is_broadcast_ether_addr(ptr->ra))
+		return false;
 	if (GET_BSS_ROLE(priv) == MWIFIEX_BSS_ROLE_UAP) {
 		return mwifiex_is_station_ampdu_allowed(priv, ptr, tid);
 	} else {
-- 
1.8.1.4

[Patch v2 5/5] mwifiex: fix warning while starting BSS

[Amitkumar Karwar]

From: Avinash Patil <patila@marvell.com>

We see this warning while starting mwifiex AP:
Unsupported RX-STBC, default to 2x2

This was happening because of wrong offset while copying HT
capabilities from BSS configuration of start_ap handler.

Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
Signed-off-by: Avinash Patil <patila@marvell.com>
---
 drivers/net/wireless/mwifiex/uap_cmd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/mwifiex/uap_cmd.c b/drivers/net/wireless/mwifiex/uap_cmd.c
index 300bab4..0f347fd 100644
--- a/drivers/net/wireless/mwifiex/uap_cmd.c
+++ b/drivers/net/wireless/mwifiex/uap_cmd.c
@@ -167,7 +167,7 @@ mwifiex_set_ht_params(struct mwifiex_private *priv,
 	ht_ie = cfg80211_find_ie(WLAN_EID_HT_CAPABILITY, params->beacon.tail,
 				 params->beacon.tail_len);
 	if (ht_ie) {
-		memcpy(&bss_cfg->ht_cap, ht_ie + 2,
+		memcpy(&bss_cfg->ht_cap, ht_ie,
 		       sizeof(struct ieee80211_ht_cap));
 		cap_info = le16_to_cpu(bss_cfg->ht_cap.cap_info);
 		memset(&bss_cfg->ht_cap.mcs, 0,
-- 
1.8.1.4