2009-04-20 18:25:08

by Reinette Chatre

[permalink] [raw]
Subject: WARN on return of ieee80211_if_config

Hi Johannes,

In 2.6.30 we are starting to see people encountering a WARN when
resuming with HW rfkill enabled. The WARN is printed from
net/mac80211/pm.c:159 in 2.6.30, for current wireless-testing it is
net/mac80211/util.c:1050.

With that function resuming multiple interfaces I do not know if it will
make sense to return an error code if one fails. So, it seems that
__ieee80211_resume()/ieee80211_reconfig() will always return zero. Even
so, the case where ieee80211_if_config() fails needs to be
accommodated.

Would something like this make sense?

diff --git a/net/mac80211/pm.c b/net/mac80211/pm.c
index 0273023..46f2961 100644
--- a/net/mac80211/pm.c
+++ b/net/mac80211/pm.c
@@ -156,8 +156,11 @@ int __ieee80211_resume(struct ieee80211_hw *hw)
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);
+ if (ieee80211_if_config(sdata, changed))
+ printk(KERN_DEBUG "%s: failed to configure interface\n",
+ sdata->dev->name);
+ else
+ ieee80211_bss_info_change_notify(sdata, ~0);
break;
case NL80211_IFTYPE_WDS:
break;


Thank you

Reinette




2009-04-20 21:06:48

by Johannes Berg

[permalink] [raw]
Subject: Re: WARN on return of ieee80211_if_config

On Mon, 2009-04-20 at 13:58 -0700, reinette chatre wrote:

> > Or will this test:
> > if (!iwl_is_alive(priv))
> > return -EAGAIN;
> >
> > kick us out before even getting to the rfkill test?
>
> Yes ... and it is not immediately clear that it is caused by rfkill. The
> STATUS_ALIVE bit that is tested here is cleared when machine is
> suspended. When machine is resumed and HW rfkill is set then it will not
> come up fully, see __iwl_up() that returns early with success if rfkill
> is enabled. STATUS_ALIVE will only be set again after HW rfkill is
> disabled and we initialize ucode again.

Ok... Very confusing. Can you change the patch to have a comment that
this should also be changed back when rfkill is integrated with
mac80211? Maybe I'll even do that for .31.

Thanks,
Johannes


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

2009-04-20 20:52:13

by Reinette Chatre

[permalink] [raw]
Subject: Re: WARN on return of ieee80211_if_config

Hi Johannes,

On Mon, 2009-04-20 at 11:35 -0700, Johannes Berg wrote:
> Hi Reinette,
>
> > In 2.6.30 we are starting to see people encountering a WARN when
> > resuming with HW rfkill enabled. The WARN is printed from
> > net/mac80211/pm.c:159 in 2.6.30, for current wireless-testing it is
> > net/mac80211/util.c:1050.
>
> Hmm, ok, that makes sense.
>
> > With that function resuming multiple interfaces I do not know if it will
> > make sense to return an error code if one fails. So, it seems that
> > __ieee80211_resume()/ieee80211_reconfig() will always return zero. Even
> > so, the case where ieee80211_if_config() fails needs to be
> > accommodated.
> >
> > Would something like this make sense?
> >
> > diff --git a/net/mac80211/pm.c b/net/mac80211/pm.c
> > index 0273023..46f2961 100644
> > --- a/net/mac80211/pm.c
> > +++ b/net/mac80211/pm.c
> > @@ -156,8 +156,11 @@ int __ieee80211_resume(struct ieee80211_hw *hw)
> > 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);
> > + if (ieee80211_if_config(sdata, changed))
> > + printk(KERN_DEBUG "%s: failed to configure interface\n",
> > + sdata->dev->name);
> > + else
> > + ieee80211_bss_info_change_notify(sdata, ~0);
>
> Not sure. That doesn't seem to make sense anyway, since iwlwifi does
> this:
>
> if (iwl_is_rfkill(priv))
> goto done;
> ...
> done:
> ...
> return 0;
>
>
> Or will this test:
> if (!iwl_is_alive(priv))
> return -EAGAIN;
>
> kick us out before even getting to the rfkill test?

Yes ... and it is not immediately clear that it is caused by rfkill. The
STATUS_ALIVE bit that is tested here is cleared when machine is
suspended. When machine is resumed and HW rfkill is set then it will not
come up fully, see __iwl_up() that returns early with success if rfkill
is enabled. STATUS_ALIVE will only be set again after HW rfkill is
disabled and we initialize ucode again.

Reinette





2009-04-20 18:35:46

by Johannes Berg

[permalink] [raw]
Subject: Re: WARN on return of ieee80211_if_config

Hi Reinette,

> In 2.6.30 we are starting to see people encountering a WARN when
> resuming with HW rfkill enabled. The WARN is printed from
> net/mac80211/pm.c:159 in 2.6.30, for current wireless-testing it is
> net/mac80211/util.c:1050.

Hmm, ok, that makes sense.

> With that function resuming multiple interfaces I do not know if it will
> make sense to return an error code if one fails. So, it seems that
> __ieee80211_resume()/ieee80211_reconfig() will always return zero. Even
> so, the case where ieee80211_if_config() fails needs to be
> accommodated.
>
> Would something like this make sense?
>
> diff --git a/net/mac80211/pm.c b/net/mac80211/pm.c
> index 0273023..46f2961 100644
> --- a/net/mac80211/pm.c
> +++ b/net/mac80211/pm.c
> @@ -156,8 +156,11 @@ int __ieee80211_resume(struct ieee80211_hw *hw)
> 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);
> + if (ieee80211_if_config(sdata, changed))
> + printk(KERN_DEBUG "%s: failed to configure interface\n",
> + sdata->dev->name);
> + else
> + ieee80211_bss_info_change_notify(sdata, ~0);

Not sure. That doesn't seem to make sense anyway, since iwlwifi does
this:

if (iwl_is_rfkill(priv))
goto done;
...
done:
...
return 0;


Or will this test:
if (!iwl_is_alive(priv))
return -EAGAIN;

kick us out before even getting to the rfkill test?

johannes


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