[PATCH] mac80211: make conf_tx non-atomic

[PATCH] mac80211: make conf_tx non-atomic

[Johannes Berg]

The conf_tx callback currently needs to be atomic, this requirement
is just because it can be called from scanning. This rearranges it
slightly to only update while not scanning (which is fine, we'll be
getting beacons when associated) and thus removes the atomic
requirement.

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
---
 include/net/mac80211.h |    2 +-
 net/mac80211/mlme.c    |   13 ++++++++-----
 2 files changed, 9 insertions(+), 6 deletions(-)

--- everything.orig/include/net/mac80211.h	2008-09-06 17:05:53.000000000 +0200
+++ everything/include/net/mac80211.h	2008-09-06 17:05:58.000000000 +0200
@@ -1142,7 +1142,7 @@ enum ieee80211_ampdu_mlme_action {
  *	of assocaited station or AP.
  *
  * @conf_tx: Configure TX queue parameters (EDCF (aifs, cw_min, cw_max),
- *	bursting) for a hardware TX queue. Must be atomic.
+ *	bursting) for a hardware TX queue.
  *
  * @get_tx_stats: Get statistics of the current TX queue status. This is used
  *	to get number of currently queued packets (queue length), maximum queue
--- everything.orig/net/mac80211/mlme.c	2008-09-06 17:06:00.000000000 +0200
+++ everything/net/mac80211/mlme.c	2008-09-06 17:07:13.000000000 +0200
@@ -2701,15 +2701,18 @@ static void ieee80211_rx_mgmt_beacon(str
 	    memcmp(ifsta->bssid, mgmt->bssid, ETH_ALEN) != 0)
 		return;
 
-	ieee80211_sta_wmm_params(local, ifsta, elems.wmm_param,
-				 elems.wmm_param_len);
-
 	/* Do not send changes to driver if we are scanning. This removes
-	 * requirement that driver's bss_info_changed function needs to be
-	 * atomic. */
+	 * requirement that a driver's bss_info_changed/conf_tx functions
+	 * need to be atomic.
+	 * This is really ugly code, we should rewrite scanning and make
+	 * all this more understandable for humans.
+	 */
 	if (local->sta_sw_scanning || local->sta_hw_scanning)
 		return;
 
+	ieee80211_sta_wmm_params(local, ifsta, elems.wmm_param,
+				 elems.wmm_param_len);
+
 	if (elems.erp_info && elems.erp_info_len >= 1)
 		changed |= ieee80211_handle_erp_ie(sdata, elems.erp_info[0]);
 	else {

Re: [PATCH] mac80211: make conf_tx non-atomic

[Ivo van Doorn]

On Saturday 06 September 2008, Johannes Berg wrote:
> The conf_tx callback currently needs to be atomic, this requirement
> is just because it can be called from scanning. This rearranges it
> slightly to only update while not scanning (which is fine, we'll be
> getting beacons when associated) and thus removes the atomic
> requirement.

Oops. Ehm how many oopses were reported since I submitted patch:
	47909b24a7e6c67ad635f36f5312839e04c7617c
	rt2x00: Initialize txop during conf_tx() callback

Because apparently that patch broke rt73usb (I completely overlooked
the conf_tx() atomic requirements). :S

Fortunately this patch would fix it again. :)

>  	/* Do not send changes to driver if we are scanning. This removes
> -	 * requirement that driver's bss_info_changed function needs to be
> -	 * atomic. */
> +	 * requirement that a driver's bss_info_changed/conf_tx functions
> +	 * need to be atomic.
> +	 * This is really ugly code, we should rewrite scanning and make
> +	 * all this more understandable for humans.
> +	 */

So bss_info_changed() is now non-atomic as well? Because that would be
very useful for rt2x00. :)

Ivo

Re: [PATCH] mac80211: make conf_tx non-atomic

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 1349 bytes --]

On Sat, 2008-09-06 at 19:50 +0200, Ivo van Doorn wrote:
> On Saturday 06 September 2008, Johannes Berg wrote:
> > The conf_tx callback currently needs to be atomic, this requirement
> > is just because it can be called from scanning. This rearranges it
> > slightly to only update while not scanning (which is fine, we'll be
> > getting beacons when associated) and thus removes the atomic
> > requirement.
> 
> Oops. Ehm how many oopses were reported since I submitted patch:
> 	47909b24a7e6c67ad635f36f5312839e04c7617c
> 	rt2x00: Initialize txop during conf_tx() callback
> 
> Because apparently that patch broke rt73usb (I completely overlooked
> the conf_tx() atomic requirements). :S

Heh, I don't know.

> Fortunately this patch would fix it again. :)

:)

> >  	/* Do not send changes to driver if we are scanning. This removes
> > -	 * requirement that driver's bss_info_changed function needs to be
> > -	 * atomic. */
> > +	 * requirement that a driver's bss_info_changed/conf_tx functions
> > +	 * need to be atomic.
> > +	 * This is really ugly code, we should rewrite scanning and make
> > +	 * all this more understandable for humans.
> > +	 */
> 
> So bss_info_changed() is now non-atomic as well? Because that would be
> very useful for rt2x00. :)

Yeah, it's been for a while, in fact.

johannes

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] mac80211: make conf_tx non-atomic

[Ivo van Doorn]

On Saturday 06 September 2008, Johannes Berg wrote:
> On Sat, 2008-09-06 at 19:50 +0200, Ivo van Doorn wrote:
> > On Saturday 06 September 2008, Johannes Berg wrote:
> > > The conf_tx callback currently needs to be atomic, this requirement
> > > is just because it can be called from scanning. This rearranges it
> > > slightly to only update while not scanning (which is fine, we'll be
> > > getting beacons when associated) and thus removes the atomic
> > > requirement.
> > 
> > Oops. Ehm how many oopses were reported since I submitted patch:
> > 	47909b24a7e6c67ad635f36f5312839e04c7617c
> > 	rt2x00: Initialize txop during conf_tx() callback
> > 
> > Because apparently that patch broke rt73usb (I completely overlooked
> > the conf_tx() atomic requirements). :S
> 
> Heh, I don't know.
> 
> > Fortunately this patch would fix it again. :)
> 
> :)
> 
> > >  	/* Do not send changes to driver if we are scanning. This removes
> > > -	 * requirement that driver's bss_info_changed function needs to be
> > > -	 * atomic. */
> > > +	 * requirement that a driver's bss_info_changed/conf_tx functions
> > > +	 * need to be atomic.
> > > +	 * This is really ugly code, we should rewrite scanning and make
> > > +	 * all this more understandable for humans.
> > > +	 */
> > 
> > So bss_info_changed() is now non-atomic as well? Because that would be
> > very useful for rt2x00. :)
> 
> Yeah, it's been for a while, in fact.

Ah ok. Well I'll create a oatch to cleanup rt2x00 then. :)

Thanks,

Ivo