[RFC] ath11k: fix netdev open race

* [RFC] ath11k: fix netdev open race
@ 2022-05-17 10:34 ` Johan Hovold
  0 siblings, 0 replies; 12+ messages in thread
From: Johan Hovold @ 2022-05-17 10:34 UTC (permalink / raw)
  To: Kalle Valo
  Cc: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	ath11k, linux-wireless, netdev, linux-kernel, Johan Hovold

Make sure to allocate resources needed before registering the device.

This specifically avoids having a racing open() trigger a BUG_ON() in
mod_timer() when ath11k_mac_op_start() is called before the
mon_reap_timer as been set up.

Fixes: d5c65159f289 ("ath11k: driver for Qualcomm IEEE 802.11ax devices")
Fixes: 840c36fa727a ("ath11k: dp: stop rx pktlog before suspend")
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---

I started hitting a BUG_ON() during ath11k probe due to a timer which
hasn't been initialised. Turns out the netdev is registered before
having been fully set up:

[  421.232410] ath11k_core_pdev_create
[  421.233854] ath11k_dp_pdev_alloc
[  421.233863] ath11k_dp_rx_pdev_srng_alloc
[  421.259161] ath11k_mac_config_mon_status_default - NULL reap timer function
[  421.259165] ath11k_pci 0006:01:00.0: failed to configure monitor status ring with default rx_filter: (-22)
[  421.373066] ath11k_dp_rx_pdev_srng_alloc - reap timer setup

Sending as an RFC as I'm not familiar with the code. It looks like
ath11k_dp_pdev_alloc() may need to be split in an alloc and attach
function.

Johan


 drivers/net/wireless/ath/ath11k/core.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/net/wireless/ath/ath11k/core.c b/drivers/net/wireless/ath/ath11k/core.c
index ea073be60c12..e090dfbfa4e2 100644
--- a/drivers/net/wireless/ath/ath11k/core.c
+++ b/drivers/net/wireless/ath/ath11k/core.c
@@ -1218,23 +1218,23 @@ static int ath11k_core_pdev_create(struct ath11k_base *ab)
 		return ret;
 	}
 
-	ret = ath11k_mac_register(ab);
+	ret = ath11k_dp_pdev_alloc(ab);
 	if (ret) {
-		ath11k_err(ab, "failed register the radio with mac80211: %d\n", ret);
+		ath11k_err(ab, "failed to attach DP pdev: %d\n", ret);
 		goto err_pdev_debug;
 	}
 
-	ret = ath11k_dp_pdev_alloc(ab);
+	ret = ath11k_mac_register(ab);
 	if (ret) {
-		ath11k_err(ab, "failed to attach DP pdev: %d\n", ret);
-		goto err_mac_unregister;
+		ath11k_err(ab, "failed register the radio with mac80211: %d\n", ret);
+		goto err_dp_pdev_free;
 	}
 
 	ret = ath11k_thermal_register(ab);
 	if (ret) {
 		ath11k_err(ab, "could not register thermal device: %d\n",
 			   ret);
-		goto err_dp_pdev_free;
+		goto err_mac_unregister;
 	}
 
 	ret = ath11k_spectral_init(ab);
@@ -1247,10 +1247,10 @@ static int ath11k_core_pdev_create(struct ath11k_base *ab)
 
 err_thermal_unregister:
 	ath11k_thermal_unregister(ab);
-err_dp_pdev_free:
-	ath11k_dp_pdev_free(ab);
 err_mac_unregister:
 	ath11k_mac_unregister(ab);
+err_dp_pdev_free:
+	ath11k_dp_pdev_free(ab);
 err_pdev_debug:
 	ath11k_debugfs_pdev_destroy(ab);
 
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread