[26/35] net: brcmfmac: Convey allocation mode as argument

Commit Message

Thomas Gleixner Sept. 27, 2020, 7:49 p.m. UTC

From: Sebastian Andrzej Siewior <[e-mail]>

The usage of in_interrupt() in drivers is phased out and Linus clearly
requested that code which changes behaviour depending on context should
either be seperated or the context be conveyed in an argument passed by the
caller, which usually knows the context.

brcmf_fweh_process_event() uses in_interrupt() to select the allocation
mode GFP_KERNEL/GFP_ATOMIC. Aside of the above reasons this check is
incomplete as it cannot detect contexts which just have preemption or
interrupts disabled.

All callchains leading to brcmf_fweh_process_event() can clearly identify
the calling context. Convey a 'gfp' argument through the callchains and let
the callers hand in the appropriate GFP mode.

This has also the advantage that any change of execution context or
preemption/interrupt state in these callchains will be detected by the
memory allocator for all GFP_KERNEL allocations.

Signed-off-by: Sebastian Andrzej Siewior <[e-mail]>
Signed-off-by: Thomas Gleixner <[e-mail]>
Cc: Arend van Spriel <[e-mail]>
Cc: Franky Lin <[e-mail]>
Cc: Hante Meuleman <[e-mail]>
Cc: Chi-Hsien Lin <[e-mail]>
Cc: Wright Feng <[e-mail]>
Cc: Kalle Valo <[e-mail]>
Cc: "David S. Miller" <[e-mail]>
Cc: Jakub Kicinski <[e-mail]>
Cc: [e-mail]
Cc: [e-mail]
Cc: [e-mail]
Cc: [e-mail]
---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h    |    5 +++--
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c   |   11 ++++++-----
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c   |    8 ++------
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.h   |    7 ++++---
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c |    2 +-
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c   |    8 ++++----
 drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c    |    2 +-
 7 files changed, 21 insertions(+), 22 deletions(-)

Comments

Arend van Spriel Sept. 28, 2020, 9:34 a.m. UTC | #1

On 9/27/2020 9:49 PM, Thomas Gleixner wrote:
> From: Sebastian Andrzej Siewior <[e-mail]>
> 
> The usage of in_interrupt() in drivers is phased out and Linus clearly
> requested that code which changes behaviour depending on context should
> either be seperated or the context be conveyed in an argument passed by the
> caller, which usually knows the context.
> 
> brcmf_fweh_process_event() uses in_interrupt() to select the allocation
> mode GFP_KERNEL/GFP_ATOMIC. Aside of the above reasons this check is
> incomplete as it cannot detect contexts which just have preemption or
> interrupts disabled.
> 
> All callchains leading to brcmf_fweh_process_event() can clearly identify
> the calling context. Convey a 'gfp' argument through the callchains and let
> the callers hand in the appropriate GFP mode.
> 
> This has also the advantage that any change of execution context or
> preemption/interrupt state in these callchains will be detected by the
> memory allocator for all GFP_KERNEL allocations.

Looking at the functions changed I see overlap with patch 25/35 so maybe 
change to gfp flag to a 'in_isr' bool. The flag can then be used for 
dealing with netif_rx as well.

Regards,
Arend

Patch

--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bus.h
@@ -249,9 +249,10 @@  int brcmf_bus_reset(struct brcmf_bus *bu
  */
 
 /* Receive frame for delivery to OS.  Callee disposes of rxp. */
-void brcmf_rx_frame(struct device *dev, struct sk_buff *rxp, bool handle_event);
+void brcmf_rx_frame(struct device *dev, struct sk_buff *rxp, bool handle_event,
+		    gfp_t gfp);
 /* Receive async event packet from firmware. Callee disposes of rxp. */
-void brcmf_rx_event(struct device *dev, struct sk_buff *rxp);
+void brcmf_rx_event(struct device *dev, struct sk_buff *rxp, gfp_t gfp);
 
 int brcmf_alloc(struct device *dev, struct brcmf_mp_device *settings);
 /* Indication from bus module regarding presence/insertion of dongle. */
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c
@@ -489,7 +489,8 @@  static int brcmf_rx_hdrpull(struct brcmf
 	return 0;
 }
 
-void brcmf_rx_frame(struct device *dev, struct sk_buff *skb, bool handle_event)
+void brcmf_rx_frame(struct device *dev, struct sk_buff *skb, bool handle_event,
+		    gfp_t gfp)
 {
 	struct brcmf_if *ifp;
 	struct brcmf_bus *bus_if = dev_get_drvdata(dev);
@@ -506,13 +507,13 @@  void brcmf_rx_frame(struct device *dev,
 		/* Process special event packets */
 		if (handle_event)
 			brcmf_fweh_process_skb(ifp->drvr, skb,
-					       BCMILCP_SUBTYPE_VENDOR_LONG);
-
+					       BCMILCP_SUBTYPE_VENDOR_LONG,
+					       gfp);
 		brcmf_netif_rx(ifp, skb);
 	}
 }
 
