2021-07-17 20:42:16

by Martin Blumenstingl

[permalink] [raw]
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 <[email protected]>
---
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
+ *
+ * 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),
--
2.32.0


2021-07-19 05:47:45

by Ping-Ke Shih

[permalink] [raw]
Subject: RE: [PATCH RFC v1 1/7] mac80211: Add stations iterator where the iterator function may sleep



> -----Original Message-----
> From: Martin Blumenstingl [mailto:[email protected]]
> Sent: Sunday, July 18, 2021 4:41 AM
> To: [email protected]
> Cc: [email protected]; [email protected]; [email protected]; [email protected];
> [email protected]; 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 <[email protected]>
> ---
> 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

2021-07-19 06:31:03

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH RFC v1 1/7] mac80211: Add stations iterator where the iterator function may sleep

>
> +/**
> + * 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