Skip to contentSkip to conversation

mac80211: fix suspend/resume races with unregister hw

PATCH In review Stanislaw Gruszka 3 participants 8 messages Aug 12, 2011 – Sep 8, 2011 3 messages not in the archive

Subsystems (3)
Networking drivers (wireless) mac80211 802.11 (including cfg80211/NL80211)
lore
Share & tools
lore

Stanislaw Gruszka

author
#0 lore raw

PATCH 1/1 mac80211: fix suspend/resume races with unregister hw

Do not call ->suspend, ->resume methods after we unregister wiphy. Also delete sta_clanup timer after we finish wiphy unregister to avoid this:

WARNING: at lib/debugobjects.c:262 debug_print_object+0x85/0xa0()
Hardware name: 6369CTO
ODEBUG: free active (active state 0) object type: timer_list hint: sta_info_cleanup+0x0/0x180 [mac80211]
Modules linked in: aes_i586 aes_generic fuse bridge stp llc autofs4 sunrpc cpufreq_ondemand acpi_cpufreq mperf ext2 dm_mod uinput thinkpad_acpi hwmon sg arc4 rt2800usb rt2800lib crc_ccitt rt2x00usb rt2x00lib mac80211 cfg80211 i2c_i801 iTCO_wdt iTCO_vendor_support e1000e ext4 mbcache jbd2 sd_mod crc_t10dif sr_mod cdrom yenta_socket ahci libahci pata_acpi ata_generic ata_piix i915 drm_kms_helper drm i2c_algo_bit video [last unloaded: microcode]
Pid: 5663, comm: pm-hibernate Not tainted 3.1.0-rc1-wl+ #19
Call Trace:
 [<c0454cfd>] warn_slowpath_common+0x6d/0xa0
 [<c05e05e5>] ? debug_print_object+0x85/0xa0
 [<c05e05e5>] ? debug_print_object+0x85/0xa0
 [<c0454dae>] warn_slowpath_fmt+0x2e/0x30
 [<c05e05e5>] debug_print_object+0x85/0xa0
 [<f8a808e0>] ? sta_info_alloc+0x1a0/0x1a0 [mac80211]
 [<c05e0bd2>] debug_check_no_obj_freed+0xe2/0x180
 [<c051175b>] kfree+0x8b/0x150
 [<f8a126ae>] cfg80211_dev_free+0x7e/0x90 [cfg80211]
 [<f8a13afd>] wiphy_dev_release+0xd/0x10 [cfg80211]
 [<c068d959>] device_release+0x19/0x80
 [<c05d06ba>] kobject_release+0x7a/0x1c0
 [<c07646a8>] ? rtnl_unlock+0x8/0x10
 [<f8a13adb>] ? wiphy_resume+0x6b/0x80 [cfg80211]
 [<c05d0640>] ? kobject_del+0x30/0x30
 [<c05d1a6d>] kref_put+0x2d/0x60
 [<c05d056d>] kobject_put+0x1d/0x50
 [<c08015f4>] ? mutex_lock+0x14/0x40
 [<c068d60f>] put_device+0xf/0x20
 [<c069716a>] dpm_resume+0xca/0x160
 [<c04912bd>] hibernation_snapshot+0xcd/0x260
 [<c04903df>] ? freeze_processes+0x3f/0x90
 [<c049151b>] hibernate+0xcb/0x1e0
 [<c048fdc0>] ? pm_async_store+0x40/0x40
 [<c048fe60>] state_store+0xa0/0xb0
 [<c048fdc0>] ? pm_async_store+0x40/0x40
 [<c05d0200>] kobj_attr_store+0x20/0x30
 [<c0575ea4>] sysfs_write_file+0x94/0xf0
 [<c051e26a>] vfs_write+0x9a/0x160
 [<c0575e10>] ? sysfs_open_file+0x200/0x200
 [<c051e3fd>] sys_write+0x3d/0x70
 [<c080959f>] sysenter_do_call+0x12/0x28
Cc [email protected]
Signed-off-by Stanislaw Gruszka <[email protected]>
---
 include/net/cfg80211.h |    3 +++
 net/mac80211/main.c    |    2 +-
 net/wireless/core.c    |    7 +++++++
 net/wireless/sysfs.c   |    6 ++++--
 4 files changed, 15 insertions(+), 3 deletions(-)
