* Fw: [PATCH] iwl3945: queue the right work if the scan needs to be aborted
@ 2010-10-05 6:43 Florian Mickler
[not found] ` <20101005084305.52a31ed4-mGsOIKOveelVRbCss4o9kg@public.gmane.org>
0 siblings, 1 reply; 13+ messages in thread
From: Florian Mickler @ 2010-10-05 6:43 UTC (permalink / raw)
To: stable-DgEjT+Ai2ygdnm+yROfE0A
Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Guy, Wey-Yi, Chatre, Reinette, Intel Linux Wireless,
John W. Linville, Berg, Johannes, Cahill, Ben M,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
commit e7ee762cf074b0fd8eec483d0cef8fdbf0d04b81
Begin forwarded message:
Date: Mon, 27 Sep 2010 13:11:54 -0700
From: "Guy, Wey-Yi" <wey-yi.w.guy-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
To: Florian Mickler <florian-sVu6HhrpSfRAfugRpC6u6w@public.gmane.org>
Cc: "linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" <linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
"Chatre, Reinette" <reinette.chatre-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>, Intel Linux Wireless
<ilw-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>, "John W. Linville" <linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org>,
"Berg, Johannes" <johannes.berg-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>, Zhu Yi <yi.zhu-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>,
"Cahill, Ben M" <ben.m.cahill-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>, "netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
<netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>, "linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org> Subject: Re: [PATCH] iwl3945: queue the
right work if the scan needs to be aborted
On Fri, 2010-09-24 at 09:22 -0700, Florian Mickler wrote:
> iwl3945's scan_completed calls into the mac80211 stack which triggers a
> warn on if there is no scan outstanding.
>
> This can be avoided by not calling scan_completed but abort_scan in
> iwl3945_request_scan in the done: branch of the function which is used
> as an error out.
>
> The done: branch seems to be an error-out branch, as, for example, if
> iwl_is_ready(priv) returns false the done: branch is executed.
>
> NOTE:
> I'm not familiar with the driver at all.
> I just quickly scanned as a reaction to
>
> https://bugzilla.kernel.org/show_bug.cgi?id=17722
>
> the users of scan_completed in the iwl3945 driver and noted the odd
> discrepancy between the comment above this instance and the comment in
> mac80211 scan_completed function.
> Signed-off-by: Florian Mickler <florian-sVu6HhrpSfRAfugRpC6u6w@public.gmane.org>
Acked-by: Wey-Yi Guy <wey-yi.w.guy-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
go into wireless-2.6 and stable only, scan fix already in
wireless-next-2.6
Thanks
Wey
> drivers/net/wireless/iwlwifi/iwl-agn-lib.c | 2 +-
> drivers/net/wireless/iwlwifi/iwl3945-base.c | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/iwlwifi/iwl-agn-lib.c b/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
> index 9dd9e64..8fd00a6 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
> +++ b/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
> @@ -1411,7 +1411,7 @@ void iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
> clear_bit(STATUS_SCAN_HW, &priv->status);
> clear_bit(STATUS_SCANNING, &priv->status);
> /* inform mac80211 scan aborted */
> - queue_work(priv->workqueue, &priv->scan_completed);
> + queue_work(priv->workqueue, &priv->abort_scan);
> }
>
> int iwlagn_manage_ibss_station(struct iwl_priv *priv,
> diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c b/drivers/net/wireless/iwlwifi/iwl3945-base.c
> index 59a308b..d31661c 100644
> --- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
> +++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
> @@ -3018,7 +3018,7 @@ void iwl3945_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
> clear_bit(STATUS_SCANNING, &priv->status);
>
> /* inform mac80211 scan aborted */
> - queue_work(priv->workqueue, &priv->scan_completed);
> + queue_work(priv->workqueue, &priv->abort_scan);
> }
>
> static void iwl3945_bg_restart(struct work_struct *data)
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: Fw: [PATCH] iwl3945: queue the right work if the scan needs to be aborted
[not found] ` <20101005084305.52a31ed4-mGsOIKOveelVRbCss4o9kg@public.gmane.org>
@ 2010-10-05 8:57 ` Stanislaw Gruszka
2010-10-05 10:12 ` Florian Mickler
[not found] ` <20101005085717.GA18012-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
0 siblings, 2 replies; 13+ messages in thread
From: Stanislaw Gruszka @ 2010-10-05 8:57 UTC (permalink / raw)
To: Florian Mickler
Cc: stable-DgEjT+Ai2ygdnm+yROfE0A,
linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Guy, Wey-Yi, Chatre, Reinette, Intel Linux Wireless,
John W. Linville, Berg, Johannes, Cahill, Ben M,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
On Tue, Oct 05, 2010 at 08:43:05AM +0200, Florian Mickler wrote:
> commit e7ee762cf074b0fd8eec483d0cef8fdbf0d04b81
>
>
> Begin forwarded message:
> On Fri, 2010-09-24 at 09:22 -0700, Florian Mickler wrote:
> > iwl3945's scan_completed calls into the mac80211 stack which triggers a
> > warn on if there is no scan outstanding.
> >
> > This can be avoided by not calling scan_completed but abort_scan in
> > iwl3945_request_scan in the done: branch of the function which is used
> > as an error out.
> >
> > The done: branch seems to be an error-out branch, as, for example, if
> > iwl_is_ready(priv) returns false the done: branch is executed.
> >
> > NOTE:
> > I'm not familiar with the driver at all.
> > I just quickly scanned as a reaction to
> >
> > https://bugzilla.kernel.org/show_bug.cgi?id=17722
> >
> > the users of scan_completed in the iwl3945 driver and noted the odd
> > discrepancy between the comment above this instance and the comment in
> > mac80211 scan_completed function.
> > Signed-off-by: Florian Mickler <florian-sVu6HhrpSfRAfugRpC6u6w@public.gmane.org>
> Acked-by: Wey-Yi Guy <wey-yi.w.guy-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > ---
> go into wireless-2.6 and stable only, scan fix already in
> wireless-next-2.6
>
> Thanks
> Wey
>
> > drivers/net/wireless/iwlwifi/iwl-agn-lib.c | 2 +-
> > drivers/net/wireless/iwlwifi/iwl3945-base.c | 2 +-
> > 2 files changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/wireless/iwlwifi/iwl-agn-lib.c b/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
> > index 9dd9e64..8fd00a6 100644
> > --- a/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
> > +++ b/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
> > @@ -1411,7 +1411,7 @@ void iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
> > clear_bit(STATUS_SCAN_HW, &priv->status);
> > clear_bit(STATUS_SCANNING, &priv->status);
> > /* inform mac80211 scan aborted */
> > - queue_work(priv->workqueue, &priv->scan_completed);
> > + queue_work(priv->workqueue, &priv->abort_scan);
Unfortunately this patch is not right thing to do. If you look at
abort_scan work, it do nothing if STATUS_SCAN_ABORTING bit is not set.
That's wrong because we have to complete scan (with abort == true).
If STATUS_SCAN_ABORTING will be set, abort_work will send scan cancel
commands to hardware what is wrong if scan was not started yet.
What we can eventually do, except apply iwl-scan rewrite from
wireless-testing, is something like that:
iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
clear_bit(STATUS_SCAN_HW, &priv->status);
clear_bit(STATUS_SCANNING, &priv->status);
/* inform mac80211 scan aborted */
set_bit(STATUS_SCAN_ABORTING, &priv->status);
queue_work(priv->workqueue, &priv->scan_completed);
ieee80211_scan_completed
if (!internal) {
bool aborted = test_bit(STATUS_SCAN_ABORTING, &priv->status);
ieee80211_scan_completed(priv->hw, aborted);
}
However, I do not think we should go with that to -stable (below
2.6.36). IIRC warnings showed up in current 2.6.36-rc, because of
some other changes in the code.
Stanislaw
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] iwl3945: queue the right work if the scan needs to be aborted
2010-10-05 8:57 ` Stanislaw Gruszka
@ 2010-10-05 10:12 ` Florian Mickler
[not found] ` <20101005121242.79cdafc2-mGsOIKOveelVRbCss4o9kg@public.gmane.org>
[not found] ` <20101005085717.GA18012-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
1 sibling, 1 reply; 13+ messages in thread
From: Florian Mickler @ 2010-10-05 10:12 UTC (permalink / raw)
To: Stanislaw Gruszka
Cc: stable, linux-wireless@vger.kernel.org, Guy, Wey-Yi,
Chatre, Reinette, Intel Linux Wireless, John W. Linville,
Berg, Johannes, Cahill, Ben M, netdev@vger.kernel.org,
linux-kernel
On Tue, 5 Oct 2010 10:57:17 +0200
Stanislaw Gruszka <sgruszka@redhat.com> wrote:
> On Tue, Oct 05, 2010 at 08:43:05AM +0200, Florian Mickler wrote:
> > commit e7ee762cf074b0fd8eec483d0cef8fdbf0d04b81
> >
> >
> > Begin forwarded message:
> > On Fri, 2010-09-24 at 09:22 -0700, Florian Mickler wrote:
> > > iwl3945's scan_completed calls into the mac80211 stack which triggers a
> > > warn on if there is no scan outstanding.
> > >
> > > This can be avoided by not calling scan_completed but abort_scan in
> > > iwl3945_request_scan in the done: branch of the function which is used
> > > as an error out.
> > >
> > > The done: branch seems to be an error-out branch, as, for example, if
> > > iwl_is_ready(priv) returns false the done: branch is executed.
> > >
> > > NOTE:
> > > I'm not familiar with the driver at all.
> > > I just quickly scanned as a reaction to
> > >
> > > https://bugzilla.kernel.org/show_bug.cgi?id=17722
> > >
> > > the users of scan_completed in the iwl3945 driver and noted the odd
> > > discrepancy between the comment above this instance and the comment in
> > > mac80211 scan_completed function.
> > > Signed-off-by: Florian Mickler <florian@mickler.org>
> > Acked-by: Wey-Yi Guy <wey-yi.w.guy@intel.com>
> > > ---
> > go into wireless-2.6 and stable only, scan fix already in
> > wireless-next-2.6
>
> >
> > Thanks
> > Wey
> >
> > > drivers/net/wireless/iwlwifi/iwl-agn-lib.c | 2 +-
> > > drivers/net/wireless/iwlwifi/iwl3945-base.c | 2 +-
> > > 2 files changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/net/wireless/iwlwifi/iwl-agn-lib.c b/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
> > > index 9dd9e64..8fd00a6 100644
> > > --- a/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
> > > +++ b/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
> > > @@ -1411,7 +1411,7 @@ void iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
> > > clear_bit(STATUS_SCAN_HW, &priv->status);
> > > clear_bit(STATUS_SCANNING, &priv->status);
> > > /* inform mac80211 scan aborted */
> > > - queue_work(priv->workqueue, &priv->scan_completed);
> > > + queue_work(priv->workqueue, &priv->abort_scan);
>
> Unfortunately this patch is not right thing to do. If you look at
> abort_scan work, it do nothing if STATUS_SCAN_ABORTING bit is not set.
> That's wrong because we have to complete scan (with abort == true).
> If STATUS_SCAN_ABORTING will be set, abort_work will send scan cancel
> commands to hardware what is wrong if scan was not started yet.
>
> What we can eventually do, except apply iwl-scan rewrite from
> wireless-testing, is something like that:
>
> iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
>
> clear_bit(STATUS_SCAN_HW, &priv->status);
> clear_bit(STATUS_SCANNING, &priv->status);
> /* inform mac80211 scan aborted */
> set_bit(STATUS_SCAN_ABORTING, &priv->status);
> queue_work(priv->workqueue, &priv->scan_completed);
>
> ieee80211_scan_completed
>
> if (!internal) {
> bool aborted = test_bit(STATUS_SCAN_ABORTING, &priv->status);
> ieee80211_scan_completed(priv->hw, aborted);
>
> }
>
> However, I do not think we should go with that to -stable (below
> 2.6.36). IIRC warnings showed up in current 2.6.36-rc, because of
> some other changes in the code.
>
> Stanislaw
Thx for looking at this. I suspect you know the code better than I do.. what is about the
first jump to :done in iwlagn_request_scan()
if (!iwl_is_ready(priv)) {
IWL_WARN(priv, "request scan called when driver not ready.\n");
goto done;
}
Does abort_scan need to do anything in that case?
I can't see where we set up the hardware for scanning in that case.
(I've gone through the codepath coming from the mac80211 hw_scan)
Regards,
Flo
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] iwl3945: queue the right work if the scan needs to be aborted
[not found] ` <20101005121242.79cdafc2-mGsOIKOveelVRbCss4o9kg@public.gmane.org>
@ 2010-10-05 10:43 ` Stanislaw Gruszka
[not found] ` <20101005104357.GB18833-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
0 siblings, 1 reply; 13+ messages in thread
From: Stanislaw Gruszka @ 2010-10-05 10:43 UTC (permalink / raw)
To: Florian Mickler
Cc: stable-DgEjT+Ai2ygdnm+yROfE0A,
linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Guy, Wey-Yi, Chatre, Reinette, Intel Linux Wireless,
John W. Linville, Berg, Johannes, Cahill, Ben M,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
On Tue, Oct 05, 2010 at 12:12:42PM +0200, Florian Mickler wrote:
> > Unfortunately this patch is not right thing to do. If you look at
> > abort_scan work, it do nothing if STATUS_SCAN_ABORTING bit is not set.
> > That's wrong because we have to complete scan (with abort == true).
> > If STATUS_SCAN_ABORTING will be set, abort_work will send scan cancel
> > commands to hardware what is wrong if scan was not started yet.
> >
> > What we can eventually do, except apply iwl-scan rewrite from
> > wireless-testing, is something like that:
> >
> > iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
> >
> > clear_bit(STATUS_SCAN_HW, &priv->status);
> > clear_bit(STATUS_SCANNING, &priv->status);
> > /* inform mac80211 scan aborted */
> > set_bit(STATUS_SCAN_ABORTING, &priv->status);
> > queue_work(priv->workqueue, &priv->scan_completed);
> >
> > ieee80211_scan_completed
> >
> > if (!internal) {
> > bool aborted = test_bit(STATUS_SCAN_ABORTING, &priv->status);
> > ieee80211_scan_completed(priv->hw, aborted);
> >
> > }
> >
> > However, I do not think we should go with that to -stable (below
> > 2.6.36). IIRC warnings showed up in current 2.6.36-rc, because of
> > some other changes in the code.
> >
> > Stanislaw
>
> Thx for looking at this. I suspect you know the code better than I do.. what is about the
> first jump to :done in iwlagn_request_scan()
>
> if (!iwl_is_ready(priv)) {
> IWL_WARN(priv, "request scan called when driver not ready.\n");
> goto done;
> }
>
> Does abort_scan need to do anything in that case?
Yes, because we do not return value to mac80211 about .hw_scan failure,
we always return 0, what is completely wrong. And yes, return error can
be done instead of queueing scan_completed to improve situation - this is
one of the thing, we do in wireless-testing.
> I can't see where we set up the hardware for scanning in that case.
> (I've gone through the codepath coming from the mac80211 hw_scan)
.hw_scan = iwl_mac_hw_scan -> iwl_scan_initiate ->
"priv->cfg->ops->utils->request_scan" = {iwl3945,iwlang}_request_scan
Stanislaw
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH] iwlwifi: fix iwlwifi scanning corner cases
[not found] ` <20101005085717.GA18012-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
@ 2010-10-05 22:21 ` Florian Mickler
[not found] ` <1286317292-10679-1-git-send-email-florian-sVu6HhrpSfRAfugRpC6u6w@public.gmane.org>
0 siblings, 1 reply; 13+ messages in thread
From: Florian Mickler @ 2010-10-05 22:21 UTC (permalink / raw)
To: linville-2XuSBdqkA4R54TAoqtyWWQ
Cc: stable-DgEjT+Ai2ygdnm+yROfE0A,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
wey-yi.w.guy-ral2JQCrhuEAvxtiuMwx3w,
reinette.chatre-ral2JQCrhuEAvxtiuMwx3w,
ilw-VuQAYsv1563Yd54FQh9/CA, johannes.berg-ral2JQCrhuEAvxtiuMwx3w,
ben.m.cahill-ral2JQCrhuEAvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
sgruszka-H+wXaHxf7aLQT0dZR+AlfA, Florian Mickler
Stanislaw Gruszka pointed out, that commit
"iwl3945: queue the right work if the scan needs to be aborted"
has an awkward definition of "right". Specifically the abort_scan work
doesn't notify the generic wireless stack that the scan was aborted.
In order to get rid of the warning in bug
https://bugzilla.kernel.org/show_bug.cgi?id=17722
we inform ieee80211_scan_completed that we are aborting
the scan by setting the apropriate status bit in request_scan and
pass it into ieee80211_scan_completed.
Signed-off-by: Florian Mickler <florian-sVu6HhrpSfRAfugRpC6u6w@public.gmane.org>
---
Hi John!
Here is the fix that Stanislaw described.
(Yes, it is in a brown paper bag.) Please wait for him to review this.
Another option would be to just revert my previous patch and live with the
warning until the scanning rework hit's mainline.
Regards,
Flo
drivers/net/wireless/iwlwifi/iwl-agn-lib.c | 4 +++-
drivers/net/wireless/iwlwifi/iwl-scan.c | 6 ++++--
drivers/net/wireless/iwlwifi/iwl3945-base.c | 3 ++-
3 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/iwlwifi/iwl-agn-lib.c b/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
index 8fd00a6..2d26767 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
+++ b/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
@@ -1410,8 +1410,10 @@ void iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
*/
clear_bit(STATUS_SCAN_HW, &priv->status);
clear_bit(STATUS_SCANNING, &priv->status);
+
/* inform mac80211 scan aborted */
- queue_work(priv->workqueue, &priv->abort_scan);
+ set_bit(STATUS_SCAN_ABORTING, &priv->status);
+ queue_work(priv->workqueue, &priv->scan_completed);
}
int iwlagn_manage_ibss_station(struct iwl_priv *priv,
diff --git a/drivers/net/wireless/iwlwifi/iwl-scan.c b/drivers/net/wireless/iwlwifi/iwl-scan.c
index a4b3663..fedf384 100644
--- a/drivers/net/wireless/iwlwifi/iwl-scan.c
+++ b/drivers/net/wireless/iwlwifi/iwl-scan.c
@@ -552,8 +552,10 @@ void iwl_bg_scan_completed(struct work_struct *work)
* into driver again into functions that will attempt to take
* mutex.
*/
- if (!internal)
- ieee80211_scan_completed(priv->hw, false);
+ if (!internal) {
+ bool aborted = test_bit(STATUS_SCAN_ABORTING, &priv->status);
+ ieee80211_scan_completed(priv->hw, aborted);
+ }
}
EXPORT_SYMBOL(iwl_bg_scan_completed);
diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c b/drivers/net/wireless/iwlwifi/iwl3945-base.c
index d31661c..da10588 100644
--- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
+++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
@@ -3018,7 +3018,8 @@ void iwl3945_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
clear_bit(STATUS_SCANNING, &priv->status);
/* inform mac80211 scan aborted */
- queue_work(priv->workqueue, &priv->abort_scan);
+ set_bit(STATUS_SCAN_ABORTING, &priv->status);
+ queue_work(priv->workqueue, &priv->scan_completed);
}
static void iwl3945_bg_restart(struct work_struct *data)
--
1.7.3
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH] iwl3945: queue the right work if the scan needs to be aborted
[not found] ` <20101005104357.GB18833-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
@ 2010-10-05 22:29 ` Florian Mickler
0 siblings, 0 replies; 13+ messages in thread
From: Florian Mickler @ 2010-10-05 22:29 UTC (permalink / raw)
To: stable-DgEjT+Ai2ygdnm+yROfE0A
Cc: Stanislaw Gruszka,
linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Guy, Wey-Yi, Chatre, Reinette, Intel Linux Wireless,
John W. Linville, Berg, Johannes, Cahill, Ben M,
netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
On Tue, 5 Oct 2010 12:43:58 +0200
Stanislaw Gruszka <sgruszka-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> > >
> > > However, I do not think we should go with that to -stable (below
> > > 2.6.36). IIRC warnings showed up in current 2.6.36-rc, because of
> > > some other changes in the code.
> > >
> > > Stanislaw
Hello stable,
scrap this. I handed out a brown paper bag bug fix and if that doesn't
get into 2.6.36, it probably should go into 2.6.36.y. But let's see...
Regards,
Flo
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] iwlwifi: fix iwlwifi scanning corner cases
[not found] ` <1286317292-10679-1-git-send-email-florian-sVu6HhrpSfRAfugRpC6u6w@public.gmane.org>
@ 2010-10-06 9:02 ` Stanislaw Gruszka
[not found] ` <20101006090228.GA2523-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
0 siblings, 1 reply; 13+ messages in thread
From: Stanislaw Gruszka @ 2010-10-06 9:02 UTC (permalink / raw)
To: Florian Mickler
Cc: linville-2XuSBdqkA4R54TAoqtyWWQ, stable-DgEjT+Ai2ygdnm+yROfE0A,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
wey-yi.w.guy-ral2JQCrhuEAvxtiuMwx3w,
reinette.chatre-ral2JQCrhuEAvxtiuMwx3w,
ilw-VuQAYsv1563Yd54FQh9/CA, johannes.berg-ral2JQCrhuEAvxtiuMwx3w,
ben.m.cahill-ral2JQCrhuEAvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Hi Florian
On Wed, Oct 06, 2010 at 12:21:32AM +0200, Florian Mickler wrote:
> Stanislaw Gruszka pointed out, that commit
> "iwl3945: queue the right work if the scan needs to be aborted"
> has an awkward definition of "right". Specifically the abort_scan work
> doesn't notify the generic wireless stack that the scan was aborted.
>
> In order to get rid of the warning in bug
> https://bugzilla.kernel.org/show_bug.cgi?id=17722
> we inform ieee80211_scan_completed that we are aborting
> the scan by setting the apropriate status bit in request_scan and
> pass it into ieee80211_scan_completed.
>
> Signed-off-by: Florian Mickler <florian-sVu6HhrpSfRAfugRpC6u6w@public.gmane.org>
> ---
>
> Hi John!
>
> Here is the fix that Stanislaw described.
> (Yes, it is in a brown paper bag.) Please wait for him to review this.
>
> Another option would be to just revert my previous patch and live with the
> warning until the scanning rework hit's mainline.
This patch is not so bad, but better would be just return value from
{iwlagn,iwl3945}_request_scan instead of queuing anything. I will
prepare patch(s) today or tomorrow.
Stanislaw
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH wireless-2.6 or stable] iwlwifi: return error when fail to start scanning
[not found] ` <20101006090228.GA2523-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
@ 2010-10-06 16:04 ` Stanislaw Gruszka
2010-10-06 16:12 ` Guy, Wey-Yi
[not found] ` <20101006160434.GB24581-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
0 siblings, 2 replies; 13+ messages in thread
From: Stanislaw Gruszka @ 2010-10-06 16:04 UTC (permalink / raw)
To: Florian Mickler
Cc: linville-2XuSBdqkA4R54TAoqtyWWQ, stable-DgEjT+Ai2ygdnm+yROfE0A,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
wey-yi.w.guy-ral2JQCrhuEAvxtiuMwx3w,
reinette.chatre-ral2JQCrhuEAvxtiuMwx3w,
ilw-VuQAYsv1563Yd54FQh9/CA, johannes.berg-ral2JQCrhuEAvxtiuMwx3w,
ben.m.cahill-ral2JQCrhuEAvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
This is stable friendly backport of wireless-testing commit
3eecce527c7434dfd16517ce08b49632c8a26717 "iwlwifi: unify scan start
checks". Wireless-testing version include also a lot of cleanups.
On error case in {iwl3945,iwlagn}_request_scan we queue abort_scan work,
which in a fact do nothing, so we never complete the scan. Thats gives
"WARNING: at net/wireless/core.c:614 wdev_cleanup_work" and stopped
network connection until reload iwlwifi modules. Return of -EIO, and
stopping queuing any work is a fix.
Note there are still many cases when we can get:
WARNING: at net/wireless/core.c:614 wdev_cleanup_work
WARNING: at net/mac80211/scan.c:266 ieee80211_scan_completed
WARNING: at net/mac80211/scan.c:269 ieee80211_scan_complete
which are not fixed. Unfortunately does not exist single, small fix
for that problems, and we probably will see some more bug reports
with scan warnings. As solution we rewrite iwl-scan.c code to avoid
all possible race conditions. That quite big patch set is queued
for 2.6.37 .
Signed-off-by: Stanislaw Gruszka <sgruszka-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
---
Patch is intended for wireless-2.6, or in case when it does not
make 2.6.36 release, for -stable 2.6.36.y
drivers/net/wireless/iwlwifi/iwl-3945.h | 2 +-
drivers/net/wireless/iwlwifi/iwl-agn-lib.c | 18 ++++++------------
drivers/net/wireless/iwlwifi/iwl-agn.h | 2 +-
drivers/net/wireless/iwlwifi/iwl-core.h | 2 +-
drivers/net/wireless/iwlwifi/iwl-scan.c | 14 +++++++++++---
drivers/net/wireless/iwlwifi/iwl3945-base.c | 19 ++++++-------------
6 files changed, 26 insertions(+), 31 deletions(-)
diff --git a/drivers/net/wireless/iwlwifi/iwl-3945.h b/drivers/net/wireless/iwlwifi/iwl-3945.h
index bb2aeeb..98509c5 100644
--- a/drivers/net/wireless/iwlwifi/iwl-3945.h
+++ b/drivers/net/wireless/iwlwifi/iwl-3945.h
@@ -295,7 +295,7 @@ extern const struct iwl_channel_info *iwl3945_get_channel_info(
extern int iwl3945_rs_next_rate(struct iwl_priv *priv, int rate);
/* scanning */
-void iwl3945_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif);
+int iwl3945_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif);
/* Requires full declaration of iwl_priv before including */
#include "iwl-io.h"
diff --git a/drivers/net/wireless/iwlwifi/iwl-agn-lib.c b/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
index 8fd00a6..2be8faa 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
+++ b/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
@@ -1147,7 +1147,7 @@ static int iwl_get_channels_for_scan(struct iwl_priv *priv,
return added;
}
-void iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
+int iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
{
struct iwl_host_cmd cmd = {
.id = REPLY_SCAN_CMD,
@@ -1394,24 +1394,18 @@ void iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
scan->len = cpu_to_le16(cmd.len);
set_bit(STATUS_SCAN_HW, &priv->status);
- if (iwl_send_cmd_sync(priv, &cmd))
+ if (iwl_send_cmd_sync(priv, &cmd)) {
+ clear_bit(STATUS_SCAN_HW, &priv->status);
goto done;
+ }
queue_delayed_work(priv->workqueue, &priv->scan_check,
IWL_SCAN_CHECK_WATCHDOG);
- return;
+ return 0;
done:
- /* Cannot perform scan. Make sure we clear scanning
- * bits from status so next scan request can be performed.
- * If we don't clear scanning status bit here all next scan
- * will fail
- */
- clear_bit(STATUS_SCAN_HW, &priv->status);
- clear_bit(STATUS_SCANNING, &priv->status);
- /* inform mac80211 scan aborted */
- queue_work(priv->workqueue, &priv->abort_scan);
+ return -EIO;
}
int iwlagn_manage_ibss_station(struct iwl_priv *priv,
diff --git a/drivers/net/wireless/iwlwifi/iwl-agn.h b/drivers/net/wireless/iwlwifi/iwl-agn.h
index cc6464d..015da26 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn.h
+++ b/drivers/net/wireless/iwlwifi/iwl-agn.h
@@ -216,7 +216,7 @@ void iwl_reply_statistics(struct iwl_priv *priv,
struct iwl_rx_mem_buffer *rxb);
/* scan */
-void iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif);
+int iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif);
/* station mgmt */
int iwlagn_manage_ibss_station(struct iwl_priv *priv,
diff --git a/drivers/net/wireless/iwlwifi/iwl-core.h b/drivers/net/wireless/iwlwifi/iwl-core.h
index 5e6ee3d..110de0f 100644
--- a/drivers/net/wireless/iwlwifi/iwl-core.h
+++ b/drivers/net/wireless/iwlwifi/iwl-core.h
@@ -109,7 +109,7 @@ struct iwl_hcmd_utils_ops {
__le16 fc, __le32 *tx_flags);
int (*calc_rssi)(struct iwl_priv *priv,
struct iwl_rx_phy_res *rx_resp);
- void (*request_scan)(struct iwl_priv *priv, struct ieee80211_vif *vif);
+ int (*request_scan)(struct iwl_priv *priv, struct ieee80211_vif *vif);
};
struct iwl_apm_ops {
diff --git a/drivers/net/wireless/iwlwifi/iwl-scan.c b/drivers/net/wireless/iwlwifi/iwl-scan.c
index a4b3663..290140a 100644
--- a/drivers/net/wireless/iwlwifi/iwl-scan.c
+++ b/drivers/net/wireless/iwlwifi/iwl-scan.c
@@ -298,6 +298,7 @@ EXPORT_SYMBOL(iwl_init_scan_params);
static int iwl_scan_initiate(struct iwl_priv *priv, struct ieee80211_vif *vif)
{
+ int ret;
lockdep_assert_held(&priv->mutex);
IWL_DEBUG_INFO(priv, "Starting scan...\n");
@@ -308,9 +309,11 @@ static int iwl_scan_initiate(struct iwl_priv *priv, struct ieee80211_vif *vif)
if (WARN_ON(!priv->cfg->ops->utils->request_scan))
return -EOPNOTSUPP;
- priv->cfg->ops->utils->request_scan(priv, vif);
+ ret = priv->cfg->ops->utils->request_scan(priv, vif);
+ if (ret)
+ clear_bit(STATUS_SCANNING, &priv->status);
+ return ret;
- return 0;
}
int iwl_mac_hw_scan(struct ieee80211_hw *hw,
@@ -380,6 +383,7 @@ void iwl_internal_short_hw_scan(struct iwl_priv *priv)
void iwl_bg_start_internal_scan(struct work_struct *work)
{
+ int ret;
struct iwl_priv *priv =
container_of(work, struct iwl_priv, start_internal_scan);
@@ -414,7 +418,11 @@ void iwl_bg_start_internal_scan(struct work_struct *work)
if (WARN_ON(!priv->cfg->ops->utils->request_scan))
goto unlock;
- priv->cfg->ops->utils->request_scan(priv, NULL);
+ ret = priv->cfg->ops->utils->request_scan(priv, NULL);
+ if (ret) {
+ clear_bit(STATUS_SCANNING, &priv->status);
+ priv->is_internal_short_scan = false;
+ }
unlock:
mutex_unlock(&priv->mutex);
}
diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c b/drivers/net/wireless/iwlwifi/iwl3945-base.c
index d31661c..fc82da0 100644
--- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
+++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
@@ -2799,7 +2799,7 @@ static void iwl3945_rfkill_poll(struct work_struct *data)
}
-void iwl3945_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
+int iwl3945_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
{
struct iwl_host_cmd cmd = {
.id = REPLY_SCAN_CMD,
@@ -3000,25 +3000,18 @@ void iwl3945_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
scan->len = cpu_to_le16(cmd.len);
set_bit(STATUS_SCAN_HW, &priv->status);
- if (iwl_send_cmd_sync(priv, &cmd))
+ if (iwl_send_cmd_sync(priv, &cmd)) {
+ clear_bit(STATUS_SCAN_HW, &priv->status);
goto done;
+ }
queue_delayed_work(priv->workqueue, &priv->scan_check,
IWL_SCAN_CHECK_WATCHDOG);
- return;
+ return 0;
done:
- /* can not perform scan make sure we clear scanning
- * bits from status so next scan request can be performed.
- * if we dont clear scanning status bit here all next scan
- * will fail
- */
- clear_bit(STATUS_SCAN_HW, &priv->status);
- clear_bit(STATUS_SCANNING, &priv->status);
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH wireless-2.6 or stable] iwlwifi: return error when fail to start scanning
2010-10-06 16:04 ` [PATCH wireless-2.6 or stable] iwlwifi: return error when fail to start scanning Stanislaw Gruszka
@ 2010-10-06 16:12 ` Guy, Wey-Yi
2010-10-06 17:32 ` Florian Mickler
[not found] ` <20101006160434.GB24581-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
1 sibling, 1 reply; 13+ messages in thread
From: Guy, Wey-Yi @ 2010-10-06 16:12 UTC (permalink / raw)
To: Stanislaw Gruszka
Cc: Florian Mickler, linville@tuxdriver.com, stable@kernel.org,
linux-wireless@vger.kernel.org, Chatre, Reinette,
ilw@linux.intel.com, Berg, Johannes, Cahill, Ben M,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
Hi Gruszka,
On Wed, 2010-10-06 at 09:04 -0700, Stanislaw Gruszka wrote:
> This is stable friendly backport of wireless-testing commit
> 3eecce527c7434dfd16517ce08b49632c8a26717 "iwlwifi: unify scan start
> checks". Wireless-testing version include also a lot of cleanups.
>
> On error case in {iwl3945,iwlagn}_request_scan we queue abort_scan work,
> which in a fact do nothing, so we never complete the scan. Thats gives
> "WARNING: at net/wireless/core.c:614 wdev_cleanup_work" and stopped
> network connection until reload iwlwifi modules. Return of -EIO, and
> stopping queuing any work is a fix.
>
> Note there are still many cases when we can get:
>
> WARNING: at net/wireless/core.c:614 wdev_cleanup_work
> WARNING: at net/mac80211/scan.c:266 ieee80211_scan_completed
> WARNING: at net/mac80211/scan.c:269 ieee80211_scan_complete
>
> which are not fixed. Unfortunately does not exist single, small fix
> for that problems, and we probably will see some more bug reports
> with scan warnings. As solution we rewrite iwl-scan.c code to avoid
> all possible race conditions. That quite big patch set is queued
> for 2.6.37 .
>
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> ---
> Patch is intended for wireless-2.6, or in case when it does not
> make 2.6.36 release, for -stable 2.6.36.y
>
Is this the replacement for Mickler's patch?
Thanks
Wey
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH wireless-2.6 or stable] iwlwifi: return error when fail to start scanning
2010-10-06 16:12 ` Guy, Wey-Yi
@ 2010-10-06 17:32 ` Florian Mickler
0 siblings, 0 replies; 13+ messages in thread
From: Florian Mickler @ 2010-10-06 17:32 UTC (permalink / raw)
To: Guy, Wey-Yi
Cc: Stanislaw Gruszka, linville@tuxdriver.com, stable@kernel.org,
linux-wireless@vger.kernel.org, Chatre, Reinette,
ilw@linux.intel.com, Berg, Johannes, Cahill, Ben M,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
On Wed, 06 Oct 2010 09:12:58 -0700
"Guy, Wey-Yi" <wey-yi.w.guy@intel.com> wrote:
> Hi Gruszka,
>
> On Wed, 2010-10-06 at 09:04 -0700, Stanislaw Gruszka wrote:
> > This is stable friendly backport of wireless-testing commit
> > 3eecce527c7434dfd16517ce08b49632c8a26717 "iwlwifi: unify scan start
> > checks". Wireless-testing version include also a lot of cleanups.
> >
> > On error case in {iwl3945,iwlagn}_request_scan we queue abort_scan work,
> > which in a fact do nothing, so we never complete the scan. Thats gives
> > "WARNING: at net/wireless/core.c:614 wdev_cleanup_work" and stopped
> > network connection until reload iwlwifi modules. Return of -EIO, and
> > stopping queuing any work is a fix.
> >
> > Note there are still many cases when we can get:
> >
> > WARNING: at net/wireless/core.c:614 wdev_cleanup_work
> > WARNING: at net/mac80211/scan.c:266 ieee80211_scan_completed
> > WARNING: at net/mac80211/scan.c:269 ieee80211_scan_complete
> >
> > which are not fixed. Unfortunately does not exist single, small fix
> > for that problems, and we probably will see some more bug reports
> > with scan warnings. As solution we rewrite iwl-scan.c code to avoid
> > all possible race conditions. That quite big patch set is queued
> > for 2.6.37 .
> >
> > Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> > ---
> > Patch is intended for wireless-2.6, or in case when it does not
> > make 2.6.36 release, for -stable 2.6.36.y
> >
>
> Is this the replacement for Mickler's patch?
>
> Thanks
> Wey
>
This is on top of current mainline, as far as I see. (And it replaces
the patch I sent few hours ago)
Regards,
Flo
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH wireless-2.6 or stable] iwlwifi: return error when fail to start scanning
[not found] ` <20101006160434.GB24581-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
@ 2010-10-06 17:45 ` Florian Mickler
2010-10-06 17:48 ` Guy, Wey-Yi
2010-10-06 20:01 ` John W. Linville
1 sibling, 1 reply; 13+ messages in thread
From: Florian Mickler @ 2010-10-06 17:45 UTC (permalink / raw)
To: Stanislaw Gruszka
Cc: linville-2XuSBdqkA4R54TAoqtyWWQ, stable-DgEjT+Ai2ygdnm+yROfE0A,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
wey-yi.w.guy-ral2JQCrhuEAvxtiuMwx3w,
reinette.chatre-ral2JQCrhuEAvxtiuMwx3w,
ilw-VuQAYsv1563Yd54FQh9/CA, johannes.berg-ral2JQCrhuEAvxtiuMwx3w,
ben.m.cahill-ral2JQCrhuEAvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Hi Stanislaw!
On Wed, 6 Oct 2010 18:04:35 +0200
Stanislaw Gruszka <sgruszka-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> This is stable friendly backport of wireless-testing commit
> 3eecce527c7434dfd16517ce08b49632c8a26717 "iwlwifi: unify scan start
> checks". Wireless-testing version include also a lot of cleanups.
>
> On error case in {iwl3945,iwlagn}_request_scan we queue abort_scan work,
> which in a fact do nothing, so we never complete the scan. Thats gives
> "WARNING: at net/wireless/core.c:614 wdev_cleanup_work" and stopped
> network connection until reload iwlwifi modules. Return of -EIO, and
> stopping queuing any work is a fix.
>
> Note there are still many cases when we can get:
>
> WARNING: at net/wireless/core.c:614 wdev_cleanup_work
> WARNING: at net/mac80211/scan.c:266 ieee80211_scan_completed
> WARNING: at net/mac80211/scan.c:269 ieee80211_scan_complete
>
> which are not fixed. Unfortunately does not exist single, small fix
> for that problems, and we probably will see some more bug reports
> with scan warnings. As solution we rewrite iwl-scan.c code to avoid
> all possible race conditions. That quite big patch set is queued
> for 2.6.37 .
>
> Signed-off-by: Stanislaw Gruszka <sgruszka-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
Not that it matters much, but I've looked through it and couldn't find
anything wrong with it.
Reviewed-by: Florian Mickler <florian-sVu6HhrpSfRAfugRpC6u6w@public.gmane.org>
Regards,
Flo
> Patch is intended for wireless-2.6, or in case when it does not
> make 2.6.36 release, for -stable 2.6.36.y
>
> drivers/net/wireless/iwlwifi/iwl-3945.h | 2 +-
> drivers/net/wireless/iwlwifi/iwl-agn-lib.c | 18 ++++++------------
> drivers/net/wireless/iwlwifi/iwl-agn.h | 2 +-
> drivers/net/wireless/iwlwifi/iwl-core.h | 2 +-
> drivers/net/wireless/iwlwifi/iwl-scan.c | 14 +++++++++++---
> drivers/net/wireless/iwlwifi/iwl3945-base.c | 19 ++++++-------------
> 6 files changed, 26 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/net/wireless/iwlwifi/iwl-3945.h b/drivers/net/wireless/iwlwifi/iwl-3945.h
> index bb2aeeb..98509c5 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-3945.h
> +++ b/drivers/net/wireless/iwlwifi/iwl-3945.h
> @@ -295,7 +295,7 @@ extern const struct iwl_channel_info *iwl3945_get_channel_info(
> extern int iwl3945_rs_next_rate(struct iwl_priv *priv, int rate);
>
> /* scanning */
> -void iwl3945_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif);
> +int iwl3945_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif);
>
> /* Requires full declaration of iwl_priv before including */
> #include "iwl-io.h"
> diff --git a/drivers/net/wireless/iwlwifi/iwl-agn-lib.c b/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
> index 8fd00a6..2be8faa 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
> +++ b/drivers/net/wireless/iwlwifi/iwl-agn-lib.c
> @@ -1147,7 +1147,7 @@ static int iwl_get_channels_for_scan(struct iwl_priv *priv,
> return added;
> }
>
> -void iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
> +int iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
> {
> struct iwl_host_cmd cmd = {
> .id = REPLY_SCAN_CMD,
> @@ -1394,24 +1394,18 @@ void iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
> scan->len = cpu_to_le16(cmd.len);
>
> set_bit(STATUS_SCAN_HW, &priv->status);
> - if (iwl_send_cmd_sync(priv, &cmd))
> + if (iwl_send_cmd_sync(priv, &cmd)) {
> + clear_bit(STATUS_SCAN_HW, &priv->status);
> goto done;
> + }
>
> queue_delayed_work(priv->workqueue, &priv->scan_check,
> IWL_SCAN_CHECK_WATCHDOG);
>
> - return;
> + return 0;
>
> done:
> - /* Cannot perform scan. Make sure we clear scanning
> - * bits from status so next scan request can be performed.
> - * If we don't clear scanning status bit here all next scan
> - * will fail
> - */
> - clear_bit(STATUS_SCAN_HW, &priv->status);
> - clear_bit(STATUS_SCANNING, &priv->status);
> - /* inform mac80211 scan aborted */
> - queue_work(priv->workqueue, &priv->abort_scan);
> + return -EIO;
> }
>
> int iwlagn_manage_ibss_station(struct iwl_priv *priv,
> diff --git a/drivers/net/wireless/iwlwifi/iwl-agn.h b/drivers/net/wireless/iwlwifi/iwl-agn.h
> index cc6464d..015da26 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-agn.h
> +++ b/drivers/net/wireless/iwlwifi/iwl-agn.h
> @@ -216,7 +216,7 @@ void iwl_reply_statistics(struct iwl_priv *priv,
> struct iwl_rx_mem_buffer *rxb);
>
> /* scan */
> -void iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif);
> +int iwlagn_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif);
>
> /* station mgmt */
> int iwlagn_manage_ibss_station(struct iwl_priv *priv,
> diff --git a/drivers/net/wireless/iwlwifi/iwl-core.h b/drivers/net/wireless/iwlwifi/iwl-core.h
> index 5e6ee3d..110de0f 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-core.h
> +++ b/drivers/net/wireless/iwlwifi/iwl-core.h
> @@ -109,7 +109,7 @@ struct iwl_hcmd_utils_ops {
> __le16 fc, __le32 *tx_flags);
> int (*calc_rssi)(struct iwl_priv *priv,
> struct iwl_rx_phy_res *rx_resp);
> - void (*request_scan)(struct iwl_priv *priv, struct ieee80211_vif *vif);
> + int (*request_scan)(struct iwl_priv *priv, struct ieee80211_vif *vif);
> };
>
> struct iwl_apm_ops {
> diff --git a/drivers/net/wireless/iwlwifi/iwl-scan.c b/drivers/net/wireless/iwlwifi/iwl-scan.c
> index a4b3663..290140a 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-scan.c
> +++ b/drivers/net/wireless/iwlwifi/iwl-scan.c
> @@ -298,6 +298,7 @@ EXPORT_SYMBOL(iwl_init_scan_params);
>
> static int iwl_scan_initiate(struct iwl_priv *priv, struct ieee80211_vif *vif)
> {
> + int ret;
> lockdep_assert_held(&priv->mutex);
>
> IWL_DEBUG_INFO(priv, "Starting scan...\n");
> @@ -308,9 +309,11 @@ static int iwl_scan_initiate(struct iwl_priv *priv, struct ieee80211_vif *vif)
> if (WARN_ON(!priv->cfg->ops->utils->request_scan))
> return -EOPNOTSUPP;
>
> - priv->cfg->ops->utils->request_scan(priv, vif);
> + ret = priv->cfg->ops->utils->request_scan(priv, vif);
> + if (ret)
> + clear_bit(STATUS_SCANNING, &priv->status);
> + return ret;
>
> - return 0;
> }
>
> int iwl_mac_hw_scan(struct ieee80211_hw *hw,
> @@ -380,6 +383,7 @@ void iwl_internal_short_hw_scan(struct iwl_priv *priv)
>
> void iwl_bg_start_internal_scan(struct work_struct *work)
> {
> + int ret;
> struct iwl_priv *priv =
> container_of(work, struct iwl_priv, start_internal_scan);
>
> @@ -414,7 +418,11 @@ void iwl_bg_start_internal_scan(struct work_struct *work)
> if (WARN_ON(!priv->cfg->ops->utils->request_scan))
> goto unlock;
>
> - priv->cfg->ops->utils->request_scan(priv, NULL);
> + ret = priv->cfg->ops->utils->request_scan(priv, NULL);
> + if (ret) {
> + clear_bit(STATUS_SCANNING, &priv->status);
> + priv->is_internal_short_scan = false;
> + }
> unlock:
> mutex_unlock(&priv->mutex);
> }
> diff --git a/drivers/net/wireless/iwlwifi/iwl3945-base.c b/drivers/net/wireless/iwlwifi/iwl3945-base.c
> index d31661c..fc82da0 100644
> --- a/drivers/net/wireless/iwlwifi/iwl3945-base.c
> +++ b/drivers/net/wireless/iwlwifi/iwl3945-base.c
> @@ -2799,7 +2799,7 @@ static void iwl3945_rfkill_poll(struct work_struct *data)
>
> }
>
> -void iwl3945_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
> +int iwl3945_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
> {
> struct iwl_host_cmd cmd = {
> .id = REPLY_SCAN_CMD,
> @@ -3000,25 +3000,18 @@ void iwl3945_request_scan(struct iwl_priv *priv, struct ieee80211_vif *vif)
> scan->len = cpu_to_le16(cmd.len);
>
> set_bit(STATUS_SCAN_HW, &priv->status);
> - if (iwl_send_cmd_sync(priv, &cmd))
> + if (iwl_send_cmd_sync(priv, &cmd)) {
> + clear_bit(STATUS_SCAN_HW, &priv->status);
> goto done;
> + }
>
> queue_delayed_work(priv->workqueue, &priv->scan_check,
> IWL_SCAN_CHECK_WATCHDOG);
>
> - return;
> + return 0;
>
> done:
> - /* can not perform scan make sure we clear scanning
> - * bits from status so next scan request can be performed.
> - * if we dont clear scanning status bit here all next scan
> - * will fail
> - */
> - clear_bit(STATUS_SCAN_HW, &priv->status);
> - clear_bit(STATUS_SCANNING, &priv->status);
> -
> - /* inform mac80211 scan aborted */
> - queue_work(priv->workqueue, &priv->abort_scan);
> + return -EIO;
> }
>
> static void iwl3945_bg_restart(struct work_struct *data)
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH wireless-2.6 or stable] iwlwifi: return error when fail to start scanning
2010-10-06 17:45 ` Florian Mickler
@ 2010-10-06 17:48 ` Guy, Wey-Yi
0 siblings, 0 replies; 13+ messages in thread
From: Guy, Wey-Yi @ 2010-10-06 17:48 UTC (permalink / raw)
To: Florian Mickler, Stanislaw Gruszka
Cc: linville@tuxdriver.com, stable@kernel.org,
linux-wireless@vger.kernel.org, Chatre, Reinette,
ilw@linux.intel.com, Berg, Johannes, Cahill, Ben M,
netdev@vger.kernel.org, linux-kernel@vger.kernel.org
On Wed, 2010-10-06 at 10:45 -0700, Florian Mickler wrote:
> Hi Stanislaw!
>
> On Wed, 6 Oct 2010 18:04:35 +0200
> Stanislaw Gruszka <sgruszka@redhat.com> wrote:
>
> > This is stable friendly backport of wireless-testing commit
> > 3eecce527c7434dfd16517ce08b49632c8a26717 "iwlwifi: unify scan start
> > checks". Wireless-testing version include also a lot of cleanups.
> >
> > On error case in {iwl3945,iwlagn}_request_scan we queue abort_scan work,
> > which in a fact do nothing, so we never complete the scan. Thats gives
> > "WARNING: at net/wireless/core.c:614 wdev_cleanup_work" and stopped
> > network connection until reload iwlwifi modules. Return of -EIO and
> > stopping queuing any work is a fix.
> >
> > Note there are still many cases when we can get:
> >
> > WARNING: at net/wireless/core.c:614 wdev_cleanup_work
> > WARNING: at net/mac80211/scan.c:266 ieee80211_scan_completed
> > WARNING: at net/mac80211/scan.c:269 ieee80211_scan_complete
> >
> > which are not fixed. Unfortunately does not exist single, small fix
> > for that problems, and we probably will see some more bug reports
> > with scan warnings. As solution we rewrite iwl-scan.c code to avoid
> > all possible race conditions. That quite big patch set is queued
> > for 2.6.37 .
> >
> > Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
Signed-off-by: Wey-Yi Guy <wey-yi.w.guy@intel.com>
looks good to me
Thanks
Wey
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH wireless-2.6 or stable] iwlwifi: return error when fail to start scanning
[not found] ` <20101006160434.GB24581-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2010-10-06 17:45 ` Florian Mickler
@ 2010-10-06 20:01 ` John W. Linville
1 sibling, 0 replies; 13+ messages in thread
From: John W. Linville @ 2010-10-06 20:01 UTC (permalink / raw)
To: Stanislaw Gruszka
Cc: Florian Mickler, stable-DgEjT+Ai2ygdnm+yROfE0A,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
wey-yi.w.guy-ral2JQCrhuEAvxtiuMwx3w,
reinette.chatre-ral2JQCrhuEAvxtiuMwx3w,
ilw-VuQAYsv1563Yd54FQh9/CA, johannes.berg-ral2JQCrhuEAvxtiuMwx3w,
ben.m.cahill-ral2JQCrhuEAvxtiuMwx3w,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
On Wed, Oct 06, 2010 at 06:04:35PM +0200, Stanislaw Gruszka wrote:
> This is stable friendly backport of wireless-testing commit
> 3eecce527c7434dfd16517ce08b49632c8a26717 "iwlwifi: unify scan start
> checks". Wireless-testing version include also a lot of cleanups.
>
> On error case in {iwl3945,iwlagn}_request_scan we queue abort_scan work,
> which in a fact do nothing, so we never complete the scan. Thats gives
> "WARNING: at net/wireless/core.c:614 wdev_cleanup_work" and stopped
> network connection until reload iwlwifi modules. Return of -EIO, and
> stopping queuing any work is a fix.
>
> Note there are still many cases when we can get:
>
> WARNING: at net/wireless/core.c:614 wdev_cleanup_work
> WARNING: at net/mac80211/scan.c:266 ieee80211_scan_completed
> WARNING: at net/mac80211/scan.c:269 ieee80211_scan_complete
>
> which are not fixed. Unfortunately does not exist single, small fix
> for that problems, and we probably will see some more bug reports
> with scan warnings. As solution we rewrite iwl-scan.c code to avoid
> all possible race conditions. That quite big patch set is queued
> for 2.6.37 .
>
> Signed-off-by: Stanislaw Gruszka <sgruszka-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> ---
> Patch is intended for wireless-2.6, or in case when it does not
> make 2.6.36 release, for -stable 2.6.36.y
I'm fine with this patch for -stable, but I'm not sure it is worthwhile
to rush it into 2.6.36. The problem it fixes is intermittent and
not always very severe, and we've already been living with it for
4 releases.
John
--
John W. Linville Someday the world will need a hero, and you
linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org might be all we have. Be ready.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2010-10-06 20:01 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-10-05 6:43 Fw: [PATCH] iwl3945: queue the right work if the scan needs to be aborted Florian Mickler
[not found] ` <20101005084305.52a31ed4-mGsOIKOveelVRbCss4o9kg@public.gmane.org>
2010-10-05 8:57 ` Stanislaw Gruszka
2010-10-05 10:12 ` Florian Mickler
[not found] ` <20101005121242.79cdafc2-mGsOIKOveelVRbCss4o9kg@public.gmane.org>
2010-10-05 10:43 ` Stanislaw Gruszka
[not found] ` <20101005104357.GB18833-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2010-10-05 22:29 ` Florian Mickler
[not found] ` <20101005085717.GA18012-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2010-10-05 22:21 ` [PATCH] iwlwifi: fix iwlwifi scanning corner cases Florian Mickler
[not found] ` <1286317292-10679-1-git-send-email-florian-sVu6HhrpSfRAfugRpC6u6w@public.gmane.org>
2010-10-06 9:02 ` Stanislaw Gruszka
[not found] ` <20101006090228.GA2523-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2010-10-06 16:04 ` [PATCH wireless-2.6 or stable] iwlwifi: return error when fail to start scanning Stanislaw Gruszka
2010-10-06 16:12 ` Guy, Wey-Yi
2010-10-06 17:32 ` Florian Mickler
[not found] ` <20101006160434.GB24581-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
2010-10-06 17:45 ` Florian Mickler
2010-10-06 17:48 ` Guy, Wey-Yi
2010-10-06 20:01 ` John W. Linville
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).