[PATCH 0/3] fixes for 2.6.33 and stable

[PATCH 0/3] fixes for 2.6.33 and stable

[Reinette Chatre]

We include three fixes for 2.6.33, all of which target stable also.

The first is a fix to a recently submitted patch "iwlwifi: Fix to set correct ht
configuration"

With the second two patches we start to address
http://bugzilla.intellinuxwireless.org/show_bug.cgi?id=2160 where it can be
seen that there are instances where the number of freed tfds are negative.
We include two patches to (1) prevent the problem from
occuring, and (2) prevent the problem from propagating when it does occur.
Unfortunately "iwlwifi: sanity check before counting number of tfds can be free" is not
the only fix needed for this situation to occur, hence "iwlwifi: error checking
for number of tfds in queue"  is needed while this problem is being
investigated further.

These patches are also available from wireless-2.6 branch on
git://git.kernel.org/pub/scm/linux/kernel/git/iwlwifi/iwlwifi-2.6.git


Dan Halperin (1):
  iwlwifi: set HT flags after channel in rxon

Wey-Yi Guy (2):
  iwlwifi: sanity check before counting number of tfds can be free
  iwlwifi: error checking for number of tfds in queue

 drivers/net/wireless/iwlwifi/iwl-4965.c |    2 +-
 drivers/net/wireless/iwlwifi/iwl-5000.c |    2 +-
 drivers/net/wireless/iwlwifi/iwl-core.c |    2 +-
 drivers/net/wireless/iwlwifi/iwl-core.h |    2 ++
 drivers/net/wireless/iwlwifi/iwl-tx.c   |   25 +++++++++++++++++++++++--
 5 files changed, 28 insertions(+), 5 deletions(-)

[PATCH 1/3] iwlwifi: set HT flags after channel in rxon

[Reinette Chatre]

From: Dan Halperin <dhalperi@cs.washington.edu>

The HT extension channel settings require priv->staging_rxon.channel to be
accurate. However, iwl_set_rxon_ht was being called before iwl_set_rxon_channel
and thus HT40 could be broken unless another call to iwl_mac_config came in.

This problem was recently introduced by "iwlwifi: Fix to set correct ht
configuration"

The particular setting in which I noticed this was monitor mode:

	iwconfig wlan0 mode monitor
	ifconfig wlan0 up
	./iw wlan0 set channel 64 HT40-
	#./iw wlan0 set channel 64 HT40-
	tcpdump -i wlan0 -y IEEE802_11_RADIO

would only catch HT40 packets if I issued the IW command twice.

>From visual inspection, iwl_set_rxon_channel does not depend on
iwl_set_rxon_ht, so simply swapping them should be safe and fixes this problem.

Signed-off-by: Daniel Halperin <dhalperi@cs.washington.edu>
Acked-by: Wey-Yi Guy <wey-yi.w.guy@intel.com>
Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
CC: stable@kernel.org
---
 drivers/net/wireless/iwlwifi/iwl-core.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-core.c b/drivers/net/wireless/iwlwifi/iwl-core.c
index d10bea6..f36f804 100644
--- a/drivers/net/wireless/iwlwifi/iwl-core.c
+++ b/drivers/net/wireless/iwlwifi/iwl-core.c
@@ -2744,8 +2744,8 @@ int iwl_mac_config(struct ieee80211_hw *hw, u32 changed)
 		if ((le16_to_cpu(priv->staging_rxon.channel) != ch))
 			priv->staging_rxon.flags = 0;
 
-		iwl_set_rxon_ht(priv, ht_conf);
 		iwl_set_rxon_channel(priv, conf->channel);
+		iwl_set_rxon_ht(priv, ht_conf);
 
 		iwl_set_flags_for_band(priv, conf->channel->band);
 		spin_unlock_irqrestore(&priv->lock, flags);
-- 
1.6.3.3

[PATCH 2/3] iwlwifi: sanity check before counting number of tfds can be free

