Linux wireless drivers development
 help / color / mirror / Atom feed
* Re: [PATCH v3 1/3] cfg80211: Add nl80211 antenna configuration
From: Bruno Randolf @ 2010-06-08  1:10 UTC (permalink / raw)
  To: Johannes Berg; +Cc: linville, linux-wireless
In-Reply-To: <1275901514.3639.4.camel@jlt3.sipsolutions.net>

On Monday 07 June 2010 18:05:14 you wrote:
> On Mon, 2010-06-07 at 15:58 +0900, Bruno Randolf wrote:
> > + * @NL80211_CMD_GET_ANTENNA: Get antenna configuration from driver.
> 
> Since we already have runtime configuration and capability information
> as part of the wiphy information printout, remind me again what was
> wrong with adding antenna information there as well rather than having a
> separate command to retrieve it?
>
> As far as differentiating that information is concerned, you can easily
> implement an iw "get antenna" command by just partially printing the
> wiphy information, for example, no?

i don't particularly care about where the antenna information is printed 
exactly, so anything will be fine with me.

but when i look at the wiphy info, it shows mostly capability information, 
like the list of supported frequencies, list of bitrates, supported interface 
modes, supported commands. the only exception is coverage class, which is the 
only runtime configuration as far as i can see. therefore i thought it doesnt 
fit there well.

maybe another command for getting runtime configuration like channel(s), fixed 
bitrates, essid, bssid, RTS/CTS, fraqmentation, txpower, power management, 
antenna settings, etc... would make sense? allthough in this list antenna is 
probably the only one which is per phy and not per interface...

i can leave the 'antenna get' command out until this is clarified.

bruno

^ permalink raw reply

* Re: [PATCH] mac80211: fix deauth before assoc
From: Miles Lane @ 2010-06-07 22:29 UTC (permalink / raw)
  To: Johannes Berg; +Cc: John Linville, linux-wireless
In-Reply-To: <1275940207.29978.24.camel@jlt3.sipsolutions.net>

Tested.  Looks good.

On Mon, Jun 7, 2010 at 3:50 PM, Johannes Berg <johannes@sipsolutions.net> wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> When we receive a deauthentication frame before
> having successfully associated, we neither print
> a message nor abort assocation. The former makes
> it hard to debug, while the latter later causes
> a warning in cfg80211 when, as will typically be
> the case, association timed out.
>
> This warning was reported by many, e.g. in
> https://bugzilla.kernel.org/show_bug.cgi?id=15981,
> but I couldn't initially pinpoint it. I verified
> the fix by hacking hostapd to send a deauth frame
> instead of an association response.
>
> Cc: stable@kernel.org
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> ---
>  net/mac80211/mlme.c |   39 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 38 insertions(+), 1 deletion(-)
>
> --- wireless-testing.orig/net/mac80211/mlme.c   2010-06-07 21:35:41.000000000 +0200
> +++ wireless-testing/net/mac80211/mlme.c        2010-06-07 21:46:00.000000000 +0200
> @@ -1695,8 +1695,45 @@ void ieee80211_sta_rx_queued_mgmt(struct
>        mutex_unlock(&ifmgd->mtx);
>
>        if (skb->len >= 24 + 2 /* mgmt + deauth reason */ &&
> -           (fc & IEEE80211_FCTL_STYPE) == IEEE80211_STYPE_DEAUTH)
> +           (fc & IEEE80211_FCTL_STYPE) == IEEE80211_STYPE_DEAUTH) {
> +               struct ieee80211_local *local = sdata->local;
> +               struct ieee80211_work *wk;
> +
> +               mutex_lock(&local->work_mtx);
> +               list_for_each_entry(wk, &local->work_list, list) {
> +                       if (wk->sdata != sdata)
> +                               continue;
> +
> +                       if (wk->type != IEEE80211_WORK_ASSOC)
> +                               continue;
> +
> +                       if (memcmp(mgmt->bssid, wk->filter_ta, ETH_ALEN))
> +                               continue;
> +                       if (memcmp(mgmt->sa, wk->filter_ta, ETH_ALEN))
> +                               continue;
> +
> +                       /*
> +                        * Printing the message only here means we can't
> +                        * spuriously print it, but it also means that it
> +                        * won't be printed when the frame comes in before
> +                        * we even tried to associate or in similar cases.
> +                        *
> +                        * Ultimately, I suspect cfg80211 should print the
> +                        * messages instead.
> +                        */
> +                       printk(KERN_DEBUG
> +                              "%s: deauthenticated from %pM (Reason: %u)\n",
> +                              sdata->name, mgmt->bssid,
> +                              le16_to_cpu(mgmt->u.deauth.reason_code));
> +
> +                       list_del_rcu(&wk->list);
> +                       free_work(wk);
> +                       break;
> +               }
> +               mutex_unlock(&local->work_mtx);
> +
>                cfg80211_send_deauth(sdata->dev, (u8 *)mgmt, skb->len);
> +       }
>  }
>
>  static void ieee80211_sta_timer(unsigned long data)
>
>
>

^ permalink raw reply

* Re: [PATCH] iwlwifi: add missing rcu_read_lock
From: Miles Lane @ 2010-06-07 22:28 UTC (permalink / raw)
  To: reinette chatre
  Cc: Johannes Berg, John Linville, Guy, Wey-Yi W, linux-wireless
In-Reply-To: <1275946462.1835.9324.camel@rchatre-DESK>

Tested.  Looks good.

On Mon, Jun 7, 2010 at 5:34 PM, reinette chatre
<reinette.chatre@intel.com> wrote:
> On Mon, 2010-06-07 at 12:20 -0700, Johannes Berg wrote:
>> From: Johannes Berg <johannes.berg@intel.com>
>>
>> Using ieee80211_find_sta() needs to be under
>> RCU read lock, which iwlwifi currently misses,
>> so fix it.
>>
>> Cc: stable@kernel.org
>> Reported-by: Miles Lane <miles.lane@gmail.com>
>> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
>
> Acked-by: Reinette Chatre <reinette.chatre@intel.com>
>
> Reinette
>
>
>

^ permalink raw reply

* Re: [PATCH] iwlwifi: add missing rcu_read_lock
From: reinette chatre @ 2010-06-07 21:34 UTC (permalink / raw)
  To: Johannes Berg; +Cc: John Linville, Guy, Wey-Yi W, Miles Lane, linux-wireless
In-Reply-To: <1275938438.29978.23.camel@jlt3.sipsolutions.net>

On Mon, 2010-06-07 at 12:20 -0700, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
> 
> Using ieee80211_find_sta() needs to be under
> RCU read lock, which iwlwifi currently misses,
> so fix it.
> 
> Cc: stable@kernel.org
> Reported-by: Miles Lane <miles.lane@gmail.com>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>

Acked-by: Reinette Chatre <reinette.chatre@intel.com>

Reinette



^ permalink raw reply

* Re: [PATCH] iwlwifi: add missing rcu_read_lock
From: Miles Lane @ 2010-06-07 21:31 UTC (permalink / raw)
  To: Johannes Berg
  Cc: John Linville, Reinette Chatre, Wey-Yi W Guy, linux-wireless
In-Reply-To: <1275938438.29978.23.camel@jlt3.sipsolutions.net>

On Mon, Jun 7, 2010 at 3:20 PM, Johannes Berg <johannes@sipsolutions.net> wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> Using ieee80211_find_sta() needs to be under
> RCU read lock, which iwlwifi currently misses,
> so fix it.
>
> Cc: stable@kernel.org
> Reported-by: Miles Lane <miles.lane@gmail.com>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> ---
>  drivers/net/wireless/iwlwifi/iwl-agn-tx.c |    2 ++
>  1 file changed, 2 insertions(+)
>
> --- wireless-testing.orig/drivers/net/wireless/iwlwifi/iwl-agn-tx.c     2010-06-07 21:17:56.000000000 +0200
> +++ wireless-testing/drivers/net/wireless/iwlwifi/iwl-agn-tx.c  2010-06-07 21:18:04.000000000 +0200
> @@ -1146,6 +1146,7 @@ static void iwlagn_tx_status(struct iwl_
>        struct ieee80211_sta *sta;
>        struct iwl_station_priv *sta_priv;
>
> +       rcu_read_lock();
>        sta = ieee80211_find_sta(priv->vif, hdr->addr1);
>        if (sta) {
>                sta_priv = (void *)sta->drv_priv;
> @@ -1154,6 +1155,7 @@ static void iwlagn_tx_status(struct iwl_
>                    atomic_dec_return(&sta_priv->pending_frames) == 0)
>                        ieee80211_sta_block_awake(priv->hw, sta, false);
>        }
> +       rcu_read_unlock();
>
>        ieee80211_tx_status_irqsafe(priv->hw, skb);
>  }
>
>
>

Unclean patch application - is this to be expected?

patching file drivers/net/wireless/iwlwifi/iwl-agn-tx.c
Hunk #1 succeeded at 1125 (offset -21 lines).
Hunk #2 succeeded at 1134 (offset -21 lines).

^ permalink raw reply

* Re: [PATCH] iwlwifi: fix-up botched revert
From: Randy Dunlap @ 2010-06-07 20:38 UTC (permalink / raw)
  To: John W. Linville
  Cc: linux-wireless, Stephen Rothwell, linux-next, linux-kernel
In-Reply-To: <1275936576-15590-1-git-send-email-linville@tuxdriver.com>

On 06/07/10 11:49, John W. Linville wrote:
> In the revert of "iwlwifi: move _agn statistics related structure", I
> need to use CONFIG_IWLWIFI_DEBUGFS instead of CONFIG_IWLWIFI_DEBUG in
> the private structure definition.  Without this patch, it is possible
> to get this:
> 
> drivers/net/wireless/iwlwifi/iwl-rx.c: In function 'iwl_accumulative_statistics':
> drivers/net/wireless/iwlwifi/iwl-rx.c:304: error: 'struct iwl_priv' has no member named 'accum_statistics'
> drivers/net/wireless/iwlwifi/iwl-rx.c:305: error: 'struct iwl_priv' has no member named 'delta_statistics'
> drivers/net/wireless/iwlwifi/iwl-rx.c:306: error: 'struct iwl_priv' has no member named 'max_delta'
> drivers/net/wireless/iwlwifi/iwl-rx.c:321: error: 'struct iwl_priv' has no member named 'accum_statistics'
> drivers/net/wireless/iwlwifi/iwl-rx.c:323: error: 'struct iwl_priv' has no member named 'accum_statistics'
> drivers/net/wireless/iwlwifi/iwl-rx.c:325: error: 'struct iwl_priv' has no member named 'accum_statistics'
> drivers/net/wireless/iwlwifi/iwl-rx.c:327: error: 'struct iwl_priv' has no member named 'accum_statistics'
> drivers/net/wireless/iwlwifi/iwl-rx.c:329: error: 'struct iwl_priv' has no member named 'accum_statistics'
> drivers/net/wireless/iwlwifi/iwl-rx.c:331: error: 'struct iwl_priv' has no member named 'accum_statistics'
> drivers/net/wireless/iwlwifi/iwl-rx.c: In function 'iwl_reply_statistics':
> drivers/net/wireless/iwlwifi/iwl-rx.c:484: error: 'struct iwl_priv' has no member named 'accum_statistics'
> drivers/net/wireless/iwlwifi/iwl-rx.c:486: error: 'struct iwl_priv' has no member named 'delta_statistics'
> drivers/net/wireless/iwlwifi/iwl-rx.c:488: error: 'struct iwl_priv' has no member named 'max_delta'
> 
> Reported-by: Randy Dunlap <randy.dunlap@oracle.com>
> Signed-off-by: John W. Linville <linville@tuxdriver.com>