include/net/cfg80211.h +3 -0
index ab12440..97db267 100644
@@ -1875,6 +1875,9 @@ struct wiphy {
 	 * you need use set_wiphy_dev() (see below) */
 	struct device dev;
 
+	/* protects ->resume, ->suspend sysfs callbacks against unregister hw */
+	bool registered;
+
 	/* dir in debugfs: ieee80211/<wiphyname> */
 	struct dentry *debugfsdir;
net/mac80211/main.c +1 -1
index 104fdd9..a5809a1 100644
@@ -1013,7 +1013,6 @@ void ieee80211_unregister_hw(struct ieee80211_hw *hw)
 	cancel_work_sync(&local->reconfig_filter);
 
 	ieee80211_clear_tx_pending(local);
-	sta_info_stop(local);
 	rate_control_deinitialize(local);
 
 	if (skb_queue_len(&local->skb_queue) ||

@@ -1025,6 +1024,7 @@ void ieee80211_unregister_hw(struct ieee80211_hw *hw)
 
 	destroy_workqueue(local->workqueue);
 	wiphy_unregister(local->hw.wiphy);
+	sta_info_stop(local);
 	ieee80211_wep_free(local);
 	ieee80211_led_exit(local);
 	kfree(local->int_scan_req);
net/wireless/core.c +7 -0
index 645437c..c148651 100644
@@ -616,6 +616,9 @@ int wiphy_register(struct wiphy *wiphy)
 	if (res)
 		goto out_rm_dev;
 
+	rtnl_lock();
+	rdev->wiphy.registered = true;
+	rtnl_unlock();
 	return 0;
 
 out_rm_dev:

@@ -647,6 +650,10 @@ void wiphy_unregister(struct wiphy *wiphy)
 {
 	struct cfg80211_registered_device *rdev = wiphy_to_dev(wiphy);
 
+	rtnl_lock();
+	rdev->wiphy.registered = false;
+	rtnl_unlock();
+
 	rfkill_unregister(rdev->rfkill);
 
 	/* protect the device list */
net/wireless/sysfs.c +4 -2
index c6e4ca6..ff57459 100644
@@ -93,7 +93,8 @@ static int wiphy_suspend(struct device *dev, pm_message_t state)
 
 	if (rdev->ops->suspend) {
 		rtnl_lock();
-		ret = rdev->ops->suspend(&rdev->wiphy, rdev->wowlan);
+		if (rdev->wiphy.registered)
+			ret = rdev->ops->suspend(&rdev->wiphy, rdev->wowlan);
 		rtnl_unlock();
 	}
 

@@ -112,7 +113,8 @@ static int wiphy_resume(struct device *dev)
 
 	if (rdev->ops->resume) {
 		rtnl_lock();
-		ret = rdev->ops->resume(&rdev->wiphy);
+		if (rdev->wiphy.registered)
+			ret = rdev->ops->resume(&rdev->wiphy);
 		rtnl_unlock();
 	}
Signature · 1 line
1.7.1
maintainer
#1 lore raw

include/net/cfg80211.h struct wiphy { · line 1879

Quoted 10 lines · Stanislaw Gruszka
include/net/cfg80211.h +2 -0
index ab12440..97db267 100644
@@ -1875,6 +1875,9 @@ struct wiphy {
 	 * you need use set_wiphy_dev() (see below) */
 	struct device dev;
 
+	/* protects ->resume, ->suspend sysfs callbacks against unregister hw */
+	bool registered;

Warning(/home/wireless/wireless-testing//include/net/cfg80211.h:1927):
No description found for parameter 'registered'

please fix.

johannes

author
#2 lore raw

PATCH 1/1 cfg80211: document wiphy->registered

Signed-off-by Stanislaw Gruszka <[email protected]>
---
 include/net/cfg80211.h |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
include/net/cfg80211.h +1 -0
index 791dd10..f226552 100644
@@ -1784,6 +1784,7 @@ struct wiphy_wowlan_support {
  * @debugfsdir: debugfs directory used for this wiphy, will be renamed
  *	automatically on wiphy renames
  * @dev: (virtual) struct device for this wiphy
+ * @registered: helps synchronize suspend/resume with wiphy unregister
  * @wext: wireless extension handlers
  * @priv: driver private data (sized according to wiphy_new() parameter)
  * @interface_modes: bitmask of interfaces types valid for this wiphy,
Signature · 1 line
1.7.1

A message that is not in the archive (sent off-list or not yet received): [email protected]

author
#5 lore raw
Quoted 12 lines · Michael Witten

My boot process is stalling proximate the following messages (after a period of time, some other message might show up, but the boot never progresses much further):

cfg80211: World regulatory domain updated:

  cfg80211:     (start_freq - end_freq @ bandwidth), (max_antenna_gain, max_eirp)
  cfg80211:     (2402000 KHz - 2472000 KHz @ 40000 KHz), (300 mBi, 2000 mBm)
  cfg80211:     (2457000 KHz - 2482000 KHz @ 20000 KHz), (300 mBi, 2000 mBm)
  cfg80211:     (2474000 KHz - 2494000 KHz @ 20000 KHz), (300 mBi, 2000 mBm)
  cfg80211:     (5170000 KHz - 5250000 KHz @ 40000 KHz), (300 mBi, 2000 mBm)
  cfg80211:     (5735000 KHz - 5835000 KHz @ 40000 KHz), (300 mBi, 2000 mBm)
Quoted 5 lines

I bisected to this patch, namely via the following commit:

commit ecb4433550f0620f3d1471ae7099037ede30a91e
Author: Stanislaw Gruszka <[email protected]>
Date: Fri Aug 12 14:00:59 2011 +0200

Grr, crap.

Quoted 7 lines

Now, it may be the case that my system startup's scripts are doing something funny, as the stall described above occurs after Arch Linux outputs:

Waiting for UDev uevents to be processed

At this point, that's all I know.

What wireless driver are you using? If this is ipw2x00 please try attached patch.

Thanks
Stanislaw

drivers/net/wireless/ipw2x00/ipw2100.c +14 -7
index 3774dd0..ef9ad79 100644
@@ -1903,15 +1903,17 @@ static void ipw2100_down(struct ipw2100_priv *priv)
 static int ipw2100_net_init(struct net_device *dev)
 {
 	struct ipw2100_priv *priv = libipw_priv(dev);
+
+	return ipw2100_up(priv, 1);
+}
+
+static int ipw2100_wdev_init(struct net_device *dev)
+{
+	struct ipw2100_priv *priv = libipw_priv(dev);
 	const struct libipw_geo *geo = libipw_get_geo(priv->ieee);
 	struct wireless_dev *wdev = &priv->ieee->wdev;
-	int ret;
 	int i;
 
-	ret = ipw2100_up(priv, 1);
-	if (ret)
-		return ret;
-
 	memcpy(wdev->wiphy->perm_addr, priv->mac_addr, ETH_ALEN);
 
 	/* fill-out priv->ieee->bg_band */

@@ -6350,9 +6352,13 @@ static int ipw2100_pci_init_one(struct pci_dev *pci_dev,
 		       "Error calling register_netdev.\n");
 		goto fail;
 	}
+	registered = 1;
+
+	err = ipw2100_wdev_init(dev);
+	if (err)
+		goto fail;
 
 	mutex_lock(&priv->action_mutex);
-	registered = 1;
 
 	IPW_DEBUG_INFO("%s: Bound to %s\n", dev->name, pci_name(pci_dev));
 

@@ -6389,7 +6395,8 @@ static int ipw2100_pci_init_one(struct pci_dev *pci_dev,
 
       fail_unlock:
 	mutex_unlock(&priv->action_mutex);
-
+	wiphy_unregister(priv->ieee->wdev.wiphy);
+	kfree(priv->ieee->bg_band.channels);
       fail:
 	if (dev) {
 		if (registered)
drivers/net/wireless/ipw2x00/ipw2200.c +27 -14

83 lines not on this page · raw message

A message that is not in the archive (sent off-list or not yet received): CAMOZ1BvsPok-AZVCwKnv0hWWXyV+np+nARjGRqwqsvC-du_GAw@mail.gmail.com

author
#7 lore raw
Quoted 28 lines · Michael Witten
Quoted 27 lines · Stanislaw Gruszka
Quoted 12 lines · Michael Witten

My boot process is stalling proximate the following messages (after a period of time, some other message might show up, but the boot never progresses much further):

  cfg80211: World regulatory domain updated:
  cfg80211:     (start_freq - end_freq @ bandwidth), (max_antenna_gain, max_eirp)
  cfg80211:     (2402000 KHz - 2472000 KHz @ 40000 KHz), (300 mBi, 2000 mBm)
  cfg80211:     (2457000 KHz - 2482000 KHz @ 20000 KHz), (300 mBi, 2000 mBm)
  cfg80211:     (2474000 KHz - 2494000 KHz @ 20000 KHz), (300 mBi, 2000 mBm)
  cfg80211:     (5170000 KHz - 5250000 KHz @ 40000 KHz), (300 mBi, 2000 mBm)
  cfg80211:     (5735000 KHz - 5835000 KHz @ 40000 KHz), (300 mBi, 2000 mBm)

Quoted 5 lines

I bisected to this patch, namely via the following commit:

  commit ecb4433550f0620f3d1471ae7099037ede30a91e
  Author: Stanislaw Gruszka <[email protected]>
  Date:   Fri Aug 12 14:00:59 2011 +0200

Grr, crap.

Quoted 6 lines

Now, it may be the case that my system startup's scripts are doing something funny, as the stall described above occurs after Arch Linux outputs:

  Waiting for UDev uevents to be processed

Quoted 8 lines
Quoted 4 lines
Quoted 1 line

At this point, that's all I know.

What wireless driver are you using? If this is ipw2x00 please try attached patch.

I am indeed using `ipw2200', and your patch seems to fix the problem.

How did you know what to do?

You are not the first person who hit this bug and did bisection :-/ Witold (CCed) did it. Can you also check if wireless network and rmmod ipw2x00 works? I did not yet receive that information from Witold regarding latest patch.

Thanks
Stanislaw

#8 lore raw
Quoted 34 lines · Stanislaw Gruszka
Quoted 33 lines · Michael Witten
Quoted 31 lines · Stanislaw Gruszka
Quoted 12 lines · Michael Witten

My boot process is stalling proximate the following messages (after a period of time, some other message might show up, but the boot never progresses much further):

  cfg80211: World regulatory domain updated:
  cfg80211:     (start_freq - end_freq @ bandwidth), (max_antenna_gain, max_eirp)
  cfg80211:     (2402000 KHz - 2472000 KHz @ 40000 KHz), (300 mBi, 2000 mBm)
  cfg80211:     (2457000 KHz - 2482000 KHz @ 20000 KHz), (300 mBi, 2000 mBm)
  cfg80211:     (2474000 KHz - 2494000 KHz @ 20000 KHz), (300 mBi, 2000 mBm)
  cfg80211:     (5170000 KHz - 5250000 KHz @ 40000 KHz), (300 mBi, 2000 mBm)
  cfg80211:     (5735000 KHz - 5835000 KHz @ 40000 KHz), (300 mBi, 2000 mBm)

Quoted 5 lines

I bisected to this patch, namely via the following commit:

  commit ecb4433550f0620f3d1471ae7099037ede30a91e
  Author: Stanislaw Gruszka <[email protected]>
  Date:   Fri Aug 12 14:00:59 2011 +0200

Grr, crap.

Quoted 7 lines

Now, it may be the case that my system startup's scripts are doing something funny, as the stall described above occurs after Arch Linux outputs:

  Waiting for UDev uevents to be processed

At this point, that's all I know.

What wireless driver are you using? If this is ipw2x00 please try attached patch.

Quoted 8 lines
Quoted 3 lines

I am indeed using `ipw2200', and your patch seems to fix the problem.

How did you know what to do?

You are not the first person who hit this bug and did bisection :-/ Witold (CCed) did it. Can you also check if wireless network and rmmod ipw2x00 works? I did not yet receive that information from Witold regarding latest patch.

I'm just checking it (latest patch). Hit some another bug in logfs :)

Signature · 1 line
Witold Baryluk

A message that is not in the archive (sent off-list or not yet received): CAMOZ1Bs_DesZoyYw6yzC8CdqR3yLgXXpBzxvsEsQ0eX4L6Vw+A@mail.gmail.com

#10 lore raw
Quoted 8 lines · Michael Witten

Interestingly, though, `lsmod' shows the following:

Module Size Used by
...
ipw2200 109206 0
...

Quoted 5 lines

even when the wireless network is up and running; I would have expected the Used by' column to show that ipw2200' is being used by at least something else; the consequence is that I am able to run rmmod' on ipw2200' while the wireless network is up and running, which seems odd (though it causes no discernible error).

Hi.

I think this was always so. I just checked other system with vanila 3.0.0 kernel, and it also says "Used by" == 0 I do not remember exactly, but I think it was always saying zero. You can actually always rmmod ipw220, regardles whetever network interface is used, opened, up, or whetever. I guess hot plug is important here.

Regards,
Witek

Signature · 1 line
Witold Baryluk