Skip to contentSkip to conversation

wcn36xx: allocate skbs with GFP_KERNEL during init

PATCH In review Daniel Mack 2 participants 4 messages Mar 19, 2018 – Apr 10, 2018 Qualcomm WCN36XX wireless driver

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 wcn36xx: allocate skbs with GFP_KERNEL during init

GFP_ATOMIC should only be used when the allocation is done from atomic context. Introduce a new flag to wcn36xx_dxe_fill_skb() and use GFP_KERNEL when pre-allocating buffers during init.

This doesn't fix an issue that was observed in the wild, but it reduces the chance of failed allocations under memory pressure.

---
 drivers/net/wireless/ath/wcn36xx/dxe.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)
drivers/net/wireless/ath/wcn36xx/dxe.c +6 -4
index 5672154948c3..3e180828fbfa 100644
@@ -275,12 +275,14 @@ static int wcn36xx_dxe_enable_ch_int(struct wcn36xx *wcn, u16 wcn_ch)
 	return 0;
 }
 
-static int wcn36xx_dxe_fill_skb(struct device *dev, struct wcn36xx_dxe_ctl *ctl)
+static int wcn36xx_dxe_fill_skb(struct device *dev,
+				struct wcn36xx_dxe_ctl *ctl,
+				gfp_t gfp)
 {
 	struct wcn36xx_dxe_desc *dxe = ctl->desc;
 	struct sk_buff *skb;
 
-	skb = alloc_skb(WCN36XX_PKT_SIZE, GFP_ATOMIC);
+	skb = alloc_skb(WCN36XX_PKT_SIZE, gfp);
 	if (skb == NULL)
 		return -ENOMEM;
 
@@ -307,7 +309,7 @@ static int wcn36xx_dxe_ch_alloc_skb(struct wcn36xx *wcn,
 	cur_ctl = wcn_ch->head_blk_ctl;
 
 	for (i = 0; i < wcn_ch->desc_num; i++) {
-		wcn36xx_dxe_fill_skb(wcn->dev, cur_ctl);
+		wcn36xx_dxe_fill_skb(wcn->dev, cur_ctl, GFP_KERNEL);
 		cur_ctl = cur_ctl->next;
 	}
 
@@ -533,7 +535,7 @@ static int wcn36xx_rx_handle_packets(struct wcn36xx *wcn,
 	while (!(dxe->ctrl & WCN36XX_DXE_CTRL_VALID_MASK)) {
 		skb = ctl->skb;
 		dma_addr = dxe->dst_addr_l;
-		ret = wcn36xx_dxe_fill_skb(wcn->dev, ctl);
+		ret = wcn36xx_dxe_fill_skb(wcn->dev, ctl, GFP_ATOMIC);
 		if (0 == ret) {
 			/* new skb allocation ok. Use the new one and queue
 			 * the old one to network system.
Signature · 1 line
2.14.3
#1 lore raw
Quoted 8 lines · Daniel Mack

GFP_ATOMIC should only be used when the allocation is done from atomic context. Introduce a new flag to wcn36xx_dxe_fill_skb() and use GFP_KERNEL when pre-allocating buffers during init.

This doesn't fix an issue that was observed in the wild, but it reduces the chance of failed allocations under memory pressure.

Any opinion about this one?

Thanks,
Daniel

Quoted 44 lines
---
 drivers/net/wireless/ath/wcn36xx/dxe.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)
drivers/net/wireless/ath/wcn36xx/dxe.c +6 -4
index 5672154948c3..3e180828fbfa 100644
@@ -275,12 +275,14 @@ static int wcn36xx_dxe_enable_ch_int(struct wcn36xx *wcn, u16 wcn_ch)
 	return 0;
 }
 
-static int wcn36xx_dxe_fill_skb(struct device *dev, struct wcn36xx_dxe_ctl *ctl)
+static int wcn36xx_dxe_fill_skb(struct device *dev,
+				struct wcn36xx_dxe_ctl *ctl,
+				gfp_t gfp)
 {
 	struct wcn36xx_dxe_desc *dxe = ctl->desc;
 	struct sk_buff *skb;
 
-	skb = alloc_skb(WCN36XX_PKT_SIZE, GFP_ATOMIC);
+	skb = alloc_skb(WCN36XX_PKT_SIZE, gfp);
 	if (skb == NULL)
 		return -ENOMEM;
 
@@ -307,7 +309,7 @@ static int wcn36xx_dxe_ch_alloc_skb(struct wcn36xx *wcn,
 	cur_ctl = wcn_ch->head_blk_ctl;
 
 	for (i = 0; i < wcn_ch->desc_num; i++) {
-		wcn36xx_dxe_fill_skb(wcn->dev, cur_ctl);
+		wcn36xx_dxe_fill_skb(wcn->dev, cur_ctl, GFP_KERNEL);
 		cur_ctl = cur_ctl->next;
 	}
 
@@ -533,7 +535,7 @@ static int wcn36xx_rx_handle_packets(struct wcn36xx *wcn,
 	while (!(dxe->ctrl & WCN36XX_DXE_CTRL_VALID_MASK)) {
 		skb = ctl->skb;
 		dma_addr = dxe->dst_addr_l;
-		ret = wcn36xx_dxe_fill_skb(wcn->dev, ctl);
+		ret = wcn36xx_dxe_fill_skb(wcn->dev, ctl, GFP_ATOMIC);
 		if (0 == ret) {
 			/* new skb allocation ok. Use the new one and queue
 			 * the old one to network system.
#2 lore raw
Quoted 11 lines · Daniel Mack
Quoted 8 lines · Daniel Mack

GFP_ATOMIC should only be used when the allocation is done from atomic context. Introduce a new flag to wcn36xx_dxe_fill_skb() and use GFP_KERNEL when pre-allocating buffers during init.

This doesn't fix an issue that was observed in the wild, but it reduces the chance of failed allocations under memory pressure.

Any opinion about this one?

It's better that you check the state from patchwork, less mail to the maintainers:

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

Signature · 1 line
Kalle Valo
#3 lore raw
In reply to Daniel Mack
Quoted 9 lines · Daniel Mack

GFP_ATOMIC should only be used when the allocation is done from atomic context. Introduce a new flag to wcn36xx_dxe_fill_skb() and use GFP_KERNEL when pre-allocating buffers during init.

This doesn't fix an issue that was observed in the wild, but it reduces the chance of failed allocations under memory pressure.

Patch applied to ath-next branch of ath.git, thanks.

5151a673da43 wcn36xx: allocate skbs with GFP_KERNEL during init

Signature · 3 lines
https://patchwork.kernel.org/patch/10291685/

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