[RFC] iwl3945: remove check_plcp_health

[RFC] iwl3945: remove check_plcp_health

[Stanislaw Gruszka]

This patch helps some users currently complaining about iwl3945
performance.

https://bugzilla.redhat.com/show_bug.cgi?id=654599
https://bugs.launchpad.net/ubuntu/maverick/+source/linux/+bug/621265/+index?comments=all

Patch does not helps all users, but they seems to be affected
by different bug or bugs.

Patch effectively reverts commit a29576a7844326c5223f4d4adbfd3f4d64173d4c
"iwl3945: add plcp error checking". It is minimal fix intended to
-stable posting. I will post cleaning up patch, if fix will be accepted.

---
 drivers/net/wireless/iwlwifi/iwl-3945.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-3945.c b/drivers/net/wireless/iwlwifi/iwl-3945.c
index 8cacb4e..eb916d7 100644
--- a/drivers/net/wireless/iwlwifi/iwl-3945.c
+++ b/drivers/net/wireless/iwlwifi/iwl-3945.c
@@ -2832,7 +2832,6 @@ static struct iwl_lib_ops iwl3945_lib = {
 	.config_ap = iwl3945_config_ap,
 	.manage_ibss_station = iwl3945_manage_ibss_station,
 	.recover_from_tx_stall = iwl_bg_monitor_recover,
-	.check_plcp_health = iwl3945_good_plcp_health,
 
 	.debugfs_ops = {
 		.rx_stats_read = iwl3945_ucode_rx_stats_read,
-- 
1.7.1

Re: [RFC] iwl3945: remove check_plcp_health

[wwguy]

On Tue, 2011-01-25 at 01:35 -0800, Stanislaw Gruszka wrote:
> This patch helps some users currently complaining about iwl3945
> performance.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=654599
> https://bugs.launchpad.net/ubuntu/maverick/+source/linux/+bug/621265/+index?comments=all
> 
> Patch does not helps all users, but they seems to be affected
> by different bug or bugs.
> 
> Patch effectively reverts commit a29576a7844326c5223f4d4adbfd3f4d64173d4c
> "iwl3945: add plcp error checking". It is minimal fix intended to
> -stable posting. I will post cleaning up patch, if fix will be accepted.
> 
> ---
Acked-by: Wey-Yi Guy <wey-yi.w.guy@intel.com>


Agree, maybe I am wrong, but I don't think this plcp checking never
being fully tested for 3945
  
Wey

Re: [RFC] iwl3945: remove check_plcp_health

[John W. Linville]

On Tue, Jan 25, 2011 at 10:35:28AM +0100, Stanislaw Gruszka wrote:
> This patch helps some users currently complaining about iwl3945
> performance.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=654599
> https://bugs.launchpad.net/ubuntu/maverick/+source/linux/+bug/621265/+index?comments=all
> 
> Patch does not helps all users, but they seems to be affected
> by different bug or bugs.
> 
> Patch effectively reverts commit a29576a7844326c5223f4d4adbfd3f4d64173d4c
> "iwl3945: add plcp error checking". It is minimal fix intended to
> -stable posting. I will post cleaning up patch, if fix will be accepted.
> 
> ---
>  drivers/net/wireless/iwlwifi/iwl-3945.c |    1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/net/wireless/iwlwifi/iwl-3945.c b/drivers/net/wireless/iwlwifi/iwl-3945.c
> index 8cacb4e..eb916d7 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-3945.c
> +++ b/drivers/net/wireless/iwlwifi/iwl-3945.c
> @@ -2832,7 +2832,6 @@ static struct iwl_lib_ops iwl3945_lib = {
>  	.config_ap = iwl3945_config_ap,
>  	.manage_ibss_station = iwl3945_manage_ibss_station,
>  	.recover_from_tx_stall = iwl_bg_monitor_recover,
> -	.check_plcp_health = iwl3945_good_plcp_health,
>  
>  	.debugfs_ops = {
>  		.rx_stats_read = iwl3945_ucode_rx_stats_read,

Did you experiment with different values for
IWL_MAX_PLCP_ERR_LONG_THRESHOLD_DEF for iwl3945?

Should we simply revert commit a29576a7844326c5223f4d4adbfd3f4d64173d4c
instead?

John
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.

Re: [RFC] iwl3945: remove check_plcp_health

[Stanislaw Gruszka]

On Tue, Jan 25, 2011 at 10:14:46AM -0500, John W. Linville wrote:
> On Tue, Jan 25, 2011 at 10:35:28AM +0100, Stanislaw Gruszka wrote:
> > This patch helps some users currently complaining about iwl3945
> > performance.
> > 
> > https://bugzilla.redhat.com/show_bug.cgi?id=654599
> > https://bugs.launchpad.net/ubuntu/maverick/+source/linux/+bug/621265/+index?comments=all
> > 
> > Patch does not helps all users, but they seems to be affected
> > by different bug or bugs.
> > 
> > Patch effectively reverts commit a29576a7844326c5223f4d4adbfd3f4d64173d4c
> > "iwl3945: add plcp error checking". It is minimal fix intended to
> > -stable posting. I will post cleaning up patch, if fix will be accepted.
> > 
> > ---
> >  drivers/net/wireless/iwlwifi/iwl-3945.c |    1 -
> >  1 files changed, 0 insertions(+), 1 deletions(-)
> > 
> > diff --git a/drivers/net/wireless/iwlwifi/iwl-3945.c b/drivers/net/wireless/iwlwifi/iwl-3945.c
> > index 8cacb4e..eb916d7 100644
> > --- a/drivers/net/wireless/iwlwifi/iwl-3945.c
> > +++ b/drivers/net/wireless/iwlwifi/iwl-3945.c
> > @@ -2832,7 +2832,6 @@ static struct iwl_lib_ops iwl3945_lib = {
> >  	.config_ap = iwl3945_config_ap,
> >  	.manage_ibss_station = iwl3945_manage_ibss_station,
> >  	.recover_from_tx_stall = iwl_bg_monitor_recover,
> > -	.check_plcp_health = iwl3945_good_plcp_health,
> >  
> >  	.debugfs_ops = {
> >  		.rx_stats_read = iwl3945_ucode_rx_stats_read,
> 
> Did you experiment with different values for
> IWL_MAX_PLCP_ERR_LONG_THRESHOLD_DEF for iwl3945?

No, I'm not able to reproduce the problem. I will request that
via bugzilla to users hitting bug.

> Should we simply revert commit a29576a7844326c5223f4d4adbfd3f4d64173d4c
> instead?

Due to massive other automatic revert fails on upstream code, also
it would be relatively hard to backport. So I decided to do oneliner
fix for stable, and clean up (revert all not necessary code) only
upstream separately.

Cheers
Stanislaw

Re: [RFC] iwl3945: remove check_plcp_health

[Stanislaw Gruszka]

On Tue, Jan 25, 2011 at 10:14:46AM -0500, John W. Linville wrote:
> On Tue, Jan 25, 2011 at 10:35:28AM +0100, Stanislaw Gruszka wrote:
> > This patch helps some users currently complaining about iwl3945
> > performance.
> > 
> > https://bugzilla.redhat.com/show_bug.cgi?id=654599
> > https://bugs.launchpad.net/ubuntu/maverick/+source/linux/+bug/621265/+index?comments=all
> > 
> > Patch does not helps all users, but they seems to be affected
> > by different bug or bugs.
> > 
> > Patch effectively reverts commit a29576a7844326c5223f4d4adbfd3f4d64173d4c
> > "iwl3945: add plcp error checking". It is minimal fix intended to
> > -stable posting. I will post cleaning up patch, if fix will be accepted.
> > 
> > ---
> >  drivers/net/wireless/iwlwifi/iwl-3945.c |    1 -
> >  1 files changed, 0 insertions(+), 1 deletions(-)
> > 
> > diff --git a/drivers/net/wireless/iwlwifi/iwl-3945.c b/drivers/net/wireless/iwlwifi/iwl-3945.c
> > index 8cacb4e..eb916d7 100644
> > --- a/drivers/net/wireless/iwlwifi/iwl-3945.c
> > +++ b/drivers/net/wireless/iwlwifi/iwl-3945.c
> > @@ -2832,7 +2832,6 @@ static struct iwl_lib_ops iwl3945_lib = {
> >  	.config_ap = iwl3945_config_ap,
> >  	.manage_ibss_station = iwl3945_manage_ibss_station,
> >  	.recover_from_tx_stall = iwl_bg_monitor_recover,
> > -	.check_plcp_health = iwl3945_good_plcp_health,
> >  
> >  	.debugfs_ops = {
> >  		.rx_stats_read = iwl3945_ucode_rx_stats_read,
> 
> Did you experiment with different values for
> IWL_MAX_PLCP_ERR_LONG_THRESHOLD_DEF for iwl3945?

For fedora user increasing threshold help as well. Ubuntu
crowd gave some confusing testing results.

Anyway, after further investigation done in 
https://bugzilla.redhat.com/show_bug.cgi?id=671366
we discovered that hw scanning during association
sometimes put hardware in state when it is not able
to receive frames at higher rates.

Hw scan can be requested by plcp check or by NM, both
cases may cause troubles.

Also tx rates can be low, using common rate scaling code
helps.

My plan to fix performance problems on 3945:
- send patch to stable for increase plcp threshold on 3945
- make sw scan default on upstream
- add module option to disable plcp_check at all 
- remove custom rate scaling algorithm

Stanislaw