-void brcmf_rx_event(struct device *dev, struct sk_buff *skb)
+void brcmf_rx_event(struct device *dev, struct sk_buff *skb, gfp_t gfp)
 {
 	struct brcmf_if *ifp;
 	struct brcmf_bus *bus_if = dev_get_drvdata(dev);
@@ -523,7 +524,7 @@  void brcmf_rx_event(struct device *dev,
 	if (brcmf_rx_hdrpull(drvr, skb, &ifp))
 		return;
 
-	brcmf_fweh_process_skb(ifp->drvr, skb, 0);
+	brcmf_fweh_process_skb(ifp->drvr, skb, 0, gfp);
 	brcmu_pkt_buf_free_skb(skb);
 }
 
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.c
@@ -387,12 +387,11 @@  int brcmf_fweh_activate_events(struct br
  */
 void brcmf_fweh_process_event(struct brcmf_pub *drvr,
 			      struct brcmf_event *event_packet,
-			      u32 packet_len)
+			      u32 packet_len, gfp_t gfp)
 {
 	enum brcmf_fweh_event_code code;
 	struct brcmf_fweh_info *fweh = &drvr->fweh;
 	struct brcmf_fweh_queue_item *event;
-	gfp_t alloc_flag = GFP_KERNEL;
 	void *data;
 	u32 datalen;
 
@@ -411,10 +410,7 @@  void brcmf_fweh_process_event(struct brc
 	    datalen + sizeof(*event_packet) > packet_len)
 		return;
 
-	if (in_interrupt())
-		alloc_flag = GFP_ATOMIC;
-
-	event = kzalloc(sizeof(*event) + datalen, alloc_flag);
+	event = kzalloc(sizeof(*event) + datalen, gfp);
 	if (!event)
 		return;
 
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/fweh.h
@@ -319,11 +319,12 @@  void brcmf_fweh_unregister(struct brcmf_
 int brcmf_fweh_activate_events(struct brcmf_if *ifp);
 void brcmf_fweh_process_event(struct brcmf_pub *drvr,
 			      struct brcmf_event *event_packet,
-			      u32 packet_len);
+			      u32 packet_len, gfp_t gfp);
 void brcmf_fweh_p2pdev_setup(struct brcmf_if *ifp, bool ongoing);
 
 static inline void brcmf_fweh_process_skb(struct brcmf_pub *drvr,
-					  struct sk_buff *skb, u16 stype)
+					  struct sk_buff *skb, u16 stype,
+					  gfp_t gfp)
 {
 	struct brcmf_event *event_packet;
 	u16 subtype, usr_stype;
@@ -354,7 +355,7 @@  static inline void brcmf_fweh_process_sk
 	if (usr_stype != BCMILCP_BCM_SUBTYPE_EVENT)
 		return;
 
-	brcmf_fweh_process_event(drvr, event_packet, skb->len + ETH_HLEN);
+	brcmf_fweh_process_event(drvr, event_packet, skb->len + ETH_HLEN, gfp);
 }
 
 #endif /* FWEH_H_ */
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/msgbuf.c
@@ -1128,7 +1128,7 @@  static void brcmf_msgbuf_process_event(s
 
 	skb->protocol = eth_type_trans(skb, ifp->ndev);
 
-	brcmf_fweh_process_skb(ifp->drvr, skb, 0);
+	brcmf_fweh_process_skb(ifp->drvr, skb, 0, GFP_KERNEL);
 
 exit:
 	brcmu_pkt_buf_free_skb(skb);
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -1701,10 +1701,10 @@  static u8 brcmf_sdio_rxglom(struct brcmf
 					   pfirst->prev);
 			skb_unlink(pfirst, &bus->glom);
 			if (brcmf_sdio_fromevntchan(&dptr[SDPCM_HWHDR_LEN]))
-				brcmf_rx_event(bus->sdiodev->dev, pfirst);
+				brcmf_rx_event(bus->sdiodev->dev, pfirst, GFP_KERNEL);
 			else
 				brcmf_rx_frame(bus->sdiodev->dev, pfirst,
-					       false);
+					       false, GFP_KERNEL);
 			bus->sdcnt.rxglompkts++;
 		}
 
@@ -2035,10 +2035,10 @@  static uint brcmf_sdio_readframes(struct
 		if (pkt->len == 0)
 			brcmu_pkt_buf_free_skb(pkt);
 		else if (rd->channel == SDPCM_EVENT_CHANNEL)
-			brcmf_rx_event(bus->sdiodev->dev, pkt);
+			brcmf_rx_event(bus->sdiodev->dev, pkt, GFP_KERNEL);
 		else
 			brcmf_rx_frame(bus->sdiodev->dev, pkt,
-				       false);
+				       false, GFP_KERNEL);
 
 		/* prepare the descriptor for the next read */
 		rd->len = rd->len_nxtfrm << 4;
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/usb.c
@@ -532,7 +532,7 @@  static void brcmf_usb_rx_complete(struct
 	if (devinfo->bus_pub.state == BRCMFMAC_USB_STATE_UP ||
 	    devinfo->bus_pub.state == BRCMFMAC_USB_STATE_SLEEP) {
 		skb_put(skb, urb->actual_length);
-		brcmf_rx_frame(devinfo->dev, skb, true);
+		brcmf_rx_frame(devinfo->dev, skb, true, GFP_ATOMIC);
 		brcmf_usb_rx_refill(devinfo, req);
 		usb_mark_last_busy(urb->dev);
 	} else {