Acked-by: Randy Dunlap <randy.dunlap@oracle.com>

Thanks.

> ---
>  drivers/net/wireless/iwlwifi/iwl-dev.h |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/net/wireless/iwlwifi/iwl-dev.h b/drivers/net/wireless/iwlwifi/iwl-dev.h
> index f359d9f..a00248c 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-dev.h
> +++ b/drivers/net/wireless/iwlwifi/iwl-dev.h
> @@ -1208,7 +1208,7 @@ struct iwl_priv {
>  	struct iwl_tt_mgmt thermal_throttle;
>  
>  	struct iwl_notif_statistics statistics;
> -#ifdef CONFIG_IWLWIFI_DEBUG
> +#ifdef CONFIG_IWLWIFI_DEBUGFS
>  	struct iwl_notif_statistics accum_statistics;
>  	struct iwl_notif_statistics delta_statistics;
>  	struct iwl_notif_statistics max_delta;


-- 
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

^ permalink raw reply

* Re: [PATCH] drivers/staging/otus/TODO: Don't bother, for documentation purposes only
From: Luis R. Rodriguez @ 2010-06-07 20:12 UTC (permalink / raw)
  To: Joe Perches
  Cc: Luis Rodriguez, Greg Kroah-Hartman, Andrew Morton,
	linux-kernel@vger.kernel.org, devel@driverdev.osuosl.org,
	linux-wireless
In-Reply-To: <1275937895.26425.41.camel@Joe-Laptop.home>

On Mon, Jun 07, 2010 at 12:11:35PM -0700, Joe Perches wrote:
> On Mon, 2010-06-07 at 12:02 -0700, Luis R. Rodriguez wrote:
> > this driver is unmaintained and its only purpose is as a
> > source of documentation for developers working on ar9170 and carl9170.
> > Once carl9170 gets 11n support and merged upstream then this driver
> > can be removed.
> 
> Then the TODO file should be updated.
> 
> Signed-off-by: Joe Perches <joe@perches.com>

Sure, thanks.

^ permalink raw reply

* [PATCH] mac80211: fix deauth before assoc
From: Johannes Berg @ 2010-06-07 19:50 UTC (permalink / raw)
  To: John Linville; +Cc: Miles Lane, linux-wireless

From: Johannes Berg <johannes.berg@intel.com>

When we receive a deauthentication frame before
having successfully associated, we neither print
a message nor abort assocation. The former makes
it hard to debug, while the latter later causes
a warning in cfg80211 when, as will typically be
the case, association timed out.

This warning was reported by many, e.g. in
https://bugzilla.kernel.org/show_bug.cgi?id=15981,
but I couldn't initially pinpoint it. I verified
the fix by hacking hostapd to send a deauth frame
instead of an association response.

Cc: stable@kernel.org
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/mlme.c |   39 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

--- wireless-testing.orig/net/mac80211/mlme.c	2010-06-07 21:35:41.000000000 +0200
+++ wireless-testing/net/mac80211/mlme.c	2010-06-07 21:46:00.000000000 +0200
@@ -1695,8 +1695,45 @@ void ieee80211_sta_rx_queued_mgmt(struct
 	mutex_unlock(&ifmgd->mtx);
 
 	if (skb->len >= 24 + 2 /* mgmt + deauth reason */ &&
-	    (fc & IEEE80211_FCTL_STYPE) == IEEE80211_STYPE_DEAUTH)
+	    (fc & IEEE80211_FCTL_STYPE) == IEEE80211_STYPE_DEAUTH) {
+		struct ieee80211_local *local = sdata->local;
+		struct ieee80211_work *wk;
+
+		mutex_lock(&local->work_mtx);
+		list_for_each_entry(wk, &local->work_list, list) {
+			if (wk->sdata != sdata)
+				continue;
+
+			if (wk->type != IEEE80211_WORK_ASSOC)
+				continue;
+
+			if (memcmp(mgmt->bssid, wk->filter_ta, ETH_ALEN))
+				continue;
+			if (memcmp(mgmt->sa, wk->filter_ta, ETH_ALEN))
+				continue;
+
+			/*
+			 * Printing the message only here means we can't
+			 * spuriously print it, but it also means that it
+			 * won't be printed when the frame comes in before
+			 * we even tried to associate or in similar cases.
+			 *
+			 * Ultimately, I suspect cfg80211 should print the
+			 * messages instead.
+			 */
+			printk(KERN_DEBUG
+			       "%s: deauthenticated from %pM (Reason: %u)\n",
+			       sdata->name, mgmt->bssid,
+			       le16_to_cpu(mgmt->u.deauth.reason_code));
+
+			list_del_rcu(&wk->list);
+			free_work(wk);
+			break;
+		}
+		mutex_unlock(&local->work_mtx);
+
 		cfg80211_send_deauth(sdata->dev, (u8 *)mgmt, skb->len);
+	}
 }
 
 static void ieee80211_sta_timer(unsigned long data)



^ permalink raw reply

* [PATCH] iwlwifi: add missing rcu_read_lock
From: Johannes Berg @ 2010-06-07 19:20 UTC (permalink / raw)
  To: John Linville; +Cc: Reinette Chatre, Wey-Yi W Guy, Miles Lane, linux-wireless

From: Johannes Berg <johannes.berg@intel.com>

Using ieee80211_find_sta() needs to be under
RCU read lock, which iwlwifi currently misses,
so fix it.

Cc: stable@kernel.org
Reported-by: Miles Lane <miles.lane@gmail.com>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 drivers/net/wireless/iwlwifi/iwl-agn-tx.c |    2 ++
 1 file changed, 2 insertions(+)

--- wireless-testing.orig/drivers/net/wireless/iwlwifi/iwl-agn-tx.c	2010-06-07 21:17:56.000000000 +0200
+++ wireless-testing/drivers/net/wireless/iwlwifi/iwl-agn-tx.c	2010-06-07 21:18:04.000000000 +0200
@@ -1146,6 +1146,7 @@ static void iwlagn_tx_status(struct iwl_
 	struct ieee80211_sta *sta;
 	struct iwl_station_priv *sta_priv;
 
+	rcu_read_lock();
 	sta = ieee80211_find_sta(priv->vif, hdr->addr1);
 	if (sta) {
 		sta_priv = (void *)sta->drv_priv;
@@ -1154,6 +1155,7 @@ static void iwlagn_tx_status(struct iwl_
 		    atomic_dec_return(&sta_priv->pending_frames) == 0)
 			ieee80211_sta_block_awake(priv->hw, sta, false);
 	}
+	rcu_read_unlock();
 
 	ieee80211_tx_status_irqsafe(priv->hw, skb);
 }



^ permalink raw reply

* [PATCH] drivers/staging/otus/TODO: Don't bother, for documentation purposes only
From: Joe Perches @ 2010-06-07 19:11 UTC (permalink / raw)
  To: Luis R. Rodriguez, Greg Kroah-Hartman
  Cc: Andrew Morton, linux-kernel@vger.kernel.org,
	devel@driverdev.osuosl.org, linux-wireless, Luis Rodriguez
In-Reply-To: <20100607190236.GD29505@tux>

On Mon, 2010-06-07 at 12:02 -0700, Luis R. Rodriguez wrote:
> this driver is unmaintained and its only purpose is as a
> source of documentation for developers working on ar9170 and carl9170.
> Once carl9170 gets 11n support and merged upstream then this driver
> can be removed.

Then the TODO file should be updated.

Signed-off-by: Joe Perches <joe@perches.com>
---
 drivers/staging/otus/TODO |   16 ++++------------
 1 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/otus/TODO b/drivers/staging/otus/TODO
index 4caf026..6fea974 100644
--- a/drivers/staging/otus/TODO
+++ b/drivers/staging/otus/TODO
@@ -2,15 +2,7 @@ I'm hesitant to add a TODO file here, as the wireless developers would
 really have people help them out on the "clean" ar9170 driver that can
 be found at the linux-wireless developer site.
 
-But, if you wish to clean up this driver instead, here's a short list of
-things that need to be done to get it into a more mergable shape:
-
-TODO:
-	- checkpatch.pl cleanups
-	- sparse cleanups
-	- port to in-kernel 80211 stack
-	- review by the wireless developer community
-
-Please send any patches to Greg Kroah-Hartman <greg@kroah.com> and
-Luis Rodriguez <Luis.Rodriguez@Atheros.com> and the
-otus-devel@lists.madwifi-project.org mailing list.
+This driver is unmaintained and its only purpose is as a
+source of documentation for developers working on ar9170 and carl9170.
+Once carl9170 gets 11n support and merged upstream then this driver
+can be removed.



^ permalink raw reply related

* Re: [PATCH 11/17] MAINTAINERS: Add STAGING - OTUS 802.11n USB WIRELESS DRIVER
From: Luis R. Rodriguez @ 2010-06-07 19:02 UTC (permalink / raw)
  To: Joe Perches
  Cc: Andrew Morton, linux-kernel@vger.kernel.org,
	devel@driverdev.osuosl.org, linux-wireless, Luis Rodriguez
In-Reply-To: <de848b7990a8a691068c8761543a321f4df3f759.1275935091.git.joe@perches.com>

On Mon, Jun 07, 2010 at 11:49:45AM -0700, Joe Perches wrote:
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
>  MAINTAINERS |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 06eed7b..4494736 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5507,6 +5507,11 @@ M:	Ossama Othman <ossama.othman@intel.com>
>  S:	Odd Fixes
>  F:	drivers/staging/memrar/
>  
> +STAGING - OTUS 802.11n USB WIRELESS DRIVER
> +M:	Luis Rodriguez <Luis.Rodriguez@Atheros.com>
> +S:	Odd Fixes
> +F:	drivers/staging/otus/
> +
>  STARFIRE/DURALAN NETWORK DRIVER
>  M:	Ion Badulescu <ionut@badula.org>
>  S:	Odd Fixes

