2009-03-13 10:25:52

by Johannes Berg

[permalink] [raw]
Subject: [PATCH] mac80211: resume properly, add suspend/resume test

When mac80211 resumes, it currently doesn't reconfigure the interfaces
entirely and also doesn't reconfigure BSS information -- fix this.

Also, to be able to test this, add a debugfs file that just calls
the suspend/resume code to see what happens when we go through that,
without needing the time-consuming suspend/resume cycle.

Signed-off-by: Johannes Berg <[email protected]>
---
zd1211rw currently doesn't survive this properly, I'm not sure why,
but it keeps saying
zd1211rw 1-1:1.0: error ioread32(CR_REG1): -11

Please test with other hardware, we need to figure out whether or
not something is missing in mac80211.

net/mac80211/debugfs.c | 24 ++++++++++++++++++++++++
net/mac80211/ieee80211_i.h | 1 +
net/mac80211/pm.c | 29 +++++++++++++++++++++++++++++
3 files changed, 54 insertions(+)

--- wireless-testing.orig/net/mac80211/pm.c 2009-03-13 10:44:34.000000000 +0100
+++ wireless-testing/net/mac80211/pm.c 2009-03-13 11:09:29.000000000 +0100
@@ -113,5 +113,34 @@ int __ieee80211_resume(struct ieee80211_
ieee80211_configure_filter(local);
netif_addr_unlock_bh(local->mdev);

+ /* Finally also reconfigure all the BSS information */
+ list_for_each_entry(sdata, &local->interfaces, list) {
+ u32 changed = ~0;
+ if (!netif_running(sdata->dev))
+ continue;
+ switch (sdata->vif.type) {
+ case NL80211_IFTYPE_STATION:
+ /* disable beacon change bits */
+ changed &= ~IEEE80211_IFCC_BEACON;
+ /* fall through */
+ case NL80211_IFTYPE_ADHOC:
+ case NL80211_IFTYPE_AP:
+ case NL80211_IFTYPE_MESH_POINT:
+ WARN_ON(ieee80211_if_config(sdata, changed));
+ ieee80211_bss_info_change_notify(sdata, ~0);
+ break;
+ case NL80211_IFTYPE_WDS:
+ break;
+ case NL80211_IFTYPE_AP_VLAN:
+ case NL80211_IFTYPE_MONITOR:
+ /* ignore virtual */
+ break;
+ case NL80211_IFTYPE_UNSPECIFIED:
+ case __NL80211_IFTYPE_AFTER_LAST:
+ WARN_ON(1);
+ break;
+ }
+ }
+
return 0;
}
--- wireless-testing.orig/net/mac80211/debugfs.c 2009-03-13 10:52:51.000000000 +0100
+++ wireless-testing/net/mac80211/debugfs.c 2009-03-13 11:24:09.000000000 +0100
@@ -40,6 +40,10 @@ static const struct file_operations name
local->debugfs.name = debugfs_create_file(#name, 0400, phyd, \
local, &name## _ops);

+#define DEBUGFS_ADD_MODE(name, mode) \
+ local->debugfs.name = debugfs_create_file(#name, mode, phyd, \
+ local, &name## _ops);
+
#define DEBUGFS_DEL(name) \
debugfs_remove(local->debugfs.name); \
local->debugfs.name = NULL;
@@ -113,6 +117,24 @@ static const struct file_operations tsf_
.open = mac80211_open_file_generic
};

+static ssize_t reset_write(struct file *file, const char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ struct ieee80211_local *local = file->private_data;
+
+ rtnl_lock();
+ __ieee80211_suspend(&local->hw);
+ __ieee80211_resume(&local->hw);
+ rtnl_unlock();
+
+ return count;
+}
+
+static const struct file_operations reset_ops = {
+ .write = reset_write,
+ .open = mac80211_open_file_generic,
+};
+
/* statistics stuff */

#define DEBUGFS_STATS_FILE(name, buflen, fmt, value...) \
@@ -254,6 +276,7 @@ void debugfs_hw_add(struct ieee80211_loc
DEBUGFS_ADD(total_ps_buffered);
DEBUGFS_ADD(wep_iv);
DEBUGFS_ADD(tsf);
+ DEBUGFS_ADD_MODE(reset, 0200);

statsd = debugfs_create_dir("statistics", phyd);
local->debugfs.statistics = statsd;
@@ -308,6 +331,7 @@ void debugfs_hw_del(struct ieee80211_loc
DEBUGFS_DEL(total_ps_buffered);
DEBUGFS_DEL(wep_iv);
DEBUGFS_DEL(tsf);
+ DEBUGFS_DEL(reset);

DEBUGFS_STATS_DEL(transmitted_fragment_count);
DEBUGFS_STATS_DEL(multicast_transmitted_frame_count);
--- wireless-testing.orig/net/mac80211/ieee80211_i.h 2009-03-13 10:55:52.000000000 +0100
+++ wireless-testing/net/mac80211/ieee80211_i.h 2009-03-13 10:55:57.000000000 +0100
@@ -777,6 +777,7 @@ struct ieee80211_local {
struct dentry *total_ps_buffered;
struct dentry *wep_iv;
struct dentry *tsf;
+ struct dentry *reset;
struct dentry *statistics;
struct local_debugfsdentries_statsdentries {
struct dentry *transmitted_fragment_count;




2009-03-13 10:44:51

by Johannes Berg

[permalink] [raw]
Subject: [PATCH v2] mac80211: resume properly, add suspend/resume test

When mac80211 resumes, it currently doesn't reconfigure the interfaces
entirely and also doesn't reconfigure BSS information -- fix this.

Also, to be able to test this, add a debugfs file that just calls
the suspend/resume code to see what happens when we go through that,
without needing the time-consuming suspend/resume cycle.

Signed-off-by: Johannes Berg <[email protected]>
---
v2: rebased on top of "stop queues across suspend/resume"

zd1211rw currently doesn't survive this properly, I'm not sure why,
but it keeps saying
zd1211rw 1-1:1.0: error ioread32(CR_REG1): -11

Please test with other hardware, we need to figure out whether or
not something is missing in mac80211.

net/mac80211/debugfs.c | 24 ++++++++++++++++++++++++
net/mac80211/ieee80211_i.h | 1 +
net/mac80211/pm.c | 29 +++++++++++++++++++++++++++++
3 files changed, 54 insertions(+)

--- wireless-testing.orig/net/mac80211/pm.c 2009-03-13 11:40:56.000000000 +0100
+++ wireless-testing/net/mac80211/pm.c 2009-03-13 11:42:15.000000000 +0100
@@ -116,6 +116,35 @@ int __ieee80211_resume(struct ieee80211_
ieee80211_configure_filter(local);
netif_addr_unlock_bh(local->mdev);

+ /* Finally also reconfigure all the BSS information */
+ list_for_each_entry(sdata, &local->interfaces, list) {
+ u32 changed = ~0;
+ if (!netif_running(sdata->dev))
+ continue;
+ switch (sdata->vif.type) {
+ case NL80211_IFTYPE_STATION:
+ /* disable beacon change bits */
+ changed &= ~IEEE80211_IFCC_BEACON;
+ /* fall through */
+ case NL80211_IFTYPE_ADHOC:
+ case NL80211_IFTYPE_AP:
+ case NL80211_IFTYPE_MESH_POINT:
+ WARN_ON(ieee80211_if_config(sdata, changed));
+ ieee80211_bss_info_change_notify(sdata, ~0);
+ break;
+ case NL80211_IFTYPE_WDS:
+ break;
+ case NL80211_IFTYPE_AP_VLAN:
+ case NL80211_IFTYPE_MONITOR:
+ /* ignore virtual */
+ break;
+ case NL80211_IFTYPE_UNSPECIFIED:
+ case __NL80211_IFTYPE_AFTER_LAST:
+ WARN_ON(1);
+ break;
+ }
+ }
+
ieee80211_wake_queues_by_reason(hw,
IEEE80211_QUEUE_STOP_REASON_SUSPEND);

--- wireless-testing.orig/net/mac80211/debugfs.c 2009-03-13 11:36:57.000000000 +0100
+++ wireless-testing/net/mac80211/debugfs.c 2009-03-13 11:41:53.000000000 +0100
@@ -40,6 +40,10 @@ static const struct file_operations name
local->debugfs.name = debugfs_create_file(#name, 0400, phyd, \
local, &name## _ops);

+#define DEBUGFS_ADD_MODE(name, mode) \
+ local->debugfs.name = debugfs_create_file(#name, mode, phyd, \
+ local, &name## _ops);
+
#define DEBUGFS_DEL(name) \
debugfs_remove(local->debugfs.name); \
local->debugfs.name = NULL;
@@ -113,6 +117,24 @@ static const struct file_operations tsf_
.open = mac80211_open_file_generic
};

+static ssize_t reset_write(struct file *file, const char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ struct ieee80211_local *local = file->private_data;
+
+ rtnl_lock();
+ __ieee80211_suspend(&local->hw);
+ __ieee80211_resume(&local->hw);
+ rtnl_unlock();
+
+ return count;
+}
+
+static const struct file_operations reset_ops = {
+ .write = reset_write,
+ .open = mac80211_open_file_generic,
+};
+
/* statistics stuff */

#define DEBUGFS_STATS_FILE(name, buflen, fmt, value...) \
@@ -254,6 +276,7 @@ void debugfs_hw_add(struct ieee80211_loc
DEBUGFS_ADD(total_ps_buffered);
DEBUGFS_ADD(wep_iv);
DEBUGFS_ADD(tsf);
+ DEBUGFS_ADD_MODE(reset, 0200);

statsd = debugfs_create_dir("statistics", phyd);
local->debugfs.statistics = statsd;
@@ -308,6 +331,7 @@ void debugfs_hw_del(struct ieee80211_loc
DEBUGFS_DEL(total_ps_buffered);
DEBUGFS_DEL(wep_iv);
DEBUGFS_DEL(tsf);
+ DEBUGFS_DEL(reset);

DEBUGFS_STATS_DEL(transmitted_fragment_count);
DEBUGFS_STATS_DEL(multicast_transmitted_frame_count);
--- wireless-testing.orig/net/mac80211/ieee80211_i.h 2009-03-13 11:40:41.000000000 +0100
+++ wireless-testing/net/mac80211/ieee80211_i.h 2009-03-13 11:41:53.000000000 +0100
@@ -775,6 +775,7 @@ struct ieee80211_local {
struct dentry *total_ps_buffered;
struct dentry *wep_iv;
struct dentry *tsf;
+ struct dentry *reset;
struct dentry *statistics;
struct local_debugfsdentries_statsdentries {
struct dentry *transmitted_fragment_count;



2009-03-13 18:51:23

by Bob Copeland

[permalink] [raw]
Subject: Re: [PATCH v2] mac80211: resume properly, add suspend/resume test

On Fri, Mar 13, 2009 at 6:58 AM, Johannes Berg
<[email protected]> wrote:
>> ath9k survives ping -f without even blinking.
>
> Actually, not quite:
>
> [ =A0289.753866]
> [ =A0289.753869] =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> [ =A0289.754164] [ INFO: inconsistent lock state ]
> [ =A0289.754372] 2.6.29-rc7-wl-17155-g2f7ab92-dirty #29
> [ =A0289.754598] ---------------------------------
> [ =A0289.754805] inconsistent {in-softirq-W} -> {softirq-on-W} usage.

[softirq-on-W stack looks like:

sta_notify
ath_node_detach
ath_tx_node_cleanup
spin_lock()

vs the ath9k_tx_tasklet]

> Looks like we need to disable BHs for the sta functions or something?
>
> johannes

Yup... or pm can take sta_lock with spin_lock_irqsave() like the other
callers of sta_notify.

=46eel free to add my Acked-by on both the PM patches. I'd been using
CONFIG_PM_DEBUG to simulate suspend/resume but since you have to
do 'devices' anyway to get the desired effect, the debug file here
is nicer.

--=20
Bob Copeland %% http://www.bobcopeland.com

2009-03-13 10:59:02

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v2] mac80211: resume properly, add suspend/resume test

On Fri, 2009-03-13 at 11:48 +0100, Johannes Berg wrote:
> On Fri, 2009-03-13 at 11:44 +0100, Johannes Berg wrote:
> > When mac80211 resumes, it currently doesn't reconfigure the interfaces
> > entirely and also doesn't reconfigure BSS information -- fix this.
> >
> > Also, to be able to test this, add a debugfs file that just calls
> > the suspend/resume code to see what happens when we go through that,
> > without needing the time-consuming suspend/resume cycle.
> >
> > Signed-off-by: Johannes Berg <[email protected]>
> > ---
> > v2: rebased on top of "stop queues across suspend/resume"
> >
> > zd1211rw currently doesn't survive this properly, I'm not sure why,
> > but it keeps saying
> > zd1211rw 1-1:1.0: error ioread32(CR_REG1): -11
> >
> > Please test with other hardware, we need to figure out whether or
> > not something is missing in mac80211.
>
> ath9k survives ping -f without even blinking.

Actually, not quite:

[ 289.753866]
[ 289.753869] =================================
[ 289.754164] [ INFO: inconsistent lock state ]
[ 289.754372] 2.6.29-rc7-wl-17155-g2f7ab92-dirty #29
[ 289.754598] ---------------------------------
[ 289.754805] inconsistent {in-softirq-W} -> {softirq-on-W} usage.
[ 289.755090] bash/5240 [HC0[0]:SC0[0]:HE1:SE1] takes:
[ 289.755324] (&txq->axq_lock){-+..}, at: [<d0000000023de2e8>] .ath_tx_node_cleanup+0xa4/0x1cc [ath9k]
[ 289.755828] {in-softirq-W} state was registered at:
[ 289.756058] [<c00000000008b3c8>] .__lock_acquire+0x6e4/0x908
[ 289.756350] [<c00000000008b690>] .lock_acquire+0xa4/0xec
[ 289.756621] [<c00000000042b980>] ._spin_lock_bh+0x58/0xb8
[ 289.756898] [<d0000000023e0938>] .ath_tx_processq+0x9c/0x564 [ath9k]
[ 289.757243] [<d0000000023e0e78>] .ath_tx_tasklet+0x78/0xa0 [ath9k]
[ 289.757579] [<d0000000023db574>] .ath9k_tasklet+0x8c/0xbc [ath9k]
[ 289.757910] [<c00000000005ca18>] .tasklet_action+0x150/0x248
[ 289.758200] [<c00000000005da60>] .__do_softirq+0xf4/0x218
[ 289.758477] [<c00000000000c4c8>] .do_softirq+0x5c/0xb8
[ 289.758741] [<c00000000005d850>] .local_bh_enable+0xd4/0x128
[ 289.759030] [<c000000000397ae4>] .dev_queue_xmit+0x428/0x47c
[ 289.759320] [<d0000000022e0620>] .ieee80211_tx_skb+0x5c/0x70 [mac80211]
[ 289.759700] [<d0000000022c772c>] .ieee80211_scan_work+0x158/0x1c0 [mac80211]
[ 289.760081] [<c00000000006dd50>] .run_workqueue+0x16c/0x298
[ 289.771199] [<c00000000006dfa4>] .worker_thread+0x128/0x154
[ 289.782275] [<c00000000007365c>] .kthread+0x78/0xc4
[ 289.793327] [<c000000000022908>] .kernel_thread+0x54/0x70
[ 289.804332] irq event stamp: 96461
[ 289.815187] hardirqs last enabled at (96461): [<c000000000428908>] .__mutex_unlock_slowpath+0x1cc/0x218
[ 289.826501] hardirqs last disabled at (96460): [<c0000000004287bc>] .__mutex_unlock_slowpath+0x80/0x218
[ 289.837782] softirqs last enabled at (96424): [<c00000000000c4c8>] .do_softirq+0x5c/0xb8
[ 289.849079] softirqs last disabled at (96413): [<c00000000000c4c8>] .do_softirq+0x5c/0xb8
[ 289.860229]
[ 289.860230] other info that might help us debug this:
[ 289.882122] 1 lock held by bash/5240:
[ 289.893117] #0: (rtnl_mutex){--..}, at: [<c0000000003a46ac>] .rtnl_lock+0x20/0x38
[ 289.904375]
[ 289.904376] stack backtrace:
[ 289.926731] Call Trace:
[ 289.937918] [c00000020484b490] [c00000000000fef8] .show_stack+0x6c/0x174 (unreliable)
[ 289.949404] [c00000020484b540] [c000000000087918] .print_usage_bug+0x1d8/0x210
[ 289.960846] [c00000020484b600] [c000000000088458] .mark_lock_irq+0x5d0/0x83c
[ 289.972185] [c00000020484b690] [c000000000088998] .mark_lock+0x2d4/0x474
[ 289.983384] [c00000020484b730] [c000000000088c60] .mark_irqflags+0x128/0x160
[ 289.994515] [c00000020484b7c0] [c00000000008b3c8] .__lock_acquire+0x6e4/0x908
[ 290.005626] [c00000020484b8c0] [c00000000008b690] .lock_acquire+0xa4/0xec
[ 290.016727] [c00000020484b980] [c00000000042b8c8] ._spin_lock+0x50/0xb0
[ 290.027837] [c00000020484ba10] [d0000000023de2e8] .ath_tx_node_cleanup+0xa4/0x1cc [ath9k]
[ 290.038983] [c00000020484bb00] [d0000000023d90b0] .ath9k_sta_notify+0xf8/0x118 [ath9k]
[ 290.050108] [c00000020484bb90] [d0000000022ecc44] .__ieee80211_suspend+0xb0/0x1e0 [mac80211]
[ 290.061261] [c00000020484bc50] [d0000000022e3914] .reset_write+0x2c/0x60 [mac80211]
[ 290.072397] [c00000020484bce0] [c00000000010b7c0] .vfs_write+0xd0/0x1bc
[ 290.083465] [c00000020484bd80] [c00000000010b9b4] .SyS_write+0x58/0xa0
[ 290.094494] [c00000020484be30] [c000000000007554] syscall_exit+0x0/0x40

Looks like we need to disable BHs for the sta functions or something?

johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2009-03-13 10:51:18

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v2] mac80211: resume properly, add suspend/resume test

On Fri, 2009-03-13 at 11:48 +0100, Johannes Berg wrote:
> On Fri, 2009-03-13 at 11:44 +0100, Johannes Berg wrote:
> > When mac80211 resumes, it currently doesn't reconfigure the interfaces
> > entirely and also doesn't reconfigure BSS information -- fix this.
> >
> > Also, to be able to test this, add a debugfs file that just calls
> > the suspend/resume code to see what happens when we go through that,
> > without needing the time-consuming suspend/resume cycle.
> >
> > Signed-off-by: Johannes Berg <[email protected]>
> > ---
> > v2: rebased on top of "stop queues across suspend/resume"
> >
> > zd1211rw currently doesn't survive this properly, I'm not sure why,
> > but it keeps saying
> > zd1211rw 1-1:1.0: error ioread32(CR_REG1): -11
> >
> > Please test with other hardware, we need to figure out whether or
> > not something is missing in mac80211.
>
> ath9k survives ping -f without even blinking.

So does iwlwifi.

johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2009-03-13 10:49:33

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v2] mac80211: resume properly, add suspend/resume test

On Fri, 2009-03-13 at 11:44 +0100, Johannes Berg wrote:
> When mac80211 resumes, it currently doesn't reconfigure the interfaces
> entirely and also doesn't reconfigure BSS information -- fix this.
>
> Also, to be able to test this, add a debugfs file that just calls
> the suspend/resume code to see what happens when we go through that,
> without needing the time-consuming suspend/resume cycle.
>
> Signed-off-by: Johannes Berg <[email protected]>
> ---
> v2: rebased on top of "stop queues across suspend/resume"
>
> zd1211rw currently doesn't survive this properly, I'm not sure why,
> but it keeps saying
> zd1211rw 1-1:1.0: error ioread32(CR_REG1): -11
>
> Please test with other hardware, we need to figure out whether or
> not something is missing in mac80211.

ath9k survives ping -f without even blinking.

johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2009-03-23 19:51:18

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v2] mac80211: resume properly, add suspend/resume test

On Mon, 2009-03-23 at 15:43 -0400, John W. Linville wrote:

> > Indeed, this file should depend on CONFIG_PM too I suppose, will fix
> > that. OTOH, a future patch I have will require _resume to always be
> > present, so I'm inclined to just always put the code in. I'll come up
> > with a fix in any case.
>
> I fixed this in wireless-testing. Feel free to change it as you like it.

Yeah, I saw, thanks, good for now. I'll fix it with the hw-recover patch
I have.

johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2009-03-20 16:03:20

by Jouni Malinen

[permalink] [raw]
Subject: Re: [PATCH v2] mac80211: resume properly, add suspend/resume test

On Fri, Mar 13, 2009 at 11:44:18AM +0100, Johannes Berg wrote:

> --- wireless-testing.orig/net/mac80211/debugfs.c 2009-03-13 11:36:57.000000000 +0100
> +++ wireless-testing/net/mac80211/debugfs.c 2009-03-13 11:41:53.000000000 +0100
> +static ssize_t reset_write(struct file *file, const char __user *user_buf,
> + size_t count, loff_t *ppos)

> + __ieee80211_suspend(&local->hw);
> + __ieee80211_resume(&local->hw);

This seems to have broken build if CONFIG_PM is not defined: pm.c, with
these functions, is not included.

--
Jouni Malinen PGP id EFC895FA

2009-03-23 19:45:41

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH v2] mac80211: resume properly, add suspend/resume test

On Sat, Mar 21, 2009 at 08:59:43AM +0100, Johannes Berg wrote:
> On Fri, 2009-03-20 at 18:03 +0200, Jouni Malinen wrote:
> > On Fri, Mar 13, 2009 at 11:44:18AM +0100, Johannes Berg wrote:
> >
> > > --- wireless-testing.orig/net/mac80211/debugfs.c 2009-03-13 11:36:57.000000000 +0100
> > > +++ wireless-testing/net/mac80211/debugfs.c 2009-03-13 11:41:53.000000000 +0100
> > > +static ssize_t reset_write(struct file *file, const char __user *user_buf,
> > > + size_t count, loff_t *ppos)
> >
> > > + __ieee80211_suspend(&local->hw);
> > > + __ieee80211_resume(&local->hw);
> >
> > This seems to have broken build if CONFIG_PM is not defined: pm.c, with
> > these functions, is not included.
>
> Indeed, this file should depend on CONFIG_PM too I suppose, will fix
> that. OTOH, a future patch I have will require _resume to always be
> present, so I'm inclined to just always put the code in. I'll come up
> with a fix in any case.

I fixed this in wireless-testing. Feel free to change it as you like it.

--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.

2009-03-21 07:59:51

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v2] mac80211: resume properly, add suspend/resume test

On Fri, 2009-03-20 at 18:03 +0200, Jouni Malinen wrote:
> On Fri, Mar 13, 2009 at 11:44:18AM +0100, Johannes Berg wrote:
>
> > --- wireless-testing.orig/net/mac80211/debugfs.c 2009-03-13 11:36:57.000000000 +0100
> > +++ wireless-testing/net/mac80211/debugfs.c 2009-03-13 11:41:53.000000000 +0100
> > +static ssize_t reset_write(struct file *file, const char __user *user_buf,
> > + size_t count, loff_t *ppos)
>
> > + __ieee80211_suspend(&local->hw);
> > + __ieee80211_resume(&local->hw);
>
> This seems to have broken build if CONFIG_PM is not defined: pm.c, with
> these functions, is not included.

Indeed, this file should depend on CONFIG_PM too I suppose, will fix
that. OTOH, a future patch I have will require _resume to always be
present, so I'm inclined to just always put the code in. I'll come up
with a fix in any case.

johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2009-03-13 21:22:15

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v2] mac80211: resume properly, add suspend/resume test

On Fri, 2009-03-13 at 14:51 -0400, Bob Copeland wrote:

> > [ 289.753866]
> > [ 289.753869] =================================
> > [ 289.754164] [ INFO: inconsistent lock state ]
> > [ 289.754372] 2.6.29-rc7-wl-17155-g2f7ab92-dirty #29
> > [ 289.754598] ---------------------------------
> > [ 289.754805] inconsistent {in-softirq-W} -> {softirq-on-W} usage.
>
> [softirq-on-W stack looks like:
>
> sta_notify
> ath_node_detach
> ath_tx_node_cleanup
> spin_lock()
>
> vs the ath9k_tx_tasklet]
>
> > Looks like we need to disable BHs for the sta functions or something?

> Yup... or pm can take sta_lock with spin_lock_irqsave() like the other
> callers of sta_notify.

Yes, we probably should do that in mac80211 for consistency, I'll cook
up a patch tomorrow or so.

> Feel free to add my Acked-by on both the PM patches. I'd been using
> CONFIG_PM_DEBUG to simulate suspend/resume but since you have to
> do 'devices' anyway to get the desired effect, the debug file here
> is nicer.

PM_DEBUG works too, this just seemed a little more self-contained.

johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2009-03-13 12:16:22

by Helmut Schaa

[permalink] [raw]
Subject: Re: [PATCH v2] mac80211: resume properly, add suspend/resume test

Am Freitag, 13. M=E4rz 2009 schrieb Johannes Berg:
> On Fri, 2009-03-13 at 11:48 +0100, Johannes Berg wrote:
> > On Fri, 2009-03-13 at 11:44 +0100, Johannes Berg wrote:
> > > When mac80211 resumes, it currently doesn't reconfigure the inter=
faces
> > > entirely and also doesn't reconfigure BSS information -- fix this=
=2E
> > >=20
> > > Also, to be able to test this, add a debugfs file that just calls
> > > the suspend/resume code to see what happens when we go through th=
at,
> > > without needing the time-consuming suspend/resume cycle.
> > >=20
> > > Signed-off-by: Johannes Berg <[email protected]>
> > > ---
> > > v2: rebased on top of "stop queues across suspend/resume"
> > >=20
> > > zd1211rw currently doesn't survive this properly, I'm not sure wh=
y,
> > > but it keeps saying
> > > zd1211rw 1-1:1.0: error ioread32(CR_REG1): -11
> > >=20
> > > Please test with other hardware, we need to figure out whether or
> > > not something is missing in mac80211.
> >=20
> > ath9k survives ping -f without even blinking.
>=20
> So does iwlwifi.

Mine does not. I got:

mac80211-phy2: failed to set key (0, XX:XX:XX:XX:XX:XX) to hardware (-2=
2)

iwconfig still shows signal strength etc. but no packets are transmitte=
d
anymore.

Helmut

2009-03-13 12:15:45

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH v2] mac80211: resume properly, add suspend/resume test

On Fri, 2009-03-13 at 13:10 +0100, Helmut Schaa wrote:

> > So does iwlwifi.
>
> Mine does not. I got:
>
> mac80211-phy2: failed to set key (0, XX:XX:XX:XX:XX:XX) to hardware (-22)
>
> iwconfig still shows signal strength etc. but no packets are transmitted
> anymore.

Ah, I had an open AP.

This is due to iwlwifi still not being fixed to use the sta_notify
callback rather than the rate control algorithm to add stations to the
internal map.

johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part