On Fri, Apr 9, 2010 at 1:19 AM, Linus Torvalds 

The screen is so fast and never stops so I don't know how to capture that. 
I just tried the patch from Wey and had to modify it slightly to make it 
work.



On Fri, Apr 9, 2010 at 2:50 AM, Guy, Wey-Yi <wey-yi.w.guy@intel.com> 
similar approach for 4965 like the newer devices. could you try the
attach patch to see if this fix your system freeze problem.

Wey, the patch doesn't apply cleanly. I believe you meant "iwl_" instead 
of "iwlagn_".

Also, the checking for IWL_INVALID_STATION should be done after the "} 
else {" as in the original code prior to your patch. I just verified this 
with the patch below and it no longer oops.





I just tried that, but not seeing any invalid station messages. The 
KERN_ERR has been added below.




On Fri, Apr 9, 2010 at 4:09 AM, Guy, Wey-Yi <wey-yi.w.guy@intel.com> 


Ok, added below.



Thanks,
Jeff

--- drivers/net/wireless/iwlwifi/iwl-4965.c.org	2010-04-09 02:11:45.000000000 +0800
+++ drivers/net/wireless/iwlwifi/iwl-4965.c	2010-04-09 03:21:57.000000000 +0800
@@ -2012,10 +2012,18 @@

  		if (txq->q.read_ptr != (scd_ssn & 0xff)) {
  			index = iwl_queue_dec_wrap(scd_ssn & 0xff, txq->q.n_bd);
-			IWL_DEBUG_TX_REPLY(priv, "Retry scheduler reclaim scd_ssn "
-					   "%d index %d\n", scd_ssn , index);
+			IWL_DEBUG_TX_REPLY(priv,
+					"Retry scheduler reclaim scd_ssn "
+					"%d index %d\n", scd_ssn , index);
  			freed = iwl_tx_queue_reclaim(priv, txq_id, index);
  			iwl_free_tfds_in_queue(priv, sta_id, tid, freed);
+			if (qc && likely(sta_id != IWL_INVALID_STATION))
+				iwl_free_tfds_in_queue(priv, sta_id, tid,
+					freed);
+			else {
+				if (sta_id == IWL_INVALID_STATION)
+				       IWL_ERR(priv, "invalid station");
+			}

  			if (priv->mac80211_registered &&
  			    (iwl_queue_space(&txq->q) > txq->q.low_mark) &&
@@ -2041,14 +2049,20 @@
  				   tx_resp->failure_frame);

  		freed = iwl_tx_queue_reclaim(priv, txq_id, ...
[ message continues ]

On Fri, Apr 9, 2010 at 4:24 AM, Guy, Wey-Yi <wey-yi.w.guy@intel.com> 

Wey,

I've updated your patch and tested it. Wireless seems ok now.

Please verify it and inform Linus once you sign-off on it.


Thanks,
Jeff

--- a/drivers/net/wireless/iwlwifi/iwl-4965.c.org	2010-04-09 02:11:45.000000000 +0800
+++ a/drivers/net/wireless/iwlwifi/iwl-4965.c	2010-04-09 03:33:43.000000000 +0800
@@ -2012,10 +2012,15 @@

  		if (txq->q.read_ptr != (scd_ssn & 0xff)) {
  			index = iwl_queue_dec_wrap(scd_ssn & 0xff, txq->q.n_bd);
-			IWL_DEBUG_TX_REPLY(priv, "Retry scheduler reclaim scd_ssn "
-					   "%d index %d\n", scd_ssn , index);
+			IWL_DEBUG_TX_REPLY(priv,
+					"Retry scheduler reclaim scd_ssn "
+					"%d index %d\n", scd_ssn , index);
  			freed = iwl_tx_queue_reclaim(priv, txq_id, index);
-			iwl_free_tfds_in_queue(priv, sta_id, tid, freed);
+			if(qc)
+				iwl_free_tfds_in_queue(priv, sta_id, tid,
+					freed);
+			else if (sta_id == IWL_INVALID_STATION)
+				       IWL_ERR(priv, "invalid station");

  			if (priv->mac80211_registered &&
  			    (iwl_queue_space(&txq->q) > txq->q.low_mark) &&
@@ -2041,14 +2046,18 @@
  				   tx_resp->failure_frame);

  		freed = iwl_tx_queue_reclaim(priv, txq_id, index);
-		iwl_free_tfds_in_queue(priv, sta_id, tid, freed);
+		if (qc && likely(sta_id != IWL_INVALID_STATION))
+			iwl_free_tfds_in_queue(priv, sta_id, tid, freed);
+		else if (sta_id == IWL_INVALID_STATION)
+		       IWL_ERR(priv, "invalid station");

  		if (priv->mac80211_registered &&
  		    (iwl_queue_space(&txq->q) > txq->q.low_mark))
  			iwl_wake_queue(priv, txq_id);
  	}

-	iwl_txq_check_empty(priv, sta_id, tid, txq_id);
+	if (qc && likely(sta_id != IWL_INVALID_STATION))
+		iwl_txq_check_empty(priv, sta_id, tid, txq_id);

  	if (iwl_check_bits(status, TX_ABORT_REQUIRED_MSK))
  		IWL_ERR(priv, "TODO:  Implement Tx ABORT REQUIRED!!!\n");
--

[ message continues ]

Hi Jeff,


do not need to check sta_id == IWL_INVALID_STATION here since already
check before.
could you try my revised version of patch I send after my first attempt.
to make sure it works. 

I attach here again in case you did not get it.

Thanks
Wey


[ message continues ]

I would prefer to take this through the normal wireless-2.6 route if
you don't mind.  Are you satisifed with this version?

Thanks,

John

P.S.  In the future, I'm also quite open to revert requests as needed.
It makes my life easier for things (even reverts) under wireless to
come through my tree rather than trying to figure-out how something
happened later...
-- 
John W. Linville		Someday the world will need a hero, and you
linville@tuxdriver.com			might be all we have.  Be ready.
--

[ message continues ]

Hi John,


Thanks for the help, the finial version should take care of the problem
and ready for upstream .

Regards

--

[ message continues ]

Dave,

This fix is intended for 2.6.34.  It resolves an issue involving an
Oops on boxes w/ iwl4965 hardware, apparently introduced by another
recent patch.  The thread describing the issue and the resolution
is here:

	http://marc.info/?l=linux-kernel&m=127074721531649&w=2

In order to avoid reverting that patch, please accept this fix instead.
As usual, please let me know if there are problems!

Thanks,

John

---

The following changes since commit 2626419ad5be1a054d350786b684b41d23de1538:
  David S. Miller (1):
        tcp: Set CHECKSUM_UNNECESSARY in tcp_init_nondata_skb

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless-2.6.git master

Wey-Yi Guy (1):
      iwlwifi: need check for valid qos packet before free

 drivers/net/wireless/iwlwifi/iwl-4965.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-4965.c b/drivers/net/wireless/iwlwifi/iwl-4965.c
index 83c52a6..8972166 100644
--- a/drivers/net/wireless/iwlwifi/iwl-4965.c
+++ b/drivers/net/wireless/iwlwifi/iwl-4965.c
@@ -2015,7 +2015,9 @@ static void iwl4965_rx_reply_tx(struct iwl_priv *priv,
 			IWL_DEBUG_TX_REPLY(priv, "Retry scheduler reclaim scd_ssn "
 					   "%d index %d\n", scd_ssn , index);
 			freed = iwl_tx_queue_reclaim(priv, txq_id, index);
-			iwl_free_tfds_in_queue(priv, sta_id, tid, freed);
+			if (qc)
+				iwl_free_tfds_in_queue(priv, sta_id,
+						       tid, freed);
 
 			if (priv->mac80211_registered &&
 			    (iwl_queue_space(&txq->q) > txq->q.low_mark) &&
@@ -2041,14 +2043,17 @@ static void iwl4965_rx_reply_tx(struct iwl_priv *priv,
 				   tx_resp->failure_frame);
 
 		freed = iwl_tx_queue_reclaim(priv, txq_id, index);
-		iwl_free_tfds_in_queue(priv, sta_id, tid, freed);
+		if (qc && likely(sta_id != IWL_INVALID_STATION))
+			iwl_free_tfds_in_queue(priv, sta_id, tid, freed);
+		else if (sta_id == IWL_INVALID_STATION)
+			IWL_DEBUG_TX_REPLY(priv, ...
[ message continues ]

From: "John W. Linville" <linville@tuxdriver.com>

Pulled, thanks John.
--

[ message continues ]

Hi Jeff,

Thanks for verify the patch, you are correct, since I am using the
wireless-testing version of the kernel, you will need to make the
changes in order to get the patch apply. Sorry for the trouble.

Regards

--

[ message continues ]

He based his patch on wireless-testing (or something similar), where
iwlagn_ is the proper prefix for the functions in question.

John

P.S. Cc'ing linux-wireless...

P.P.S.  You might try this version of his later patch...

From ece6444c2fe80dab679beb5f0d58b091f1933b00 Mon Sep 17 00:00:00 2001
From: Wey-Yi Guy <wey-yi.w.guy@intel.com>
Date: Thu, 8 Apr 2010 13:17:37 -0700
Subject: [PATCH] iwlwifi: need check for valid qos packet before free

For 4965, need to check it is valid qos frame before free, only valid
QoS frame has the tid used to free the packets.

Signed-off-by: Wey-Yi Guy <wey-yi.w.guy@intel.com>
Signed-off-by: John W. Linville <linville@tuxdriver.com>
---
 drivers/net/wireless/iwlwifi/iwl-4965.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-4965.c b/drivers/net/wireless/iwlwifi/iwl-4965.c
index 83c52a6..8972166 100644
--- a/drivers/net/wireless/iwlwifi/iwl-4965.c
+++ b/drivers/net/wireless/iwlwifi/iwl-4965.c
@@ -2015,7 +2015,9 @@ static void iwl4965_rx_reply_tx(struct iwl_priv *priv,
 			IWL_DEBUG_TX_REPLY(priv, "Retry scheduler reclaim scd_ssn "
 					   "%d index %d\n", scd_ssn , index);
 			freed = iwl_tx_queue_reclaim(priv, txq_id, index);
-			iwl_free_tfds_in_queue(priv, sta_id, tid, freed);
+			if (qc)
+				iwl_free_tfds_in_queue(priv, sta_id,
+						       tid, freed);
 
 			if (priv->mac80211_registered &&
 			    (iwl_queue_space(&txq->q) > txq->q.low_mark) &&
@@ -2041,14 +2043,17 @@ static void iwl4965_rx_reply_tx(struct iwl_priv *priv,
 				   tx_resp->failure_frame);
 
 		freed = iwl_tx_queue_reclaim(priv, txq_id, index);
-		iwl_free_tfds_in_queue(priv, sta_id, tid, freed);
+		if (qc && likely(sta_id != IWL_INVALID_STATION))
+			iwl_free_tfds_in_queue(priv, sta_id, tid, freed);
+		else if (sta_id == IWL_INVALID_STATION)
+			IWL_DEBUG_TX_REPLY(priv, "Station not known\n");
 
 		if (priv->mac80211_registered &&
 		    (iwl_queue_space(&txq->q) > txq->q.low_mark))
 ...
[ message continues ]

"She based her patch..." -- my apologies!

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

[ message continues ]