[Reinette Chatre]

From: Wey-Yi Guy <wey-yi.w.guy@intel.com>

Check the frame control for ieee80211_is_data_qos() is true before
counting the number of tfds can be free, the tfds_in_queue only
increment when ieee80211_is_data_qos() is true before transmit; so it should
only decrement if the type match.

This fix starts to address
http://bugzilla.intellinuxwireless.org/show_bug.cgi?id=2160 . There are
more situations in which this problem (freed tfds below zero) can occur so
this bug is not closed yet.

Signed-off-by: Wey-Yi Guy <wey-yi.w.guy@intel.com>
Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
CC: stable@kernel.org
---
 drivers/net/wireless/iwlwifi/iwl-tx.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-tx.c b/drivers/net/wireless/iwlwifi/iwl-tx.c
index 87ce2bd..9196d3f 100644
--- a/drivers/net/wireless/iwlwifi/iwl-tx.c
+++ b/drivers/net/wireless/iwlwifi/iwl-tx.c
@@ -1131,6 +1131,7 @@ int iwl_tx_queue_reclaim(struct iwl_priv *priv, int txq_id, int index)
 	struct iwl_queue *q = &txq->q;
 	struct iwl_tx_info *tx_info;
 	int nfreed = 0;
+	struct ieee80211_hdr *hdr;
 
 	if ((index >= q->n_bd) || (iwl_queue_used(q, index) == 0)) {
 		IWL_ERR(priv, "Read index for DMA queue txq id (%d), index %d, "
@@ -1145,13 +1146,19 @@ int iwl_tx_queue_reclaim(struct iwl_priv *priv, int txq_id, int index)
 
 		tx_info = &txq->txb[txq->q.read_ptr];
 		iwl_tx_status(priv, tx_info->skb[0]);
+
+		if (tx_info->skb[0]) {
+			hdr = (struct ieee80211_hdr *)
+				((struct sk_buff *)tx_info->skb[0])->data;
+			if (hdr && ieee80211_is_data_qos(hdr->frame_control))
+				nfreed++;
+		}
 		tx_info->skb[0] = NULL;
 
 		if (priv->cfg->ops->lib->txq_inval_byte_cnt_tbl)
 			priv->cfg->ops->lib->txq_inval_byte_cnt_tbl(priv, txq);
 
 		priv->cfg->ops->lib->txq_free_tfd(priv, txq);
-		nfreed++;
 	}
 	return nfreed;
 }
-- 
1.6.3.3

Re: [PATCH 2/3] iwlwifi: sanity check before counting number of tfds can be free

[Stanislaw Gruszka]

Hi Reinette 
On Thu, Feb 18, 2010 at 10:01:40PM -0800, Reinette Chatre wrote:
> This fix starts to address
> http://bugzilla.intellinuxwireless.org/show_bug.cgi?id=2160 . There are
> more situations in which this problem (freed tfds below zero) can occur so
> this bug is not closed yet.

Only starts ... ,hmm so these two patches does not fix any issue.

