| Message ID | 1433166856-3285-1-git-send-email-Wojciech.Dubowik@neratec.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Johannes Berg |
| Headers | show |
Hi Wojciech, On Mon, Jun 1, 2015 at 11:54 PM, Wojciech Dubowik <Wojciech.Dubowik@neratec.com> wrote: > We call rcu locked ieee80211_csa_update_counter from > already locked section. Fix it by decrementing counter > directly instead of calling ieee80211_csa_update_counter. Stupid question: wouldn't it be better to split the work from ieee80211_csa_update_counter() into a separate function without locking and call that instead? Thanks,
On 01/06/15 16:13, Julian Calaby wrote: > Hi Wojciech, > > On Mon, Jun 1, 2015 at 11:54 PM, Wojciech Dubowik > <Wojciech.Dubowik@neratec.com> wrote: >> We call rcu locked ieee80211_csa_update_counter from >> already locked section. Fix it by decrementing counter >> directly instead of calling ieee80211_csa_update_counter. > Stupid question: wouldn't it be better to split the work from > ieee80211_csa_update_counter() into a separate function without > locking and call that instead? Yes. It would be better for maintenance. It's just that they will have different input parameters ieee80211_csa_update_counter(struct ieee80211_vif * __ieee80211_csa_update_counter(struct beacon_data * as it doesn't make sense to dereference beacon twice. I guess it's not a problem? Wojtek > > Thanks, > -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 2015-06-01 at 16:23 +0200, Wojciech Dubowik wrote: > On 01/06/15 16:13, Julian Calaby wrote: > > Hi Wojciech, > > > > On Mon, Jun 1, 2015 at 11:54 PM, Wojciech Dubowik > > <Wojciech.Dubowik@neratec.com> wrote: > >> We call rcu locked ieee80211_csa_update_counter from > >> already locked section. Fix it by decrementing counter > >> directly instead of calling ieee80211_csa_update_counter. > > Stupid question: wouldn't it be better to split the work from > > ieee80211_csa_update_counter() into a separate function without > > locking and call that instead? > Yes. It would be better for maintenance. It's just that they will > have different input parameters > > ieee80211_csa_update_counter(struct ieee80211_vif * > __ieee80211_csa_update_counter(struct beacon_data * > > as it doesn't make sense to dereference beacon twice. > > I guess it's not a problem? Seems fine to me. johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c index 8df1342..9233559 100644 --- a/net/mac80211/tx.c +++ b/net/mac80211/tx.c @@ -3338,7 +3338,7 @@ __ieee80211_beacon_get(struct ieee80211_hw *hw, if (beacon) { if (beacon->csa_counter_offsets[0]) { if (!is_template) - ieee80211_csa_update_counter(vif); + WARN_ON_ONCE(beacon->csa_current_counter--); ieee80211_set_csa(sdata, beacon); } @@ -3384,7 +3384,7 @@ __ieee80211_beacon_get(struct ieee80211_hw *hw, if (beacon->csa_counter_offsets[0]) { if (!is_template) - ieee80211_csa_update_counter(vif); + WARN_ON_ONCE(beacon->csa_current_counter--); ieee80211_set_csa(sdata, beacon); } @@ -3414,7 +3414,7 @@ __ieee80211_beacon_get(struct ieee80211_hw *hw, * for now we leave it consistent with overall * mac80211's behavior. */ - ieee80211_csa_update_counter(vif); + WARN_ON_ONCE(beacon->csa_current_counter--); ieee80211_set_csa(sdata, beacon); }
We call rcu locked ieee80211_csa_update_counter from already locked section. Fix it by decrementing counter directly instead of calling ieee80211_csa_update_counter. Signed-off-by: Wojciech Dubowik <Wojciech.Dubowik@neratec.com> --- net/mac80211/tx.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)