* [PATCH 1/4] net: mac80211: util.c: Fix RCU list usage warnings
@ 2020-04-09 8:28 madhuparnabhowmik10
2020-04-24 9:18 ` Johannes Berg
0 siblings, 1 reply; 2+ messages in thread
From: madhuparnabhowmik10 @ 2020-04-09 8:28 UTC (permalink / raw)
To: johannes, davem, kuba
Cc: linux-wireless, netdev, linux-kernel, frextrite, joel, paulmck,
linux-kernel-mentees, Madhuparna Bhowmik
From: Madhuparna Bhowmik <madhuparnabhowmik10@gmail.com>
This patch fixes the following warning (CONIG_PROVE_RCU_LIST)
in ieee80211_check_combinations().
WARNING: suspicious RCU usage
[ 80.933723] 5.6.0+ #4 Not tainted
[ 80.933733] -----------------------------
[ 80.933746] net/mac80211/util.c:3934 RCU-list traversed in non-reader section!!
Also, fix the other uses of list_for_each_entry_rcu() by either using
list_for_each_entry() instead (When mutex or spinlock is always held
in the function) or pass the necessary lockdep condition.
Signed-off-by: Madhuparna Bhowmik <madhuparnabhowmik10@gmail.com>
---
net/mac80211/util.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 20436c86b9bf..f4b0434024c0 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -254,7 +254,7 @@ static void __ieee80211_wake_txqs(struct ieee80211_sub_if_data *sdata, int ac)
sdata->vif.txqs_stopped[ac] = false;
- list_for_each_entry_rcu(sta, &local->sta_list, list) {
+ list_for_each_entry(sta, &local->sta_list, list) {
if (sdata != sta->sdata)
continue;
@@ -719,7 +719,8 @@ static void __iterate_interfaces(struct ieee80211_local *local,
struct ieee80211_sub_if_data *sdata;
bool active_only = iter_flags & IEEE80211_IFACE_ITER_ACTIVE;
- list_for_each_entry_rcu(sdata, &local->interfaces, list) {
+ list_for_each_entry_rcu(sdata, &local->interfaces, list,
+ (lockdep_is_held(&local->iflist_mtx)|| lockdep_rtnl_is_held())) {
switch (sdata->vif.type) {
case NL80211_IFTYPE_MONITOR:
if (!(sdata->u.mntr.flags & MONITOR_FLAG_ACTIVE))
@@ -3931,7 +3932,7 @@ int ieee80211_check_combinations(struct ieee80211_sub_if_data *sdata,
params.num_different_channels++;
}
- list_for_each_entry_rcu(sdata_iter, &local->interfaces, list) {
+ list_for_each_entry(sdata_iter, &local->interfaces, list) {
struct wireless_dev *wdev_iter;
wdev_iter = &sdata_iter->wdev;
@@ -3982,7 +3983,7 @@ int ieee80211_max_num_channels(struct ieee80211_local *local)
ieee80211_chanctx_radar_detect(local, ctx);
}
- list_for_each_entry_rcu(sdata, &local->interfaces, list)
+ list_for_each_entry(sdata, &local->interfaces, list)
params.iftype_num[sdata->wdev.iftype]++;
err = cfg80211_iter_combinations(local->hw.wiphy, ¶ms,
--
2.17.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH 1/4] net: mac80211: util.c: Fix RCU list usage warnings
2020-04-09 8:28 [PATCH 1/4] net: mac80211: util.c: Fix RCU list usage warnings madhuparnabhowmik10
@ 2020-04-24 9:18 ` Johannes Berg
0 siblings, 0 replies; 2+ messages in thread
From: Johannes Berg @ 2020-04-24 9:18 UTC (permalink / raw)
To: madhuparnabhowmik10, davem, kuba
Cc: linux-wireless, netdev, linux-kernel, frextrite, joel, paulmck,
linux-kernel-mentees
Hi,
> This patch fixes the following warning (CONIG_PROVE_RCU_LIST)
> in ieee80211_check_combinations().
Thanks, and sorry for the delay.
> +++ b/net/mac80211/util.c
> @@ -254,7 +254,7 @@ static void __ieee80211_wake_txqs(struct ieee80211_sub_if_data *sdata, int ac)
>
> sdata->vif.txqs_stopped[ac] = false;
>
> - list_for_each_entry_rcu(sta, &local->sta_list, list) {
> + list_for_each_entry(sta, &local->sta_list, list) {
> if (sdata != sta->sdata)
> continue;
In this case, for example, I don't even understand why the warning would
happen, because certainly the only caller of this (_ieee80211_wake_txqs)
does rcu_read_lock()?
I'm also not convinced that the necessary lock is actually held here,
this comes from a tasklet that doesn't hold any locks?
I'd appreciate if you could add comments/explain why you think the
changes were right, or ideally even add "lockdep_assert_held()"
annotations. That would make it much easier to check this patch.
> @@ -3931,7 +3932,7 @@ int ieee80211_check_combinations(struct ieee80211_sub_if_data *sdata,
> params.num_different_channels++;
> }
>
> - list_for_each_entry_rcu(sdata_iter, &local->interfaces, list) {
> + list_for_each_entry(sdata_iter, &local->interfaces, list) {
> struct wireless_dev *wdev_iter;
>
> wdev_iter = &sdata_iter->wdev;
> @@ -3982,7 +3983,7 @@ int ieee80211_max_num_channels(struct ieee80211_local *local)
> ieee80211_chanctx_radar_detect(local, ctx);
> }
>
> - list_for_each_entry_rcu(sdata, &local->interfaces, list)
> + list_for_each_entry(sdata, &local->interfaces, list)
> params.iftype_num[sdata->wdev.iftype]++;
These changes correct, as far as I can tell, in that they rely on the
RTNL now - but can you perhaps document that as well?
There doesn't seem to be any multi-lock version of lockdep_assert_held()
or is there? That'd be _really_ useful here, because I want to get rid
of some RTNL reliance in the longer term, and having annotation here
saying "either RTNL or iflist_mtx is fine" would be good.
johannes
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-04-24 9:19 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-09 8:28 [PATCH 1/4] net: mac80211: util.c: Fix RCU list usage warnings madhuparnabhowmik10
2020-04-24 9:18 ` Johannes Berg
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).