diff mbox series

[RFC,v1,1/7] mac80211: Add stations iterator where the iterator function may sleep

Message ID 20210717204057.67495-2-martin.blumenstingl@googlemail.com (mailing list archive)
State RFC
Delegated to: Netdev Maintainers
Headers show
Series rtw88: prepare locking for SDIO support | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count success Link
netdev/tree_selection success Guessed tree name to be net-next
netdev/subject_prefix success Link
netdev/cc_maintainers warning 2 maintainers not CCed: davem@davemloft.net kuba@kernel.org
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 119 this patch: 119
netdev/kdoc fail Errors and warnings before: 0 this patch: 1
netdev/verify_fixes success Link
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 43 lines checked
netdev/build_allmodconfig_warn success Errors and warnings before: 119 this patch: 119
netdev/header_inline success Link

Commit Message

Martin Blumenstingl July 17, 2021, 8:40 p.m. UTC
ieee80211_iterate_active_interfaces() and
ieee80211_iterate_active_interfaces_atomic() already exist, where the
former allows the iterator function to sleep. Add
ieee80211_iterate_stations() which is similar to
ieee80211_iterate_stations_atomic() but allows the iterator to sleep.
This is needed for adding SDIO support to the rtw88 driver. Some
interators there are reading or writing registers. With the SDIO ops
(sdio_readb, sdio_writeb and friends) this means that the iterator
function may sleep.

Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
---
 include/net/mac80211.h | 18 ++++++++++++++++++
 net/mac80211/util.c    | 13 +++++++++++++
 2 files changed, 31 insertions(+)

Comments

Ping-Ke Shih July 19, 2021, 5:46 a.m. UTC | #1
> -----Original Message-----
> From: Martin Blumenstingl [mailto:martin.blumenstingl@googlemail.com]
> Sent: Sunday, July 18, 2021 4:41 AM
> To: linux-wireless@vger.kernel.org
> Cc: tony0620emma@gmail.com; kvalo@codeaurora.org; johannes@sipsolutions.net; netdev@vger.kernel.org;
> linux-kernel@vger.kernel.org; Neo Jou; Jernej Skrabec; Martin Blumenstingl
> Subject: [PATCH RFC v1 1/7] mac80211: Add stations iterator where the iterator function may sleep
> 
> ieee80211_iterate_active_interfaces() and
> ieee80211_iterate_active_interfaces_atomic() already exist, where the
> former allows the iterator function to sleep. Add
> ieee80211_iterate_stations() which is similar to
> ieee80211_iterate_stations_atomic() but allows the iterator to sleep.
> This is needed for adding SDIO support to the rtw88 driver. Some
> interators there are reading or writing registers. With the SDIO ops
> (sdio_readb, sdio_writeb and friends) this means that the iterator
> function may sleep.
> 
> Signed-off-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> ---
>  include/net/mac80211.h | 18 ++++++++++++++++++
>  net/mac80211/util.c    | 13 +++++++++++++
>  2 files changed, 31 insertions(+)
> 
> diff --git a/include/net/mac80211.h b/include/net/mac80211.h
> index d8a1d09a2141..77de89690008 100644
> --- a/include/net/mac80211.h
> +++ b/include/net/mac80211.h
> @@ -5575,6 +5575,24 @@ void ieee80211_iterate_active_interfaces_mtx(struct ieee80211_hw *hw,
>  						struct ieee80211_vif *vif),
>  					     void *data);
> 
> +/**
> + * ieee80211_iterate_stations_atomic - iterate stations

ieee80211_iterate_stations - ...

[...]

--
Ping-Ke
Johannes Berg July 19, 2021, 6:30 a.m. UTC | #2
> 
> +/**
> + * ieee80211_iterate_stations_atomic - iterate stations

Copy/paste issue, as PK pointed out too.

> + *
> + * This function iterates over all stations associated with a given
> + * hardware that are currently uploaded to the driver and calls the callback
> + * function for them.
> + * This function allows the iterator function to sleep, when the iterator
> + * function is atomic @ieee80211_iterate_stations_atomic can be used.
> 

I have no real objections to this, but I think you should carefully
document something like "the driver must not call this with a lock held
that it can also take in response to callbacks from mac80211, and it
must not call this within callbacks made by mac80211" or something like
that, because both of those things are going to cause deadlocks.

johannes
diff mbox series

Patch

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index d8a1d09a2141..77de89690008 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -5575,6 +5575,24 @@  void ieee80211_iterate_active_interfaces_mtx(struct ieee80211_hw *hw,
 						struct ieee80211_vif *vif),
 					     void *data);
 
+/**
+ * ieee80211_iterate_stations_atomic - iterate stations
+ *
+ * This function iterates over all stations associated with a given
+ * hardware that are currently uploaded to the driver and calls the callback
+ * function for them.
+ * This function allows the iterator function to sleep, when the iterator
+ * function is atomic @ieee80211_iterate_stations_atomic can be used.
+ *
+ * @hw: the hardware struct of which the interfaces should be iterated over
+ * @iterator: the iterator function to call, cannot sleep
+ * @data: first argument of the iterator function
+ */
+void ieee80211_iterate_stations(struct ieee80211_hw *hw,
+				void (*iterator)(void *data,
+						 struct ieee80211_sta *sta),
+				void *data);
+
 /**
  * ieee80211_iterate_stations_atomic - iterate stations
  *
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 05e96212b104..c6984d0464f2 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -862,6 +862,19 @@  static void __iterate_stations(struct ieee80211_local *local,
 	}
 }
 
+void ieee80211_iterate_stations(struct ieee80211_hw *hw,
+				void (*iterator)(void *data,
+						 struct ieee80211_sta *sta),
+				void *data)
+{
+	struct ieee80211_local *local = hw_to_local(hw);
+
+	mutex_lock(&local->sta_mtx);
+	__iterate_stations(local, iterator, data);
+	mutex_unlock(&local->sta_mtx);
+}
+EXPORT_SYMBOL_GPL(ieee80211_iterate_stations);
+
 void ieee80211_iterate_stations_atomic(struct ieee80211_hw *hw,
 			void (*iterator)(void *data,
 					 struct ieee80211_sta *sta),