iwl3945: remove check_plcp_health

RFC In review Stanislaw Gruszka 3 participants 5 messages Jan 25, 2011 – Feb 8, 2011 Networking drivers (wireless)

lore

Share & tools

lore

Stanislaw Gruszka

author

2011-01-25 09:35 UTC #0 lore raw

RFC 1/1 iwl3945: remove check_plcp_health

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(-)

drivers/net/wireless/iwlwifi/iwl-3945.c +0 -1

index 8cacb4e..eb916d7 100644
@@ -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,

Signature · 1 line

1.7.1

wwguy

Acked-by on v1 1/1

2011-01-25 14:57 UTC #1 lore raw

Quoted 6 lines · 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

Quoted 8 lines

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 <[email protected]>

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

Wey

John W. Linville

2011-01-25 15:15 UTC #2 lore raw

In reply to Stanislaw Gruszka

Quoted 22 lines · 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
(quoted diff) +0 -2
-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(-)
drivers/net/wireless/iwlwifi/iwl-3945.c +0 -0
index 8cacb4e..eb916d7 100644

drivers/net/wireless/iwlwifi/iwl-3945.c static struct iwl_lib_ops iwl3945_lib = { · line 2837

Quoted 7 lines

drivers/net/wireless/iwlwifi/iwl-3945.c +0 -1

@@ -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

Signature · 2 lines

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

Stanislaw Gruszka

author

2011-01-26 07:16 UTC #3 lore raw

Quoted 28 lines · John W. Linville

Quoted 27 lines · 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
(quoted diff) +0 -2
-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(-)
drivers/net/wireless/iwlwifi/iwl-3945.c +0 -1
index 8cacb4e..eb916d7 100644
@@ -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,
 

Quoted 5 lines

drivers/net/wireless/iwlwifi/iwl-3945.c static struct iwl_lib_ops iwl3945_lib = { · line 2837
Quoted 2 lines
drivers/net/wireless/iwlwifi/iwl-3945.c +0 -0
 	.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.

Quoted 2 lines

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

Stanislaw Gruszka

author

2011-02-08 07:35 UTC #4 lore raw

In reply to John W. Linville

Quoted 28 lines · John W. Linville

Quoted 27 lines · 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
(quoted diff) +0 -2
-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(-)
drivers/net/wireless/iwlwifi/iwl-3945.c +0 -1
index 8cacb4e..eb916d7 100644
@@ -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,
 

Quoted 5 lines

drivers/net/wireless/iwlwifi/iwl-3945.c static struct iwl_lib_ops iwl3945_lib = { · line 2837
Quoted 2 lines
drivers/net/wireless/iwlwifi/iwl-3945.c +0 -0
 	.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