Nack, this driver is unmaintained and its only purpose is as a
source of documentation for developers working on ar9170 and carl9170.
Once carl9170 gets 11n support and merged upstream then this driver
can be removed.

  Luis

^ permalink raw reply

* [PATCH] iwlwifi: fix-up botched revert
From: John W. Linville @ 2010-06-07 18:49 UTC (permalink / raw)
  To: linux-wireless
  Cc: Stephen Rothwell, linux-next, linux-kernel, Randy Dunlap,
	John W. Linville
In-Reply-To: <20100607090230.deed8bd1.randy.dunlap@oracle.com>

In the revert of "iwlwifi: move _agn statistics related structure", I
need to use CONFIG_IWLWIFI_DEBUGFS instead of CONFIG_IWLWIFI_DEBUG in
the private structure definition.  Without this patch, it is possible
to get this:

drivers/net/wireless/iwlwifi/iwl-rx.c: In function 'iwl_accumulative_statistics':
drivers/net/wireless/iwlwifi/iwl-rx.c:304: error: 'struct iwl_priv' has no member named 'accum_statistics'
drivers/net/wireless/iwlwifi/iwl-rx.c:305: error: 'struct iwl_priv' has no member named 'delta_statistics'
drivers/net/wireless/iwlwifi/iwl-rx.c:306: error: 'struct iwl_priv' has no member named 'max_delta'
drivers/net/wireless/iwlwifi/iwl-rx.c:321: error: 'struct iwl_priv' has no member named 'accum_statistics'
drivers/net/wireless/iwlwifi/iwl-rx.c:323: error: 'struct iwl_priv' has no member named 'accum_statistics'
drivers/net/wireless/iwlwifi/iwl-rx.c:325: error: 'struct iwl_priv' has no member named 'accum_statistics'
drivers/net/wireless/iwlwifi/iwl-rx.c:327: error: 'struct iwl_priv' has no member named 'accum_statistics'
drivers/net/wireless/iwlwifi/iwl-rx.c:329: error: 'struct iwl_priv' has no member named 'accum_statistics'
drivers/net/wireless/iwlwifi/iwl-rx.c:331: error: 'struct iwl_priv' has no member named 'accum_statistics'
drivers/net/wireless/iwlwifi/iwl-rx.c: In function 'iwl_reply_statistics':
drivers/net/wireless/iwlwifi/iwl-rx.c:484: error: 'struct iwl_priv' has no member named 'accum_statistics'
drivers/net/wireless/iwlwifi/iwl-rx.c:486: error: 'struct iwl_priv' has no member named 'delta_statistics'
drivers/net/wireless/iwlwifi/iwl-rx.c:488: error: 'struct iwl_priv' has no member named 'max_delta'

Reported-by: Randy Dunlap <randy.dunlap@oracle.com>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
 drivers/net/wireless/iwlwifi/iwl-dev.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-dev.h b/drivers/net/wireless/iwlwifi/iwl-dev.h