> diff --git a/drivers/net/wireless/iwlwifi/iwl-tx.c b/drivers/net/wireless/iwlwifi/iwl-tx.c
> index 87ce2bd..9196d3f 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-tx.c
> +++ b/drivers/net/wireless/iwlwifi/iwl-tx.c
> @@ -1131,6 +1131,7 @@ int iwl_tx_queue_reclaim(struct iwl_priv *priv, int txq_id, int index)
>  	struct iwl_queue *q = &txq->q;
>  	struct iwl_tx_info *tx_info;
>  	int nfreed = 0;
> +	struct ieee80211_hdr *hdr;
>  
>  	if ((index >= q->n_bd) || (iwl_queue_used(q, index) == 0)) {
>  		IWL_ERR(priv, "Read index for DMA queue txq id (%d), index %d, "
> @@ -1145,13 +1146,19 @@ int iwl_tx_queue_reclaim(struct iwl_priv *priv, int txq_id, int index)
>  
>  		tx_info = &txq->txb[txq->q.read_ptr];
>  		iwl_tx_status(priv, tx_info->skb[0]);
> +
> +		if (tx_info->skb[0]) {
> +			hdr = (struct ieee80211_hdr *)
> +				((struct sk_buff *)tx_info->skb[0])->data;

Not needed cast.

> +			if (hdr && ieee80211_is_data_qos(hdr->frame_control))
> +				nfreed++;
> +		}

I think additional line is needed to make things work. Something like below
should work, but I have no time to test it today.

diff --git a/drivers/net/wireless/iwlwifi/iwl-5000.c b/drivers/net/wireless/iwlwifi/iwl-5000.c
index de45f30..6b516c4 100644
--- a/drivers/net/wireless/iwlwifi/iwl-5000.c
+++ b/drivers/net/wireless/iwlwifi/iwl-5000.c
@@ -1153,8 +1153,7 @@ static void iwl5000_rx_reply_tx(struct iwl_priv *priv,
 				   tx_resp->failure_frame);
 
 		freed = iwl_tx_queue_reclaim(priv, txq_id, index);
-		if (ieee80211_is_data_qos(tx_resp->frame_ctrl))
-			priv->stations[sta_id].tid[tid].tfds_in_queue -= freed;
+		priv->stations[sta_id].tid[tid].tfds_in_queue -= freed;
 
 		if (priv->mac80211_registered &&
 		    (iwl_queue_space(&txq->q) > txq->q.low_mark))
diff --git a/drivers/net/wireless/iwlwifi/iwl-tx.c b/drivers/net/wireless/iwlwifi/iwl-tx.c
index 281d318..c051f9f 100644
--- a/drivers/net/wireless/iwlwifi/iwl-tx.c
+++ b/drivers/net/wireless/iwlwifi/iwl-tx.c
@@ -1132,6 +1132,7 @@ int iwl_tx_queue_reclaim(struct iwl_priv *priv, int txq_id, int index)
 	struct iwl_queue *q = &txq->q;
 	struct iwl_tx_info *tx_info;
 	int nfreed = 0;
+	struct ieee80211_hdr *hdr;
 
 	if ((index >= q->n_bd) || (iwl_queue_used(q, index) == 0)) {
 		IWL_ERR(priv, "Read index for DMA queue txq id (%d), index %d, "
@@ -1146,13 +1147,18 @@ int iwl_tx_queue_reclaim(struct iwl_priv *priv, int txq_id, int index)
 
 		tx_info = &txq->txb[txq->q.read_ptr];
 		iwl_tx_status(priv, tx_info->skb[0]);
+
+		if (tx_info->skb[0]) {
+			hdr = (struct ieee80211_hdr *) tx_info->skb[0]->data;
+			if (hdr && ieee80211_is_data_qos(hdr->frame_control))
+				nfreed++;
+		}
 		tx_info->skb[0] = NULL;
 
 		if (priv->cfg->ops->lib->txq_inval_byte_cnt_tbl)
 			priv->cfg->ops->lib->txq_inval_byte_cnt_tbl(priv, txq);
 
 		priv->cfg->ops->lib->txq_free_tfd(priv, txq);
-		nfreed++;
 	}
 	return nfreed;
 }

Cheers
Stanislaw

Re: [PATCH 2/3] iwlwifi: sanity check before counting number of tfds can be free

[Guy, Wey-Yi]

Hi Stanislaw,

Thanks for the feedback.

On Fri, 2010-02-19 at 08:40 -0800, Stanislaw Gruszka wrote:
> Hi Reinette 
> On Thu, Feb 18, 2010 at 10:01:40PM -0800, Reinette Chatre wrote:
> > This fix starts to address
> > http://bugzilla.intellinuxwireless.org/show_bug.cgi?id=2160 . There are
> > more situations in which this problem (freed tfds below zero) can occur so
> > this bug is not closed yet.
> 
> Only starts ... ,hmm so these two patches does not fix any issue.
> 
> > diff --git a/drivers/net/wireless/iwlwifi/iwl-tx.c b/drivers/net/wireless/iwlwifi/iwl-tx.c
> > index 87ce2bd..9196d3f 100644
> > --- a/drivers/net/wireless/iwlwifi/iwl-tx.c
> > +++ b/drivers/net/wireless/iwlwifi/iwl-tx.c
> > @@ -1131,6 +1131,7 @@ int iwl_tx_queue_reclaim(struct iwl_priv *priv, int txq_id, int index)
> >  	struct iwl_queue *q = &txq->q;
> >  	struct iwl_tx_info *tx_info;
> >  	int nfreed = 0;
> > +	struct ieee80211_hdr *hdr;
> >  
> >  	if ((index >= q->n_bd) || (iwl_queue_used(q, index) == 0)) {
> >  		IWL_ERR(priv, "Read index for DMA queue txq id (%d), index %d, "
> > @@ -1145,13 +1146,19 @@ int iwl_tx_queue_reclaim(struct iwl_priv *priv, int txq_id, int index)
> >  
> >  		tx_info = &txq->txb[txq->q.read_ptr];
> >  		iwl_tx_status(priv, tx_info->skb[0]);
> > +
> > +		if (tx_info->skb[0]) {
> > +			hdr = (struct ieee80211_hdr *)
> > +				((struct sk_buff *)tx_info->skb[0])->data;
> 
> Not needed cast.
> 
you are right, no need to cast.

> > +			if (hdr && ieee80211_is_data_qos(hdr->frame_control))
> > +				nfreed++;
> > +		}
> 
> I think additional line is needed to make things work. Something like below
> should work, but I have no time to test it today.
> 
> diff --git a/drivers/net/wireless/iwlwifi/iwl-5000.c b/drivers/net/wireless/iwlwifi/iwl-5000.c
> index de45f30..6b516c4 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-5000.c
> +++ b/drivers/net/wireless/iwlwifi/iwl-5000.c
> @@ -1153,8 +1153,7 @@ static void iwl5000_rx_reply_tx(struct iwl_priv *priv,
>  				   tx_resp->failure_frame);
>  
>  		freed = iwl_tx_queue_reclaim(priv, txq_id, index);
> -		if (ieee80211_is_data_qos(tx_resp->frame_ctrl))
> -			priv->stations[sta_id].tid[tid].tfds_in_queue -= freed;
> +		priv->stations[sta_id].tid[tid].tfds_in_queue -= freed;

Correct, I miss this place, there is no need to check tx_resp here, and
I will prefer to do sanity check before decrement the tfds_in_queue to
prevent it go into negative range.

>  
>  		if (priv->mac80211_registered &&
>  		    (iwl_queue_space(&txq->q) > txq->q.low_mark))
> diff --git a/drivers/net/wireless/iwlwifi/iwl-tx.c b/drivers/net/wireless/iwlwifi/iwl-tx.c
> index 281d318..c051f9f 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-tx.c
> +++ b/drivers/net/wireless/iwlwifi/iwl-tx.c
> @@ -1132,6 +1132,7 @@ int iwl_tx_queue_reclaim(struct iwl_priv *priv, int txq_id, int index)
>  	struct iwl_queue *q = &txq->q;
>  	struct iwl_tx_info *tx_info;
>  	int nfreed = 0;
> +	struct ieee80211_hdr *hdr;
>  
>  	if ((index >= q->n_bd) || (iwl_queue_used(q, index) == 0)) {
>  		IWL_ERR(priv, "Read index for DMA queue txq id (%d), index %d, "
> @@ -1146,13 +1147,18 @@ int iwl_tx_queue_reclaim(struct iwl_priv *priv, int txq_id, int index)
>  
>  		tx_info = &txq->txb[txq->q.read_ptr];
>  		iwl_tx_status(priv, tx_info->skb[0]);
> +
> +		if (tx_info->skb[0]) {
> +			hdr = (struct ieee80211_hdr *) tx_info->skb[0]->data;
> +			if (hdr && ieee80211_is_data_qos(hdr->frame_control))
> +				nfreed++;
> +		}
>  		tx_info->skb[0] = NULL;
>  
>  		if (priv->cfg->ops->lib->txq_inval_byte_cnt_tbl)
>  			priv->cfg->ops->lib->txq_inval_byte_cnt_tbl(priv, txq);
>  
>  		priv->cfg->ops->lib->txq_free_tfd(priv, txq);
> -		nfreed++;
>  	}
>  	return nfreed;
>  }
> 
Thanks very much for checking, I will re-submit the patch to fix the
issues.

Wey

[PATCH 3/3] iwlwifi: error checking for number of tfds in queue

[Reinette Chatre]

From: Wey-Yi Guy <wey-yi.w.guy@intel.com>

When receive reply_tx and ready to decrement the count for number of
tfds in queue, do error checking to prevent error condition and
tfds_in_queue become negative number.

This fix starts to address
http://bugzilla.intellinuxwireless.org/show_bug.cgi?id=2160 by preventing
the bad situation from propagation when it does occur. This is needed while
the cause of the decrement error is still traced.

Signed-off-by: Wey-Yi Guy <wey-yi.w.guy@intel.com>
Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
CC: stable@kernel.org
---
 drivers/net/wireless/iwlwifi/iwl-4965.c |    2 +-
 drivers/net/wireless/iwlwifi/iwl-5000.c |    2 +-
 drivers/net/wireless/iwlwifi/iwl-core.h |    2 ++
 drivers/net/wireless/iwlwifi/iwl-tx.c   |   16 +++++++++++++++-
 4 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-4965.c b/drivers/net/wireless/iwlwifi/iwl-4965.c
index 9b4b8b5..3146281 100644
--- a/drivers/net/wireless/iwlwifi/iwl-4965.c
+++ b/drivers/net/wireless/iwlwifi/iwl-4965.c
@@ -2008,7 +2008,7 @@ 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);
-			priv->stations[sta_id].tid[tid].tfds_in_queue -= freed;
+			iwl_free_tfds_in_queue(priv, sta_id, tid, freed);
 
 			if (priv->mac80211_registered &&
 			    (iwl_queue_space(&txq->q) > txq->q.low_mark) &&
diff --git a/drivers/net/wireless/iwlwifi/iwl-5000.c b/drivers/net/wireless/iwlwifi/iwl-5000.c
index de45f30..845a348 100644
--- a/drivers/net/wireless/iwlwifi/iwl-5000.c
+++ b/drivers/net/wireless/iwlwifi/iwl-5000.c
@@ -1125,7 +1125,7 @@ static void iwl5000_rx_reply_tx(struct iwl_priv *priv,
 					scd_ssn , index, txq_id, txq->swq_id);
 
 			freed = iwl_tx_queue_reclaim(priv, txq_id, index);
-			priv->stations[sta_id].tid[tid].tfds_in_queue -= freed;
+			iwl_free_tfds_in_queue(priv, sta_id, tid, freed);
 
 			if (priv->mac80211_registered &&
 			    (iwl_queue_space(&txq->q) > txq->q.low_mark) &&
diff --git a/drivers/net/wireless/iwlwifi/iwl-core.h b/drivers/net/wireless/iwlwifi/iwl-core.h
index 675b7df..fe37d6a 100644
--- a/drivers/net/wireless/iwlwifi/iwl-core.h
+++ b/drivers/net/wireless/iwlwifi/iwl-core.h
@@ -446,6 +446,8 @@ void iwl_hw_txq_ctx_free(struct iwl_priv *priv);
 int iwl_hw_tx_queue_init(struct iwl_priv *priv,
 			 struct iwl_tx_queue *txq);
 int iwl_txq_update_write_ptr(struct iwl_priv *priv, struct iwl_tx_queue *txq);
+void iwl_free_tfds_in_queue(struct iwl_priv *priv,
+			    int sta_id, int tid, int freed);
 int iwl_tx_queue_init(struct iwl_priv *priv, struct iwl_tx_queue *txq,
 		      int slots_num, u32 txq_id);
 void iwl_tx_queue_free(struct iwl_priv *priv, int txq_id);
diff --git a/drivers/net/wireless/iwlwifi/iwl-tx.c b/drivers/net/wireless/iwlwifi/iwl-tx.c
index 9196d3f..036e662 100644
--- a/drivers/net/wireless/iwlwifi/iwl-tx.c
+++ b/drivers/net/wireless/iwlwifi/iwl-tx.c
@@ -120,6 +120,20 @@ int iwl_txq_update_write_ptr(struct iwl_priv *priv, struct iwl_tx_queue *txq)
 EXPORT_SYMBOL(iwl_txq_update_write_ptr);
 
 
+void iwl_free_tfds_in_queue(struct iwl_priv *priv,
+			    int sta_id, int tid, int freed)
+{
+	if (priv->stations[sta_id].tid[tid].tfds_in_queue >= freed)
+		priv->stations[sta_id].tid[tid].tfds_in_queue -= freed;
+	else {
+		IWL_ERR(priv, "free more than tfds_in_queue (%d:%d)\n",
+			priv->stations[sta_id].tid[tid].tfds_in_queue,
+			freed);
+		priv->stations[sta_id].tid[tid].tfds_in_queue = 0;
+	}
+}
+EXPORT_SYMBOL(iwl_free_tfds_in_queue);
+
 /**
  * iwl_tx_queue_free - Deallocate DMA queue.
  * @txq: Transmit queue to deallocate.
@@ -1566,7 +1580,7 @@ void iwl_rx_reply_compressed_ba(struct iwl_priv *priv,
 	if (txq->q.read_ptr != (ba_resp_scd_ssn & 0xff)) {
 		/* calculate mac80211 ampdu sw queue to wake */
 		int freed = iwl_tx_queue_reclaim(priv, scd_flow, index);
-		priv->stations[sta_id].tid[tid].tfds_in_queue -= freed;
+		iwl_free_tfds_in_queue(priv, sta_id, tid, freed);
 
 		if ((iwl_queue_space(&txq->q) > txq->q.low_mark) &&
 		    priv->mac80211_registered &&
-- 
1.6.3.3

Re: [PATCH 3/3] iwlwifi: error checking for number of tfds in queue

[John W. Linville]

On Thu, Feb 18, 2010 at 10:01:41PM -0800, Reinette Chatre wrote:
> From: Wey-Yi Guy <wey-yi.w.guy@intel.com>
> 
> When receive reply_tx and ready to decrement the count for number of
> tfds in queue, do error checking to prevent error condition and
> tfds_in_queue become negative number.
> 
> This fix starts to address
> http://bugzilla.intellinuxwireless.org/show_bug.cgi?id=2160 by preventing
> the bad situation from propagation when it does occur. This is needed while
> the cause of the decrement error is still traced.
> 
> Signed-off-by: Wey-Yi Guy <wey-yi.w.guy@intel.com>
> Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
> CC: stable@kernel.org

Since this apparently doesn't amount to a fix for the cited issue,
it doesn't seem worth pusing for 2.6.33 at the moment.  Should I take
it in wireless-next-2.6?  Or hold-out for a corrected version?

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

Re: [PATCH 3/3] iwlwifi: error checking for number of tfds in queue

[reinette chatre]

On Fri, 2010-02-19 at 12:34 -0800, John W. Linville wrote:
> On Thu, Feb 18, 2010 at 10:01:41PM -0800, Reinette Chatre wrote:
> > From: Wey-Yi Guy <wey-yi.w.guy@intel.com>
> > 
> > When receive reply_tx and ready to decrement the count for number of
> > tfds in queue, do error checking to prevent error condition and
> > tfds_in_queue become negative number.
> > 
> > This fix starts to address
> > http://bugzilla.intellinuxwireless.org/show_bug.cgi?id=2160 by preventing
> > the bad situation from propagation when it does occur. This is needed while
> > the cause of the decrement error is still traced.
> > 
> > Signed-off-by: Wey-Yi Guy <wey-yi.w.guy@intel.com>
> > Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
> > CC: stable@kernel.org
> 
> Since this apparently doesn't amount to a fix for the cited issue,
> it doesn't seem worth pusing for 2.6.33 at the moment.  Should I take
> it in wireless-next-2.6?  Or hold-out for a corrected version?

A version addressing Stanislaw's feedback is on its way. I apologize for
the delay. This patch does fix one instance of the issue and we would
like this fix to be included.

Reinette

[PATCH v2 3/3] iwlwifi: error checking for number of tfds in queue

[reinette chatre]

From: Wey-Yi Guy <wey-yi.w.guy@intel.com>

When receive reply_tx and ready to decrement the count for number of
tfds in queue, do error checking to prevent error condition and
tfds_in_queue become negative number.

Signed-off-by: Wey-Yi Guy <wey-yi.w.guy@intel.com>
Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
CC: stable@kernel.org
---
v2: ensure safe decrement is always done, one instance was missed in v1

 drivers/net/wireless/iwlwifi/iwl-4965.c |    2 +-
 drivers/net/wireless/iwlwifi/iwl-5000.c |    4 ++--
 drivers/net/wireless/iwlwifi/iwl-core.h |    2 ++
 drivers/net/wireless/iwlwifi/iwl-tx.c   |   16 +++++++++++++++-
 4 files changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-4965.c b/drivers/net/wireless/iwlwifi/iwl-4965.c
index 9b4b8b5..3146281 100644
--- a/drivers/net/wireless/iwlwifi/iwl-4965.c
+++ b/drivers/net/wireless/iwlwifi/iwl-4965.c
@@ -2008,7 +2008,7 @@ 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);
-			priv->stations[sta_id].tid[tid].tfds_in_queue -= freed;
+			iwl_free_tfds_in_queue(priv, sta_id, tid, freed);
 
 			if (priv->mac80211_registered &&
 			    (iwl_queue_space(&txq->q) > txq->q.low_mark) &&
diff --git a/drivers/net/wireless/iwlwifi/iwl-5000.c b/drivers/net/wireless/iwlwifi/iwl-5000.c
index de45f30..f27c514 100644
--- a/drivers/net/wireless/iwlwifi/iwl-5000.c
+++ b/drivers/net/wireless/iwlwifi/iwl-5000.c
@@ -1125,7 +1125,7 @@ static void iwl5000_rx_reply_tx(struct iwl_priv *priv,
 					scd_ssn , index, txq_id, txq->swq_id);
 
 			freed = iwl_tx_queue_reclaim(priv, txq_id, index);
-			priv->stations[sta_id].tid[tid].tfds_in_queue -= freed;
+			iwl_free_tfds_in_queue(priv, sta_id, tid, freed);
 
 			if (priv->mac80211_registered &&
 			    (iwl_queue_space(&txq->q) > txq->q.low_mark) &&
@@ -1154,7 +1154,7 @@ static void iwl5000_rx_reply_tx(struct iwl_priv *priv,
 
 		freed = iwl_tx_queue_reclaim(priv, txq_id, index);
 		if (ieee80211_is_data_qos(tx_resp->frame_ctrl))
-			priv->stations[sta_id].tid[tid].tfds_in_queue -= freed;
+			iwl_free_tfds_in_queue(priv, sta_id, tid, freed);
 
 		if (priv->mac80211_registered &&
 		    (iwl_queue_space(&txq->q) > txq->q.low_mark))
diff --git a/drivers/net/wireless/iwlwifi/iwl-core.h b/drivers/net/wireless/iwlwifi/iwl-core.h
index 675b7df..fe37d6a 100644
--- a/drivers/net/wireless/iwlwifi/iwl-core.h
+++ b/drivers/net/wireless/iwlwifi/iwl-core.h
@@ -446,6 +446,8 @@ void iwl_hw_txq_ctx_free(struct iwl_priv *priv);
 int iwl_hw_tx_queue_init(struct iwl_priv *priv,
 			 struct iwl_tx_queue *txq);
 int iwl_txq_update_write_ptr(struct iwl_priv *priv, struct iwl_tx_queue *txq);
+void iwl_free_tfds_in_queue(struct iwl_priv *priv,
+			    int sta_id, int tid, int freed);
 int iwl_tx_queue_init(struct iwl_priv *priv, struct iwl_tx_queue *txq,
 		      int slots_num, u32 txq_id);
 void iwl_tx_queue_free(struct iwl_priv *priv, int txq_id);
diff --git a/drivers/net/wireless/iwlwifi/iwl-tx.c b/drivers/net/wireless/iwlwifi/iwl-tx.c
index 87ce2bd..72136c8 100644
--- a/drivers/net/wireless/iwlwifi/iwl-tx.c
+++ b/drivers/net/wireless/iwlwifi/iwl-tx.c
@@ -120,6 +120,20 @@ int iwl_txq_update_write_ptr(struct iwl_priv *priv, struct iwl_tx_queue *txq)
 EXPORT_SYMBOL(iwl_txq_update_write_ptr);
 
 
+void iwl_free_tfds_in_queue(struct iwl_priv *priv,
+			    int sta_id, int tid, int freed)
+{
+	if (priv->stations[sta_id].tid[tid].tfds_in_queue >= freed)
+		priv->stations[sta_id].tid[tid].tfds_in_queue -= freed;
+	else {
+		IWL_ERR(priv, "free more than tfds_in_queue (%u:%d)\n",
+			priv->stations[sta_id].tid[tid].tfds_in_queue,
+			freed);
+		priv->stations[sta_id].tid[tid].tfds_in_queue = 0;
+	}
+}
+EXPORT_SYMBOL(iwl_free_tfds_in_queue);
+
 /**
  * iwl_tx_queue_free - Deallocate DMA queue.
  * @txq: Transmit queue to deallocate.
@@ -1559,7 +1573,7 @@ void iwl_rx_reply_compressed_ba(struct iwl_priv *priv,
 	if (txq->q.read_ptr != (ba_resp_scd_ssn & 0xff)) {
 		/* calculate mac80211 ampdu sw queue to wake */
 		int freed = iwl_tx_queue_reclaim(priv, scd_flow, index);
-		priv->stations[sta_id].tid[tid].tfds_in_queue -= freed;
+		iwl_free_tfds_in_queue(priv, sta_id, tid, freed);
 
 		if ((iwl_queue_space(&txq->q) > txq->q.low_mark) &&
 		    priv->mac80211_registered &&
-- 
1.6.3.3

Re: [PATCH v2 3/3] iwlwifi: error checking for number of tfds in queue

[reinette chatre]

On Fri, 2010-02-19 at 15:24 -0800, reinette chatre wrote:
> From: Wey-Yi Guy <wey-yi.w.guy@intel.com>
> 
> When receive reply_tx and ready to decrement the count for number of
> tfds in queue, do error checking to prevent error condition and
> tfds_in_queue become negative number.
> 
> Signed-off-by: Wey-Yi Guy <wey-yi.w.guy@intel.com>
> Signed-off-by: Reinette Chatre <reinette.chatre@intel.com>
> CC: stable@kernel.org
> ---
> v2: ensure safe decrement is always done, one instance was missed in v1

I will resend this patch as part of a new series. Sorry for the noise.

Reinette