index f359d9f..a00248c 100644
--- a/drivers/net/wireless/iwlwifi/iwl-dev.h
+++ b/drivers/net/wireless/iwlwifi/iwl-dev.h
@@ -1208,7 +1208,7 @@ struct iwl_priv {
 	struct iwl_tt_mgmt thermal_throttle;
 
 	struct iwl_notif_statistics statistics;
-#ifdef CONFIG_IWLWIFI_DEBUG
+#ifdef CONFIG_IWLWIFI_DEBUGFS
 	struct iwl_notif_statistics accum_statistics;
 	struct iwl_notif_statistics delta_statistics;
 	struct iwl_notif_statistics max_delta;
-- 
1.6.6.1


^ permalink raw reply related

* Re: [PATCH] mac80211: fix function pointer check
From: Luis R. Rodriguez @ 2010-06-07 17:45 UTC (permalink / raw)
  To: Holger Schurig; +Cc: linux-wireless, John W. Linville
In-Reply-To: <201006071633.49820.holgerschurig@gmail.com>

On Mon, Jun 7, 2010 at 7:33 AM, Holger Schurig <holgerschurig@gmail.com> wrote:
> This makes "iw wlan0 dump survey" work again with
> mac80211-based drivers that support it, e.g. ath5k.
>
> Signed-off-by: Holger Schurig <holgerschurig@gmail.com>

Is this a stable fix?

  Luis

^ permalink raw reply

* Re: [PATCH] mac80211: Add netif state checking to ieee80211_ifa_changed
From: reinette chatre @ 2010-06-07 16:52 UTC (permalink / raw)
  To: Juuso Oikarinen; +Cc: linville@tuxdriver.com, linux-wireless@vger.kernel.org
In-Reply-To: <1275897132-10932-1-git-send-email-juuso.oikarinen@nokia.com>

Hi Juuso,

On Mon, 2010-06-07 at 00:52 -0700, Juuso Oikarinen wrote:
> There's a window for ieee80211_ifa_changed() to get called whilst the
> managed mode mutex has not been initialized when opening and stopping the
> interface. Currently this causes a kernel BUG like the following:
> 
> [  132.460013] kernel BUG at /home/wifi/iwlwifi-2.6/net/mac80211/main.c:380!
> [  132.460013] invalid opcode: 0000 [#1] SMP
> 
> The mutex is initialized during open(), hence once netif_running() is true,
> the mutex should be valid. Fix by adding a netif_running() check to the
> function.
> 
> Reported-by: Reinette Chatre <reinette.chatre@intel.com>
> Signed-off-by: Juuso Oikarinen <juuso.oikarinen@nokia.com>

Thank you very much. I've been running with this patch for more than ten
iterations of my testing and I have not encountered this issue again.

Tested-by: Reinette Chatre <reinette.chatre@intel.com>

Reinette



^ permalink raw reply

* Re: linux-next: Tree for June 7 (iwlwifi)
From: Randy Dunlap @ 2010-06-07 16:02 UTC (permalink / raw)
  To: Stephen Rothwell, linux-wireless; +Cc: linux-next, LKML
In-Reply-To: <20100607142034.c96c2509.sfr@canb.auug.org.au>

On Mon, 7 Jun 2010 14:20:34 +1000 Stephen Rothwell wrote:

> Hi all,
> 
> Changes since 20100604:


2 different builds:

drivers/net/wireless/iwlwifi/iwl-rx.c: In function 'iwl_accumulative_statistics':
drivers/net/wireless/iwlwifi/iwl-rx.c:304: error: 'struct iwl_priv' has no member named 'accum_statistics'
drivers/net/wireless/iwlwifi/iwl-rx.c:305: error: 'struct iwl_priv' has no member named 'delta_statistics'
drivers/net/wireless/iwlwifi/iwl-rx.c:306: error: 'struct iwl_priv' has no member named 'max_delta'
drivers/net/wireless/iwlwifi/iwl-rx.c:321: error: 'struct iwl_priv' has no member named 'accum_statistics'
drivers/net/wireless/iwlwifi/iwl-rx.c:323: error: 'struct iwl_priv' has no member named 'accum_statistics'
drivers/net/wireless/iwlwifi/iwl-rx.c:325: error: 'struct iwl_priv' has no member named 'accum_statistics'
drivers/net/wireless/iwlwifi/iwl-rx.c:327: error: 'struct iwl_priv' has no member named 'accum_statistics'
drivers/net/wireless/iwlwifi/iwl-rx.c:329: error: 'struct iwl_priv' has no member named 'accum_statistics'
drivers/net/wireless/iwlwifi/iwl-rx.c:331: error: 'struct iwl_priv' has no member named 'accum_statistics'
drivers/net/wireless/iwlwifi/iwl-rx.c: In function 'iwl_reply_statistics':
drivers/net/wireless/iwlwifi/iwl-rx.c:484: error: 'struct iwl_priv' has no member named 'accum_statistics'
drivers/net/wireless/iwlwifi/iwl-rx.c:486: error: 'struct iwl_priv' has no member named 'delta_statistics'
drivers/net/wireless/iwlwifi/iwl-rx.c:488: error: 'struct iwl_priv' has no member named 'max_delta'


--and--

drivers/net/wireless/iwlwifi/iwl-agn-debugfs.c: In function 'iwl_ucode_rx_stats_read':
drivers/net/wireless/iwlwifi/iwl-agn-debugfs.c:86: error: 'struct iwl_priv' has no member named 'accum_statistics'
drivers/net/wireless/iwlwifi/iwl-agn-debugfs.c:87: error: 'struct iwl_priv' has no member named 'accum_statistics'
drivers/net/wireless/iwlwifi/iwl-agn-debugfs.c:88: error: 'struct iwl_priv' has no member named 'accum_statistics'
drivers/net/wireless/iwlwifi/iwl-agn-debugfs.c:89: error: 'struct iwl_priv' has no member named 'accum_statistics'
drivers/net/wireless/iwlwifi/iwl-agn-debugfs.c:90: error: 'struct iwl_priv' has no member named 'delta_statistics'
drivers/net/wireless/iwlwifi/iwl-agn-debugfs.c:91: error: 'struct iwl_priv' has no member named 'delta_statistics'
drivers/net/wireless/iwlwifi/iwl-agn-debugfs.c:92: error: 'struct iwl_priv' has no member named 'delta_statistics'
drivers/net/wireless/iwlwifi/iwl-agn-debugfs.c:93: error: 'struct iwl_priv' has no member named 'delta_statistics'
drivers/net/wireless/iwlwifi/iwl-agn-debugfs.c:94: error: 'struct iwl_priv' has no member named 'max_delta'
drivers/net/wireless/iwlwifi/iwl-agn-debugfs.c:95: error: 'struct iwl_priv' has no member named 'max_delta'
drivers/net/wireless/iwlwifi/iwl-agn-debugfs.c:96: error: 'struct iwl_priv' has no member named 'max_delta'
drivers/net/wireless/iwlwifi/iwl-agn-debugfs.c:97: error: 'struct iwl_priv' has no member named 'max_delta'
drivers/net/wireless/iwlwifi/iwl-agn-debugfs.c: In function 'iwl_ucode_tx_stats_read':
drivers/net/wireless/iwlwifi/iwl-agn-debugfs.c:564: error: 'struct iwl_priv' has no member named 'accum_statistics'
drivers/net/wireless/iwlwifi/iwl-agn-debugfs.c:565: error: 'struct iwl_priv' has no member named 'delta_statistics'
drivers/net/wireless/iwlwifi/iwl-agn-debugfs.c:566: error: 'struct iwl_priv' has no member named 'max_delta'
drivers/net/wireless/iwlwifi/iwl-agn-debugfs.c: In function 'iwl_ucode_general_stats_read':
drivers/net/wireless/iwlwifi/iwl-agn-debugfs.c:783: error: 'struct iwl_priv' has no member named 'accum_statistics'
drivers/net/wireless/iwlwifi/iwl-agn-debugfs.c:784: error: 'struct iwl_priv' has no member named 'delta_statistics'
drivers/net/wireless/iwlwifi/iwl-agn-debugfs.c:785: error: 'struct iwl_priv' has no member named 'max_delta'
drivers/net/wireless/iwlwifi/iwl-agn-debugfs.c:786: error: 'struct iwl_priv' has no member named 'accum_statistics'
drivers/net/wireless/iwlwifi/iwl-agn-debugfs.c:787: error: 'struct iwl_priv' has no member named 'delta_statistics'
drivers/net/wireless/iwlwifi/iwl-agn-debugfs.c:788: error: 'struct iwl_priv' has no member named 'max_delta'
drivers/net/wireless/iwlwifi/iwl-agn-debugfs.c:789: error: 'struct iwl_priv' has no member named 'accum_statistics'
drivers/net/wireless/iwlwifi/iwl-agn-debugfs.c:790: error: 'struct iwl_priv' has no member named 'delta_statistics'
drivers/net/wireless/iwlwifi/iwl-agn-debugfs.c:791: error: 'struct iwl_priv' has no member named 'max_delta'


config files available if needed.

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

^ permalink raw reply

* Re: [RFC PATCH] mac80211: Fix circular locking dependency in ARP filter handling
From: Johannes Berg @ 2010-06-07 15:09 UTC (permalink / raw)
  To: Juuso Oikarinen; +Cc: linux-wireless@vger.kernel.org, reinette.chatre@intel.com
In-Reply-To: <1275918144.5277.30408.camel@wimaxnb.nmp.nokia.com>

On Mon, 2010-06-07 at 16:42 +0300, Juuso Oikarinen wrote:

> > > +#ifdef CONFIG_INET
> > > +		cancel_work_sync(&sdata->u.mgd.arp_config_work);
> > > +#endif
> > 
> > No can do, this is under RTNL and thus can't block waiting for a work
> > that acquires the RTNL ... the work might already be running, waiting
> > for the RTNL, by the time you get here. This will also get you a lockdep
> > complaint.
> 
> The work-func escapes based on ieee80211_sdata_running before acquiring
> the rtnl - hence here it should never get to the lock. I saw the same
> being used in those other work funcs too. I was beginning work to try to
> validate that, so it's not enough?

No, it's not enough. Like I said: "the work might already be running,
waiting for the RTNL" -- and once it obtains the RTNL, sdata has already
been freed!

> > This is why 
> > 
> > > @@ -379,7 +379,8 @@ static int ieee80211_ifa_changed(struct notifier_block *nb,
> > >  	ifmgd = &sdata->u.mgd;
> > >  	mutex_lock(&ifmgd->mtx);
> > >  	if (ifmgd->associated)
> > > -		ieee80211_set_arp_filter(sdata);
> > > +		ieee80211_queue_work(&sdata->local->hw,
> > > +				     &sdata->u.mgd.arp_config_work);
> > >  	mutex_unlock(&ifmgd->mtx);
> > 
> > No need to do change it here since the rtnl is held outside.
> 
> Yes, I know rtnl is held outside. I changed this for two reasons. First,
> I think it maybe better if the driver function is always called in the
> same scope. Secondly, this gets rid of inetdevs intermediate
> configurations (if you change IP address, it will first remove the
> previous one, and immediately after add a new one, resulting in two
> calls to the driver config function.)

Interesting. But that only works if it does that both together under
rtnl. I don't think the context really matters -- it should be the same
at least from a locking POV you have both rtnl and mgd->mtx acquired.

> > Also, and this applies to the change in mlme.c too, you must never put
> > work that acquires the rtnl onto the mac80211 workqueue ... that's what
> > you were trying to fix to start with!
> 
> > But because the interface might go away before your work runs, you're in
> > a stupid situation where you can't really use a per-interface work
> > either ... I think you probably need to have the work in ieee80211_local
> > and iterate the interface list.
> 
> I thought about iterating the interface list. I assume you imply calling
> the configure function for every interface.

Only if it's associated, I guess.

> Going still back to the current patch: assuming that you overlooked the
> sdata_running() call in the arp_config_work() function, and we can after
> all cancel_work_sync in _stop(), would using the kernel's default
> workqueue solve the rtnl problem, or are the rtnl dependencies there
> too?

Using the default wq would solve the rtnl from workqueue problem,
obviously, but wouldn't fix the cancel_work_sync problem.

johannes


^ permalink raw reply

* [PATCH] mac80211: fix function pointer check
From: Holger Schurig @ 2010-06-07 14:33 UTC (permalink / raw)
  To: linux-wireless, John W. Linville

This makes "iw wlan0 dump survey" work again with
mac80211-based drivers that support it, e.g. ath5k.

Signed-off-by: Holger Schurig <holgerschurig@gmail.com>

---
 net/mac80211/driver-ops.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- linux-wl.orig/net/mac80211/driver-ops.h
+++ linux-wl/net/mac80211/driver-ops.h
@@ -360,7 +360,7 @@ static inline int drv_get_survey(struct
 				struct survey_info *survey)
 {
 	int ret = -EOPNOTSUPP;
-	if (local->ops->conf_tx)
+	if (local->ops->get_survey)
 		ret = local->ops->get_survey(&local->hw, idx, survey);
 	/* trace_drv_get_survey(local, idx, survey, ret); */
 	return ret;

^ permalink raw reply

* Re: [RFC PATCH] mac80211: Fix circular locking dependency in ARP filter handling
From: Juuso Oikarinen @ 2010-06-07 13:42 UTC (permalink / raw)
  To: ext Johannes Berg
  Cc: linux-wireless@vger.kernel.org, reinette.chatre@intel.com
In-Reply-To: <1275916761.29978.15.camel@jlt3.sipsolutions.net>

On Mon, 2010-06-07 at 15:19 +0200, ext Johannes Berg wrote:
> On Mon, 2010-06-07 at 16:06 +0300, Juuso Oikarinen wrote:
> 
> > --- a/net/mac80211/iface.c
> > +++ b/net/mac80211/iface.c
> > @@ -477,6 +477,9 @@ static int ieee80211_stop(struct net_device *dev)
> >  		cancel_work_sync(&sdata->u.mgd.chswitch_work);
> >  		cancel_work_sync(&sdata->u.mgd.monitor_work);
> >  		cancel_work_sync(&sdata->u.mgd.beacon_connection_loss_work);
> > +#ifdef CONFIG_INET
> > +		cancel_work_sync(&sdata->u.mgd.arp_config_work);
> > +#endif
> 
> No can do, this is under RTNL and thus can't block waiting for a work
> that acquires the RTNL ... the work might already be running, waiting
> for the RTNL, by the time you get here. This will also get you a lockdep
> complaint.

The work-func escapes based on ieee80211_sdata_running before acquiring
the rtnl - hence here it should never get to the lock. I saw the same
being used in those other work funcs too. I was beginning work to try to
validate that, so it's not enough?

> This is why 
> 
> > @@ -379,7 +379,8 @@ static int ieee80211_ifa_changed(struct notifier_block *nb,
> >  	ifmgd = &sdata->u.mgd;
> >  	mutex_lock(&ifmgd->mtx);
> >  	if (ifmgd->associated)
> > -		ieee80211_set_arp_filter(sdata);
> > +		ieee80211_queue_work(&sdata->local->hw,
> > +				     &sdata->u.mgd.arp_config_work);
> >  	mutex_unlock(&ifmgd->mtx);
> 
> No need to do change it here since the rtnl is held outside.

Yes, I know rtnl is held outside. I changed this for two reasons. First,
I think it maybe better if the driver function is always called in the
same scope. Secondly, this gets rid of inetdevs intermediate
configurations (if you change IP address, it will first remove the
previous one, and immediately after add a new one, resulting in two
calls to the driver config function.)

> Also, and this applies to the change in mlme.c too, you must never put
> work that acquires the rtnl onto the mac80211 workqueue ... that's what
> you were trying to fix to start with!

> But because the interface might go away before your work runs, you're in
> a stupid situation where you can't really use a per-interface work
> either ... I think you probably need to have the work in ieee80211_local
> and iterate the interface list.

I thought about iterating the interface list. I assume you imply calling
the configure function for every interface.

Going still back to the current patch: assuming that you overlooked the
sdata_running() call in the arp_config_work() function, and we can after
all cancel_work_sync in _stop(), would using the kernel's default
workqueue solve the rtnl problem, or are the rtnl dependencies there
too?

-Juuso

> johannes
> 



^ permalink raw reply

* Re: kernel 2.6.35-rc1 on avila ixp4xx board booting problem
From: Pavel Roskin @ 2010-06-07 13:37 UTC (permalink / raw)
  To: Stefan; +Cc: linux-wireless
In-Reply-To: <4C0CCE51.7080403@googlemail.com>

On Mon, 2010-06-07 at 03:47 -0700, Stefan wrote:

> I just compiled the kernel from wireless-testing git tree for my
> gateworks avila ixp4xx  board. I used the OpenWrt toolchain for cross
> compiling for the arm - architecture and I used the .config of the
> original kernel of OpenWrt (2.6.32-10)
> 
> The kernel hangs up directly after being uncompressed.
...
> console output:

> Uncompressing Linux... done, booting the kernel.
> 
> I don't know, how to debug such behaviour. Any suggestions would be
> extremely helpful.
> I attached my kernel config.

Ypou are posting to a wrong mailing list.
linux-wireless@vger.kernel.org is about wireless drivers.  I see no
evidence that wireless drivers have anything to do with your problem.

Please look for mailing lists about embedded devices and ARM.

-- 
Regards,
Pavel Roskin

^ permalink raw reply

* [PATCH] mac80211: Fix bss_info_changed comment regarding sleeping
From: Ivo van Doorn @ 2010-06-07 13:20 UTC (permalink / raw)
  To: John W. Linville; +Cc: linux-wireless, Johannes Berg, rt2x00 Users List

The bss_info_changed callback function is allowed to sleep,
however when operating in Mesh mode, it can be used in
atomic context. Fixing it would seem to require major
locking changes in the mesh implementation.

Signed-off-by: Ivo van Doorn <IvDoorn@gmail.com>
---
 include/net/mac80211.h |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index de22cbf..73cf3ad 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -1533,7 +1533,8 @@ enum ieee80211_ampdu_mlme_action {
  *	This function should not be used if no BSS has been set, unless
  *	for association indication. The @changed parameter indicates which
  *	of the bss parameters has changed when a call is made. The callback
- *	can sleep.
+ *	can sleep except when operating in Mesh mode where the callback
+ *	must be atomic.
  *
  * @prepare_multicast: Prepare for multicast filter configuration.
  *	This callback is optional, and its return value is passed
-- 
1.6.6.1


^ permalink raw reply related

* Re: [RFC PATCH] mac80211: Fix circular locking dependency in ARP filter handling
From: Johannes Berg @ 2010-06-07 13:19 UTC (permalink / raw)
  To: Juuso Oikarinen; +Cc: linux-wireless, reinette.chatre
In-Reply-To: <1275915965-10124-1-git-send-email-juuso.oikarinen@nokia.com>

On Mon, 2010-06-07 at 16:06 +0300, Juuso Oikarinen wrote:

> --- a/net/mac80211/iface.c
> +++ b/net/mac80211/iface.c
> @@ -477,6 +477,9 @@ static int ieee80211_stop(struct net_device *dev)
>  		cancel_work_sync(&sdata->u.mgd.chswitch_work);
>  		cancel_work_sync(&sdata->u.mgd.monitor_work);
>  		cancel_work_sync(&sdata->u.mgd.beacon_connection_loss_work);
> +#ifdef CONFIG_INET
> +		cancel_work_sync(&sdata->u.mgd.arp_config_work);
> +#endif

No can do, this is under RTNL and thus can't block waiting for a work
that acquires the RTNL ... the work might already be running, waiting
for the RTNL, by the time you get here. This will also get you a lockdep
complaint.

This is why 

> @@ -379,7 +379,8 @@ static int ieee80211_ifa_changed(struct notifier_block *nb,
>  	ifmgd = &sdata->u.mgd;
>  	mutex_lock(&ifmgd->mtx);
>  	if (ifmgd->associated)
> -		ieee80211_set_arp_filter(sdata);
> +		ieee80211_queue_work(&sdata->local->hw,
> +				     &sdata->u.mgd.arp_config_work);
>  	mutex_unlock(&ifmgd->mtx);

No need to do change it here since the rtnl is held outside.

Also, and this applies to the change in mlme.c too, you must never put
work that acquires the rtnl onto the mac80211 workqueue ... that's what
you were trying to fix to start with!

But because the interface might go away before your work runs, you're in
a stupid situation where you can't really use a per-interface work
either ... I think you probably need to have the work in ieee80211_local
and iterate the interface list.

johannes


^ permalink raw reply

* Re: Locking in new ARP query filtering patch
From: Juuso Oikarinen @ 2010-06-07 13:18 UTC (permalink / raw)
  To: ext reinette chatre
  Cc: johannes@sipsolutions.net, linux-wireless@vger.kernel.org
In-Reply-To: <1275802426.1835.4408.camel@rchatre-DESK>

On Sun, 2010-06-06 at 07:33 +0200, ext reinette chatre wrote:
> Hi Juuso,
> 
> The new "mac80211: Add support for hardware ARP query filtering" is
> triggering some issues in my environment. I am not familiar with details
> of mac80211 locking and can thus not say if it is an issue with this
> patch itself or something in iwlwifi that is exposed by it. Could you
> please help take a look?

> When running with "mac80211: Add support for hardware ARP query
> filtering" I get the following lockdep warning when unloading iwl3945.
> The issue was bisected to this patch and when reverted this warning
> disappears. I also got a kernel BUG in what appears to be the locking
> code, which also did not appear since I reverted this patch.

Hi Reinette,

I'm also very green with mac80211 locking, but as you see I am currently
in the process of getting a crash course, and it hurts :P

I submitted a RFC PATCH attempting to fix the interlocking issue - as
per Johannes's analysis of the bug - and submitted another PATCH to fix
the BUG.

It's better to check Johannes's response to the patches first, but if he
doesn't say they're totally wrong, I'd appreciate if you could try them
out in your environment to see if your problems get solved.

And thanks for pointing these out.

-Juuso

> Here is the lockdep warning:
> [   92.026800] =======================================================
> [   92.030507] [ INFO: possible circular locking dependency detected ]
> [   92.030507] 2.6.34-04781-g2b2c009 #85
> [   92.030507] -------------------------------------------------------
> [   92.030507] modprobe/5225 is trying to acquire lock:
> [   92.030507]  ((wiphy_name(local->hw.wiphy))){+.+.+.}, at: [<ffffffff8105b5c0>] flush_workq
> ueue+0x0/0xb0
> [   92.030507]
> [   92.030507] but task is already holding lock:
> [   92.030507]  (rtnl_mutex){+.+.+.}, at: [<ffffffff812b9ce2>] rtnl_lock+0x12/0x20
> [   92.030507]
> [   92.030507] which lock already depends on the new lock.
> [   92.030507]
> [   92.030507]
> [   92.030507] the existing dependency chain (in reverse order) is:
> [   92.030507]
> [   92.030507] -> #2 (rtnl_mutex){+.+.+.}:
> [   92.030507]        [<ffffffff810761fb>] lock_acquire+0xdb/0x110
> [   92.030507]        [<ffffffff81341754>] mutex_lock_nested+0x44/0x300
> [   92.030507]        [<ffffffff812b9ce2>] rtnl_lock+0x12/0x20
> [   92.030507]        [<ffffffffa022d47c>] ieee80211_assoc_done+0x6c/0xe0 [mac80211]
> [   92.030507]        [<ffffffffa022f2ad>] ieee80211_work_work+0x31d/0x1280 [mac80211]
> [   92.030507]        [<ffffffff8105a520>] worker_thread+0x230/0x370
> [   92.030507]        [<ffffffff8105ecc6>] kthread+0x96/0xb0
> [   92.030507]        [<ffffffff81003a94>] kernel_thread_helper+0x4/0x10
> [   92.030507]
> [   92.030507] -> #1 ((&local->work_work)){+.+.+.}:
> [   92.030507]        [<ffffffff810761fb>] lock_acquire+0xdb/0x110
> [   92.030507]        [<ffffffff8105a51a>] worker_thread+0x22a/0x370
> [   92.030507]        [<ffffffff8105ecc6>] kthread+0x96/0xb0
> [   92.030507]        [<ffffffff81003a94>] kernel_thread_helper+0x4/0x10
> [   92.030507]
> [   92.030507] -> #0 ((wiphy_name(local->hw.wiphy))){+.+.+.}:
> [   92.030507]        [<ffffffff81075fdc>] __lock_acquire+0x1c0c/0x1d50
> [   92.030507]        [<ffffffff810761fb>] lock_acquire+0xdb/0x110
> [   92.030507]        [<ffffffff8105b60e>] flush_workqueue+0x4e/0xb0
> [   92.030507]        [<ffffffffa023ff7b>] ieee80211_stop_device+0x2b/0xb0 [mac80211]
> [   92.030507]        [<ffffffffa0231635>] ieee80211_stop+0x3e5/0x680 [mac80211]
> [   92.030507]        [<ffffffff812acdfc>] __dev_close+0x4c/0x80
> [   92.030507]        [<ffffffff812ace4a>] dev_close+0x1a/0x50
> [   92.030507]        [<ffffffff812acf4a>] rollback_registered_many+0xca/0x2e0
> [   92.030507]        [<ffffffff812ad176>] unregister_netdevice_many+0x16/0x70
> [   92.030507]        [<ffffffffa0230713>] ieee80211_remove_interfaces+0xb3/0xf0 [mac80211]
> [   92.030507]        [<ffffffffa02200ae>] ieee80211_unregister_hw+0x4e/0x110 [mac80211]
> [   92.030507]        [<ffffffffa0308e00>] iwl3945_pci_remove+0x90/0x79a [iwl3945]
> [   92.030507]        [<ffffffff811d5c02>] pci_device_remove+0x32/0x60
> [   92.030507]        [<ffffffff8124f11a>] __device_release_driver+0x7a/0xe0
> [   92.030507]        [<ffffffff8124f238>] driver_detach+0xb8/0xc0
> [   92.030507]        [<ffffffff8124e034>] bus_remove_driver+0x94/0x100
> [   92.030507]        [<ffffffff8124f858>] driver_unregister+0x58/0x90
> [   92.030507]        [<ffffffff811d5eb8>] pci_unregister_driver+0x48/0xc0
> [   92.030507]        [<ffffffffa030951a>] iwl3945_exit+0x10/0x17 [iwl3945]
> [   92.030507]        [<ffffffff810800f7>] sys_delete_module+0x167/0x250
> [   92.030507]        [<ffffffff81002c6b>] system_call_fastpath+0x16/0x1b
> [   92.030507]   
> [   92.030507] other info that might help us debug this:
> [   92.030507]   
> [   92.030507] 1 lock held by modprobe/5225:
> [   92.030507]  #0:  (rtnl_mutex){+.+.+.}, at: [<ffffffff812b9ce2>] rtnl_lock+0x12/0x20
> [   92.030507]   
> [   92.030507] stack backtrace:
> [   92.030507] Pid: 5225, comm: modprobe Not tainted 2.6.34-04781-g2b2c009 #85
> [   92.030507] Call Trace:
> [   92.030507]  [<ffffffff81073d41>] print_circular_bug+0x101/0x110
> [   92.030507]  [<ffffffff81075fdc>] __lock_acquire+0x1c0c/0x1d50
> [   92.030507]  [<ffffffff813432eb>] ? _raw_spin_unlock_irq+0x2b/0x40
> [   92.030507]  [<ffffffff81073215>] ? trace_hardirqs_on_caller+0x135/0x180
> [   92.030507]  [<ffffffff8105b14e>] ? __cancel_work_timer+0xfe/0x230
> [   92.030507]  [<ffffffff810761fb>] lock_acquire+0xdb/0x110
> [   92.030507]  [<ffffffff8105b5c0>] ? flush_workqueue+0x0/0xb0
> [   92.030507]  [<ffffffff8105b60e>] flush_workqueue+0x4e/0xb0
> [   92.030507]  [<ffffffff8105b5c0>] ? flush_workqueue+0x0/0xb0
> [   92.030507]  [<ffffffffa023ff7b>] ieee80211_stop_device+0x2b/0xb0 [mac80211]
> [   92.030507]  [<ffffffffa0231635>] ieee80211_stop+0x3e5/0x680 [mac80211]
> [   92.030507]  [<ffffffffa02312ae>] ? ieee80211_stop+0x5e/0x680 [mac80211]
> [   92.030507]  [<ffffffff812c1e55>] ? dev_deactivate+0x1c5/0x1f0
> [   92.030507]  [<ffffffff812acdfc>] __dev_close+0x4c/0x80
> [   92.030507]  [<ffffffff812ace4a>] dev_close+0x1a/0x50
> [   92.030507]  [<ffffffff812acf4a>] rollback_registered_many+0xca/0x2e0
> [   92.030507]  [<ffffffff812ad176>] unregister_netdevice_many+0x16/0x70
> [   92.030507]  [<ffffffffa0230713>] ieee80211_remove_interfaces+0xb3/0xf0 [mac80211]
> [   92.030507]  [<ffffffffa02200ae>] ieee80211_unregister_hw+0x4e/0x110 [mac80211]
> [   92.030507]  [<ffffffffa0308e00>] iwl3945_pci_remove+0x90/0x79a [iwl3945]
> [   92.030507]  [<ffffffff81063cfe>] ? up_read+0x1e/0x40
> [   92.030507]  [<ffffffff811d5c02>] pci_device_remove+0x32/0x60
> [   92.030507]  [<ffffffff8124f11a>] __device_release_driver+0x7a/0xe0
> [   92.030507]  [<ffffffff8124f238>] driver_detach+0xb8/0xc0
> [   92.030507]  [<ffffffff8124e034>] bus_remove_driver+0x94/0x100
> [   92.030507]  [<ffffffff8124f858>] driver_unregister+0x58/0x90
> [   92.030507]  [<ffffffff811d5eb8>] pci_unregister_driver+0x48/0xc0
> [   92.030507]  [<ffffffffa030951a>] iwl3945_exit+0x10/0x17 [iwl3945]
> [   92.030507]  [<ffffffff810800f7>] sys_delete_module+0x167/0x250
> [   92.030507]  [<ffffffff81343729>] ? retint_swapgs+0xe/0x13
> [   92.030507]  [<ffffffff81073215>] ? trace_hardirqs_on_caller+0x135/0x180
> [   92.030507]  [<ffffffff81342a02>] ? trace_hardirqs_on_thunk+0x3a/0x3f
> [   92.030507]  [<ffffffff81002c6b>] system_call_fastpath+0x16/0x1b
> 
> 
> Here is the BUG:
> 
> [  132.460013] kernel BUG at /home/wifi/iwlwifi-2.6/net/mac80211/main.c:380!
> [  132.460013] invalid opcode: 0000 [#1] SMP
> [  132.460013] last sysfs file: /sys/devices/virtual/misc/rfkill/dev
> [  132.460013] CPU 0
> [  132.460013] Modules linked in: iwl3945(+) iwlcore mac80211 cfg80211 rfkill binfmt_misc ipv
> 6 sbs sbshc psmouse serio_raw pcspkr evdev pegasus mii battery container video output ac proc
> essor intel_agp button ext3 jbd mbcache sg sd_mod sr_mod cdrom ahci libahci libata ehci_hcd s
> csi_mod uhci_hcd usbcore thermal fan thermal_sys
> [  132.460013]
> [  132.460013] Pid: 5186, comm: modprobe Not tainted 2.6.35-rc1-wl-63997-gca0217c #206 To be
> filled by O.E.M./Montevina platform
> [  132.460013] RIP: 0010:[<ffffffffa022fac2>]  [<ffffffffa022fac2>] ieee80211_alloc_hw+0x502/
> 0x520 [mac80211]
> [  132.460013] RSP: 0018:ffff88003a9c7c78  EFLAGS: 00010246
> [  132.460013] RAX: ffff880039c61960 RBX: ffff880039c602e0 RCX: 0000000000000001
> [  132.460013] RDX: ffffffffa02177a0 RSI: ffffffffa02177a8 RDI: ffffffffa0217a34
> [  132.460013] RBP: ffff88003a9c7c98 R08: 0000000000000008 R09: ffffffff815461f8
> [  132.460013] R10: 0000000000000001 R11: 0000000000000000 R12: ffff880039c605a0
> [  132.460013] R13: ffffffffa0318fc0 R14: ffff88003b616000 R15: ffff88003a8e8000
> [  132.460013] FS:  00007f24191876f0(0000) GS:ffff880002000000(0000) knlGS:0000000000000000
> [  132.460013] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> [  132.460013] CR2: 00007ffef6721080 CR3: 000000003aae4000 CR4: 00000000000006f0
> [  132.460013] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [  132.460013] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> [  132.460013] Process modprobe (pid: 5186, threadinfo ffff88003a9c6000, task ffff880039a24c9
> 0)
> [  132.460013] Stack:
> [  132.460013]  ffffffffa0319480 ffffffffa0319480 ffff88003b616000 ffff88003b616000
> [  132.460013] <0> ffff88003a9c7cb8 ffffffffa02af53e ffffffffa0319480 ffff88003b616088
> [  132.460013] <0> ffff88003a9c7d38 ffffffffa02fe1a7 0000000000000246 ffffffffa0318f48
> [  132.460013] Call Trace:
> [  132.460013]  [<ffffffffa02af53e>] iwl_alloc_all+0x1e/0x50 [iwlcore]
> [  132.460013]  [<ffffffffa02fe1a7>] iwl3945_pci_probe+0x27/0x1640 [iwl3945]
> [  132.460013]  [<ffffffff813446b6>] ? _raw_spin_unlock+0x26/0x30
> [  132.460013]  [<ffffffff811d48c0>] ? pci_match_device+0x20/0xd0
> [  132.460013]  [<ffffffff811d4772>] local_pci_probe+0x12/0x20
> [  132.460013]  [<ffffffff811d5880>] pci_device_probe+0x80/0xa0
> [  132.460013]  [<ffffffff8124fa5b>] ? driver_sysfs_add+0x6b/0x90
> [  132.460013]  [<ffffffff8124fb95>] driver_probe_device+0x95/0x1d0
> [  132.460013]  [<ffffffff8124fd6b>] __driver_attach+0x9b/0xb0
> [  132.460013]  [<ffffffff8124fcd0>] ? __driver_attach+0x0/0xb0
> [  132.460013]  [<ffffffff8124f23b>] bus_for_each_dev+0x6b/0xa0
> [  132.460013]  [<ffffffff8124f9ec>] driver_attach+0x1c/0x20
> [  132.460013]  [<ffffffff8124e995>] bus_add_driver+0x1b5/0x300
> [  132.460013]  [<ffffffffa0145000>] ? iwl3945_init+0x0/0x80 [iwl3945]
> [  132.460013]  [<ffffffff8125007c>] driver_register+0x7c/0x170
> [  132.460013]  [<ffffffffa0145000>] ? iwl3945_init+0x0/0x80 [iwl3945]
> [  132.460013]  [<ffffffff811d5b3a>] __pci_register_driver+0x6a/0xf0
> [  132.460013]  [<ffffffffa0145000>] ? iwl3945_init+0x0/0x80 [iwl3945]
> [  132.460013]  [<ffffffffa014504a>] iwl3945_init+0x4a/0x80 [iwl3945]
> [  132.460013]  [<ffffffff810001d8>] do_one_initcall+0x38/0x190
> [  132.460013]  [<ffffffff810800d4>] sys_init_module+0xe4/0x260
> [  132.460013]  [<ffffffff81002c6b>] system_call_fastpath+0x16/0x1b
> [  132.460013] Code: c7 c6 9d 12 27 a0 49 89 84 24 a0 03 00 00 49 89 84 24 a8 03 00 00 e8 6e
> 60 f9 e0 4c 89 e0 5b 41 5c 41 5d 41 5e c9 c3 0f 0b eb fe <0f> 0b eb fe 0f 0b eb fe 0f 0b eb f
> e 0f 0b eb fe 0f 0b eb fe 0f
> [  132.460013] RIP  [<ffffffffa022fac2>] ieee80211_alloc_hw+0x502/0x520 [mac80211]
> [  132.460013]  RSP <ffff88003a9c7c78>
> [  132.822653] ---[ end trace 4026d119e16fef57 ]---
> 
> The code in which the above appears is:
> (gdb) l *(ieee80211_alloc_hw+0x502)
> 0xaf2 is in ieee80211_alloc_hw (/home/wifi/iwlwifi-2.6/net/mac80211/main.c:380).
> 375	
> 376		ifmgd = &sdata->u.mgd;
> 377		mutex_lock(&ifmgd->mtx);
> 378		if (ifmgd->associated)
> 379			ieee80211_set_arp_filter(sdata);
> 380		mutex_unlock(&ifmgd->mtx);
> 381	
> 382		return NOTIFY_DONE;
> 383	}
> 384	#endif
> (gdb) q
> 
> Thank you
> 
> Reinette
> 
> 



^ permalink raw reply

* [RFC PATCH] mac80211: Fix circular locking dependency in ARP filter handling
From: Juuso Oikarinen @ 2010-06-07 13:06 UTC (permalink / raw)
  To: linux-wireless; +Cc: reinette.chatre

There is a circular locking dependency when configuring the
hardware ARP filters on association, occurring when flushing the mac80211
workqueue. This is what happens:

[   92.026800] =======================================================
[   92.030507] [ INFO: possible circular locking dependency detected ]
[   92.030507] 2.6.34-04781-g2b2c009 #85
[   92.030507] -------------------------------------------------------
[   92.030507] modprobe/5225 is trying to acquire lock:
[   92.030507]  ((wiphy_name(local->hw.wiphy))){+.+.+.}, at: [<ffffffff8105b5c0>] flush_workq
ueue+0x0/0xb0
[   92.030507]
[   92.030507] but task is already holding lock:
[   92.030507]  (rtnl_mutex){+.+.+.}, at: [<ffffffff812b9ce2>] rtnl_lock+0x12/0x20
[   92.030507]
[   92.030507] which lock already depends on the new lock.
[   92.030507]
[   92.030507]
[   92.030507] the existing dependency chain (in reverse order) is:
[   92.030507]
[   92.030507] -> #2 (rtnl_mutex){+.+.+.}:
[   92.030507]        [<ffffffff810761fb>] lock_acquire+0xdb/0x110
[   92.030507]        [<ffffffff81341754>] mutex_lock_nested+0x44/0x300
[   92.030507]        [<ffffffff812b9ce2>] rtnl_lock+0x12/0x20
[   92.030507]        [<ffffffffa022d47c>] ieee80211_assoc_done+0x6c/0xe0 [mac80211]
[   92.030507]        [<ffffffffa022f2ad>] ieee80211_work_work+0x31d/0x1280 [mac80211]

[   92.030507] -> #1 ((&local->work_work)){+.+.+.}:
[   92.030507]        [<ffffffff810761fb>] lock_acquire+0xdb/0x110
[   92.030507]        [<ffffffff8105a51a>] worker_thread+0x22a/0x370
[   92.030507]        [<ffffffff8105ecc6>] kthread+0x96/0xb0
[   92.030507]        [<ffffffff81003a94>] kernel_thread_helper+0x4/0x10
[   92.030507]
[   92.030507] -> #0 ((wiphy_name(local->hw.wiphy))){+.+.+.}:
[   92.030507]        [<ffffffff81075fdc>] __lock_acquire+0x1c0c/0x1d50
[   92.030507]        [<ffffffff810761fb>] lock_acquire+0xdb/0x110
[   92.030507]        [<ffffffff8105b60e>] flush_workqueue+0x4e/0xb0
[   92.030507]        [<ffffffffa023ff7b>] ieee80211_stop_device+0x2b/0xb0 [mac80211]
[   92.030507]        [<ffffffffa0231635>] ieee80211_stop+0x3e5/0x680 [mac80211]

Fix the problem by moving the hardware ARP filter configuration to a
workqueue function.

Reported-by: Reinette Chatre <reinette.chatre@intel.com>
Signed-off-by: Juuso Oikarinen <juuso.oikarinen@nokia.com>
---
 net/mac80211/ieee80211_i.h |    3 +++
 net/mac80211/iface.c       |    3 +++
 net/mac80211/main.c        |    3 ++-
 net/mac80211/mlme.c        |   41 +++++++++++++++++++++++++++++------------
 4 files changed, 37 insertions(+), 13 deletions(-)

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 4d3883e..893a5cc 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -329,6 +329,9 @@ struct ieee80211_if_managed {
 	struct work_struct monitor_work;
 	struct work_struct chswitch_work;
 	struct work_struct beacon_connection_loss_work;
+#ifdef CONFIG_INET
+	struct work_struct arp_config_work;
+#endif
 
 	unsigned long probe_timeout;
 	int probe_send_count;
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 1afa9ec..44bf800 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -477,6 +477,9 @@ static int ieee80211_stop(struct net_device *dev)
 		cancel_work_sync(&sdata->u.mgd.chswitch_work);
 		cancel_work_sync(&sdata->u.mgd.monitor_work);
 		cancel_work_sync(&sdata->u.mgd.beacon_connection_loss_work);
+#ifdef CONFIG_INET
+		cancel_work_sync(&sdata->u.mgd.arp_config_work);
+#endif
 
 		/*
 		 * When we get here, the interface is marked down.
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index 88b671a..5583980 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -379,7 +379,8 @@ static int ieee80211_ifa_changed(struct notifier_block *nb,
 	ifmgd = &sdata->u.mgd;
 	mutex_lock(&ifmgd->mtx);
 	if (ifmgd->associated)
-		ieee80211_set_arp_filter(sdata);
+		ieee80211_queue_work(&sdata->local->hw,
+				     &sdata->u.mgd.arp_config_work);
 	mutex_unlock(&ifmgd->mtx);
 
 	return NOTIFY_DONE;
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index ac68c41..c3bf0a2 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -853,6 +853,11 @@ static void ieee80211_set_associated(struct ieee80211_sub_if_data *sdata,
 
 	ieee80211_bss_info_change_notify(sdata, bss_info_changed);
 
+#ifdef CONFIG_INET
+	/* Configure hardware ARP filter with current IPv4 config */
+	ieee80211_queue_work(&sdata->local->hw, &sdata->u.mgd.arp_config_work);
+#endif
+
 	mutex_lock(&local->iflist_mtx);
 	ieee80211_recalc_ps(local, -1);
 	ieee80211_recalc_smps(local, sdata);
@@ -1767,6 +1772,26 @@ static void ieee80211_sta_rx_queued_mgmt(struct ieee80211_sub_if_data *sdata,
 	kfree_skb(skb);
 }
 
+#ifdef CONFIG_INET
+void ieee80211_arp_config_work(struct work_struct *work)
+{
+	struct ieee80211_sub_if_data *sdata =
+		container_of(work, struct ieee80211_sub_if_data,
+			     u.mgd.arp_config_work);
+	struct ieee80211_if_managed *ifmgd = &sdata->u.mgd;
+
+	if (!ieee80211_sdata_running(sdata))
+		return;
+
+	rtnl_lock();
+	mutex_lock(&ifmgd->mtx);
+	if (ifmgd->associated)
+		ieee80211_set_arp_filter(sdata);
+	mutex_unlock(&ifmgd->mtx);
+	rtnl_unlock();
+}
+#endif
+
 static void ieee80211_sta_timer(unsigned long data)
 {
 	struct ieee80211_sub_if_data *sdata =
@@ -1959,6 +1984,9 @@ void ieee80211_sta_setup_sdata(struct ieee80211_sub_if_data *sdata)
 	INIT_WORK(&ifmgd->chswitch_work, ieee80211_chswitch_work);
 	INIT_WORK(&ifmgd->beacon_connection_loss_work,
 		  ieee80211_beacon_connection_loss_work);
+#ifdef CONFIG_INET
+	INIT_WORK(&ifmgd->arp_config_work, ieee80211_arp_config_work);
+#endif
 	setup_timer(&ifmgd->timer, ieee80211_sta_timer,
 		    (unsigned long) sdata);
 	setup_timer(&ifmgd->bcn_mon_timer, ieee80211_sta_bcn_mon_timer,
@@ -2116,19 +2144,8 @@ static enum work_done_result ieee80211_assoc_done(struct ieee80211_work *wk,
 			cfg80211_send_assoc_timeout(wk->sdata->dev,
 						    wk->filter_ta);
 			return WORK_DONE_DESTROY;
-#ifdef CONFIG_INET
-		} else {
-			mutex_unlock(&wk->sdata->u.mgd.mtx);
-
-			/*
-			 * configure ARP filter IP addresses to the driver,
-			 * intentionally outside the mgd mutex.
-			 */
-			rtnl_lock();
-			ieee80211_set_arp_filter(wk->sdata);
-			rtnl_unlock();
-#endif
 		}
+		mutex_unlock(&wk->sdata->u.mgd.mtx);
 	}
 
 	cfg80211_send_rx_assoc(wk->sdata->dev, skb->data, skb->len);
-- 
1.6.3.3


^ permalink raw reply related

* [RFC 21/21] mac80211: allow drivers to sleep in ampdu_action
From: Johannes Berg @ 2010-06-07 11:01 UTC (permalink / raw)
  To: linux-wireless
In-Reply-To: <20100607110133.472649120@sipsolutions.net>

From: Johannes Berg <johannes.berg@intel.com>

Allow drivers to sleep, and indicate this in
the documentation. ath9k has some locking I
don't understand, so keep it safe and disable
BHs in it, all other drivers look fine with
the context change.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 drivers/net/wireless/ath/ath9k/main.c |    4 ++++
 include/net/mac80211.h                |    2 +-
 net/mac80211/driver-ops.h             |    3 +--
 3 files changed, 6 insertions(+), 3 deletions(-)

--- wireless-testing.orig/include/net/mac80211.h	2010-06-07 12:42:24.000000000 +0200
+++ wireless-testing/include/net/mac80211.h	2010-06-07 12:42:35.000000000 +0200
@@ -1640,7 +1640,7 @@ enum ieee80211_ampdu_mlme_action {
  * 	is the first frame we expect to perform the action on. Notice
  * 	that TX/RX_STOP can pass NULL for this parameter.
  *	Returns a negative error code on failure.
- *	The callback must be atomic.
+ *	The callback can sleep.
  *
  * @get_survey: Return per-channel survey information
  *
--- wireless-testing.orig/drivers/net/wireless/ath/ath9k/main.c	2010-06-07 12:43:05.000000000 +0200
+++ wireless-testing/drivers/net/wireless/ath/ath9k/main.c	2010-06-07 12:43:55.000000000 +0200
@@ -1769,6 +1769,8 @@ static int ath9k_ampdu_action(struct iee
 	struct ath_softc *sc = aphy->sc;
 	int ret = 0;
 
+	local_bh_disable();
+
 	switch (action) {
 	case IEEE80211_AMPDU_RX_START:
 		if (!(sc->sc_flags & SC_OP_RXAGGR))
@@ -1798,6 +1800,8 @@ static int ath9k_ampdu_action(struct iee
 			  "Unknown AMPDU action\n");
 	}
 
+	local_bh_enable();
+
 	return ret;
 }
 
--- wireless-testing.orig/net/mac80211/driver-ops.h	2010-06-07 12:42:42.000000000 +0200
+++ wireless-testing/net/mac80211/driver-ops.h	2010-06-07 12:42:46.000000000 +0200
@@ -352,11 +352,10 @@ static inline int drv_ampdu_action(struc
 
 	might_sleep();
 
-	local_bh_disable();
 	if (local->ops->ampdu_action)
 		ret = local->ops->ampdu_action(&local->hw, &sdata->vif, action,
 					       sta, tid, ssn);
-	local_bh_enable();
+
 	trace_drv_ampdu_action(local, sdata, action, sta, tid, ssn, ret);
 	return ret;
 }



^ permalink raw reply

* [RFC 20/21] mac80211: change TX aggregation locking
From: Johannes Berg @ 2010-06-07 11:01 UTC (permalink / raw)
  To: linux-wireless
In-Reply-To: <20100607110133.472649120@sipsolutions.net>

From: Johannes Berg <johannes.berg@intel.com>

To prepare for allowing drivers to sleep in
ampdu_action, change the locking in the TX
aggregation code to use the mutex the RX part
already uses. The spinlock is still necessary
around some code to avoid races with TX, but
now we can also synchronize_net() to avoid
getting an inconsistent sequence number.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/agg-tx.c     |   90 +++++++++++++++++++++++++---------------------
 net/mac80211/driver-ops.h |    3 +
 net/mac80211/ht.c         |    6 ---
 3 files changed, 54 insertions(+), 45 deletions(-)

--- wireless-testing.orig/net/mac80211/agg-tx.c	2010-06-07 12:22:12.000000000 +0200
+++ wireless-testing/net/mac80211/agg-tx.c	2010-06-07 12:32:28.000000000 +0200
@@ -140,18 +140,25 @@ int ___ieee80211_stop_tx_ba_session(stru
 	struct tid_ampdu_tx *tid_tx = sta->ampdu_mlme.tid_tx[tid];
 	int ret;
 
-	lockdep_assert_held(&sta->lock);
+	lockdep_assert_held(&sta->ampdu_mlme.mtx);
 
-	if (WARN_ON(!tid_tx))
+	spin_lock_bh(&sta->lock);
+
+	if (!tid_tx) {
+		spin_unlock_bh(&sta->lock);
 		return -ENOENT;
+	}
 
 	if (test_bit(HT_AGG_STATE_WANT_START, &tid_tx->state)) {
 		/* not even started yet! */
 		rcu_assign_pointer(sta->ampdu_mlme.tid_tx[tid], NULL);
+		spin_unlock_bh(&sta->lock);
 		call_rcu(&tid_tx->rcu_head, kfree_tid_tx);
 		return 0;
 	}
 
+	spin_unlock_bh(&sta->lock);
+
 #ifdef CONFIG_MAC80211_HT_DEBUG
 	printk(KERN_DEBUG "Tx BA session stop requested for %pM tid %u\n",
 	       sta->sta.addr, tid);
@@ -281,10 +288,11 @@ void ieee80211_tx_ba_session_handle_star
 	clear_bit(HT_AGG_STATE_WANT_START, &tid_tx->state);
 
 	/*
-	 * This might be off by one due to a race that we can't
-	 * really prevent here without synchronize_net() which
-	 * can't be called now.
+	 * make sure no packets are being processed to get
+	 * valid starting sequence number
 	 */
+	synchronize_net();
+
 	start_seq_num = sta->tid_seq[tid] >> 4;
 
 	ret = drv_ampdu_action(local, sdata, IEEE80211_AMPDU_TX_START,
@@ -294,7 +302,10 @@ void ieee80211_tx_ba_session_handle_star
 		printk(KERN_DEBUG "BA request denied - HW unavailable for"
 					" tid %d\n", tid);
 #endif
+		spin_lock_bh(&sta->lock);
 		rcu_assign_pointer(sta->ampdu_mlme.tid_tx[tid], NULL);
+		spin_unlock_bh(&sta->lock);
+
 		ieee80211_wake_queue_agg(local, tid);
 		call_rcu(&tid_tx->rcu_head, kfree_tid_tx);
 		return;
@@ -445,16 +456,25 @@ ieee80211_agg_splice_finish(struct ieee8
 	ieee80211_wake_queue_agg(local, tid);
 }
 
-/* caller must hold sta->lock */
 static void ieee80211_agg_tx_operational(struct ieee80211_local *local,
 					 struct sta_info *sta, u16 tid)
 {
-	lockdep_assert_held(&sta->lock);
+	lockdep_assert_held(&sta->ampdu_mlme.mtx);
 
 #ifdef CONFIG_MAC80211_HT_DEBUG
 	printk(KERN_DEBUG "Aggregation is on for tid %d\n", tid);
 #endif
 
+	drv_ampdu_action(local, sta->sdata,
+			 IEEE80211_AMPDU_TX_OPERATIONAL,
+			 &sta->sta, tid, NULL);
+
+	/*
+	 * synchronize with TX path, while splicing the TX path
+	 * should block so it won't put more packets onto pending.
+	 */
+	spin_lock_bh(&sta->lock);
+
 	ieee80211_agg_splice_packets(local, sta->ampdu_mlme.tid_tx[tid], tid);
 	/*
 	 * Now mark as operational. This will be visible
@@ -464,9 +484,7 @@ static void ieee80211_agg_tx_operational
 	set_bit(HT_AGG_STATE_OPERATIONAL, &sta->ampdu_mlme.tid_tx[tid]->state);
 	ieee80211_agg_splice_finish(local, tid);
 
-	drv_ampdu_action(local, sta->sdata,
-			 IEEE80211_AMPDU_TX_OPERATIONAL,
-			 &sta->sta, tid, NULL);
+	spin_unlock_bh(&sta->lock);
 }
 
 void ieee80211_start_tx_ba_cb(struct ieee80211_vif *vif, u8 *ra, u16 tid)
@@ -486,37 +504,35 @@ void ieee80211_start_tx_ba_cb(struct iee
 		return;
 	}
 
-	rcu_read_lock();
+	mutex_lock(&local->sta_mtx);
 	sta = sta_info_get(sdata, ra);
 	if (!sta) {
-		rcu_read_unlock();
+		mutex_unlock(&local->sta_mtx);
 #ifdef CONFIG_MAC80211_HT_DEBUG
 		printk(KERN_DEBUG "Could not find station: %pM\n", ra);
 #endif
 		return;
 	}
 
-	spin_lock_bh(&sta->lock);
+	mutex_lock(&sta->ampdu_mlme.mtx);
 	tid_tx = sta->ampdu_mlme.tid_tx[tid];
 
 	if (WARN_ON(!tid_tx)) {
 #ifdef CONFIG_MAC80211_HT_DEBUG
 		printk(KERN_DEBUG "addBA was not requested!\n");
 #endif
-		spin_unlock_bh(&sta->lock);
-		rcu_read_unlock();
-		return;
+		goto unlock;
 	}
 
 	if (WARN_ON(test_and_set_bit(HT_AGG_STATE_DRV_READY, &tid_tx->state)))
-		goto out;
+		goto unlock;
 
 	if (test_bit(HT_AGG_STATE_RESPONSE_RECEIVED, &tid_tx->state))
 		ieee80211_agg_tx_operational(local, sta, tid);
 
- out:
-	spin_unlock_bh(&sta->lock);
-	rcu_read_unlock();
+ unlock:
+	mutex_unlock(&sta->ampdu_mlme.mtx);
+	mutex_unlock(&local->sta_mtx);
 }
 
 void ieee80211_start_tx_ba_cb_irqsafe(struct ieee80211_vif *vif,
@@ -548,21 +564,14 @@ EXPORT_SYMBOL(ieee80211_start_tx_ba_cb_i
 int __ieee80211_stop_tx_ba_session(struct sta_info *sta, u16 tid,
 				   enum ieee80211_back_parties initiator)
 {
-	struct tid_ampdu_tx *tid_tx;
 	int ret;
 
-	spin_lock_bh(&sta->lock);
-	tid_tx = sta->ampdu_mlme.tid_tx[tid];
-
-	if (!tid_tx) {
-		ret = -ENOENT;
-		goto unlock;
-	}
+	mutex_lock(&sta->ampdu_mlme.mtx);
 
 	ret = ___ieee80211_stop_tx_ba_session(sta, tid, initiator);
 
- unlock:
-	spin_unlock_bh(&sta->lock);
+	mutex_unlock(&sta->ampdu_mlme.mtx);
+
 	return ret;
 }
 
@@ -627,16 +636,17 @@ void ieee80211_stop_tx_ba_cb(struct ieee
 	       ra, tid);
 #endif /* CONFIG_MAC80211_HT_DEBUG */
 
-	rcu_read_lock();
+	mutex_lock(&local->sta_mtx);
+
 	sta = sta_info_get(sdata, ra);
 	if (!sta) {
 #ifdef CONFIG_MAC80211_HT_DEBUG
 		printk(KERN_DEBUG "Could not find station: %pM\n", ra);
 #endif
-		rcu_read_unlock();
-		return;
+		goto unlock;
 	}
 
+	mutex_lock(&sta->ampdu_mlme.mtx);
 	spin_lock_bh(&sta->lock);
 	tid_tx = sta->ampdu_mlme.tid_tx[tid];
 
@@ -644,9 +654,7 @@ void ieee80211_stop_tx_ba_cb(struct ieee
 #ifdef CONFIG_MAC80211_HT_DEBUG
 		printk(KERN_DEBUG "unexpected callback to A-MPDU stop\n");
 #endif
-		spin_unlock_bh(&sta->lock);
-		rcu_read_unlock();
-		return;
+		goto unlock_sta;
 	}
 
 	if (tid_tx->stop_initiator == WLAN_BACK_INITIATOR)
@@ -672,8 +680,11 @@ void ieee80211_stop_tx_ba_cb(struct ieee
 
 	call_rcu(&tid_tx->rcu_head, kfree_tid_tx);
 
+ unlock_sta:
 	spin_unlock_bh(&sta->lock);
-	rcu_read_unlock();
+	mutex_unlock(&sta->ampdu_mlme.mtx);
+ unlock:
+	mutex_unlock(&local->sta_mtx);
 }
 
 void ieee80211_stop_tx_ba_cb_irqsafe(struct ieee80211_vif *vif,
@@ -714,10 +725,9 @@ void ieee80211_process_addba_resp(struct
 	capab = le16_to_cpu(mgmt->u.action.u.addba_resp.capab);
 	tid = (capab & IEEE80211_ADDBA_PARAM_TID_MASK) >> 2;
 
-	spin_lock_bh(&sta->lock);
+	mutex_lock(&sta->ampdu_mlme.mtx);
 
 	tid_tx = sta->ampdu_mlme.tid_tx[tid];
-
 	if (!tid_tx)
 		goto out;
 
@@ -751,5 +761,5 @@ void ieee80211_process_addba_resp(struct
 	}
 
  out:
-	spin_unlock_bh(&sta->lock);
+	mutex_unlock(&sta->ampdu_mlme.mtx);
 }
--- wireless-testing.orig/net/mac80211/ht.c	2010-06-07 12:22:08.000000000 +0200
+++ wireless-testing/net/mac80211/ht.c	2010-06-07 12:22:13.000000000 +0200
@@ -136,11 +136,7 @@ void ieee80211_ba_session_work(struct wo
 			___ieee80211_stop_rx_ba_session(
 				sta, tid, WLAN_BACK_RECIPIENT,
 				WLAN_REASON_QSTA_TIMEOUT);
-	}
-	mutex_unlock(&sta->ampdu_mlme.mtx);
 
-	spin_lock_bh(&sta->lock);
-	for (tid = 0; tid < STA_TID_NUM; tid++) {
 		tid_tx = sta->ampdu_mlme.tid_tx[tid];
 		if (!tid_tx)
 			continue;
@@ -152,7 +148,7 @@ void ieee80211_ba_session_work(struct wo
 			___ieee80211_stop_tx_ba_session(sta, tid,
 							WLAN_BACK_INITIATOR);
 	}
-	spin_unlock_bh(&sta->lock);
+	mutex_unlock(&sta->ampdu_mlme.mtx);
 }
 
 void ieee80211_send_delba(struct ieee80211_sub_if_data *sdata,
--- wireless-testing.orig/net/mac80211/driver-ops.h	2010-06-07 12:22:20.000000000 +0200
+++ wireless-testing/net/mac80211/driver-ops.h	2010-06-07 12:22:31.000000000 +0200
@@ -349,6 +349,9 @@ static inline int drv_ampdu_action(struc
 				   u16 *ssn)
 {
 	int ret = -EOPNOTSUPP;
+
+	might_sleep();
+
 	local_bh_disable();
 	if (local->ops->ampdu_action)
 		ret = local->ops->ampdu_action(&local->hw, &sdata->vif, action,



^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox