All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Williams <dcbw@redhat.com>
To: "Chatre, Reinette" <reinette.chatre@intel.com>
Cc: Sebastian Siewior <ipw+2200@ml.breakpoint.cc>,
	"Zhu, Yi" <yi.zhu@intel.com>,
	James Ketrenos <jketreno@linux.intel.com>,
	linux-wireless@vger.kernel.org,
	ipw2100-devel@lists.sourceforge.net
Subject: RE: ipw2200 stalls on high load
Date: Mon, 04 Feb 2008 07:37:29 -0500	[thread overview]
Message-ID: <1202128649.10632.4.camel@localhost.localdomain> (raw)
In-Reply-To: <D936D925018D154694D8A362EEB08920038F4382@orsmsx416.amr.corp.intel.com>

On Fri, 2008-02-01 at 14:29 -0800, Chatre, Reinette wrote:
> On , Sebastian Siewior wrote:
> 
> > * Chatre, Reinette | 2008-01-28 10:40:16 [-0800]:
> > 
> >> On ,Sebastian Siewior wrote:
> >> 
> >>> Hello,
> >>> 
> >>> my wireless connection stalls after like 5 seconds once the
> >>> downloading performance passes around 600 KiB/sec. I noticed in
> >>> dmesg the following:
> >>>> ipw2200: Firmware error detected.  Restarting.
> >> 
> >> Could you please submit a bug at http://www.bughost.org to enable us
> >> to track this problem?
> > Sure. #1487 looks very close. The firmware version seems to be
> > different as well as the "the driver messsage". Do you want me to
> > open a new one or should I follow-up?
> 
> This bug may be similar to the bug that was fixed for the 3945 and 4965.
> See the patch "iwlwifi: fix ucode assertion for RX queue overrun" for
> these drivers. We need somebody to port this change to the ipw2200.
> Recently there has also been talk about these issues on the
> ipw3945-devel mailing list.

Something like the following?  Turns out the rxq->processed isn't really
used that much, and 3945/4965 don't use that field at all (but use
->read exclusively instead).  And since it appears that the replenish
function is simpler in the 2200, it doesn't need to be split like
3945/4965.  I haven't been able to stress my 2200 enough to trigger the
new codepath though.


diff --git a/drivers/net/wireless/ipw2200.c b/drivers/net/wireless/ipw2200.c
index 3e6ad7b..8856eea 100644
--- a/drivers/net/wireless/ipw2200.c
+++ b/drivers/net/wireless/ipw2200.c
@@ -3365,7 +3365,6 @@ static void ipw_rx_queue_reset(struct ipw_priv *priv,
 	/* Set us so that we have processed and used all buffers, but have
 	 * not restocked the Rx queue with fresh buffers */
 	rxq->read = rxq->write = 0;
-	rxq->processed = RX_QUEUE_SIZE - 1;
 	rxq->free_count = 0;
 	spin_unlock_irqrestore(&rxq->lock, flags);
 }
@@ -3607,7 +3606,22 @@ static int ipw_load(struct ipw_priv *priv)
  * Driver allocates buffers of this size for Rx
  */
 
-static inline int ipw_queue_space(const struct clx2_queue *q)
+/**
+ * ipw_rx_queue_space - Return number of free slots available in queue.
+ */
+static int ipw_rx_queue_space(const struct ipw_rx_queue *q)
+{
+	int s = q->read - q->write;
+	if (s <= 0)
+		s += RX_QUEUE_SIZE;
+	/* keep some buffer to not confuse full and empty queue */
+	s -= 2;
+	if (s < 0)
+		s = 0;
+	return s;
+}
+
+static inline int ipw_tx_queue_space(const struct clx2_queue *q)
 {
 	int s = q->last_used - q->first_empty;
 	if (s <= 0)
@@ -4947,7 +4961,7 @@ static int ipw_queue_tx_reclaim(struct ipw_priv *priv,
 		priv->tx_packets++;
 	}
       done:
-	if ((ipw_queue_space(q) > q->low_mark) &&
+	if ((ipw_tx_queue_space(q) > q->low_mark) &&
 	    (qindex >= 0) &&
 	    (priv->status & STATUS_ASSOCIATED) && netif_running(priv->net_dev))
 		netif_wake_queue(priv->net_dev);
@@ -4965,7 +4979,7 @@ static int ipw_queue_tx_hcmd(struct ipw_priv *priv, int hcmd, void *buf,
 	struct clx2_queue *q = &txq->q;
 	struct tfd_frame *tfd;
 
-	if (ipw_queue_space(q) < (sync ? 1 : 2)) {
+	if (ipw_tx_queue_space(q) < (sync ? 1 : 2)) {
 		IPW_ERROR("No space for Tx\n");
 		return -EBUSY;
 	}
@@ -5070,7 +5084,7 @@ static void ipw_rx_queue_restock(struct ipw_priv *priv)
 
 	spin_lock_irqsave(&rxq->lock, flags);
 	write = rxq->write;
-	while ((rxq->write != rxq->processed) && (rxq->free_count)) {
+	while ((ipw_rx_queue_space(rxq) > 0) && (rxq->free_count)) {
 		element = rxq->rx_free.next;
 		rxb = list_entry(element, struct ipw_rx_mem_buffer, list);
 		list_del(element);
@@ -5187,7 +5201,6 @@ static struct ipw_rx_queue *ipw_rx_queue_alloc(struct ipw_priv *priv)
 	/* Set us so that we have processed and used all buffers, but have
 	 * not restocked the Rx queue with fresh buffers */
 	rxq->read = rxq->write = 0;
-	rxq->processed = RX_QUEUE_SIZE - 1;
 	rxq->free_count = 0;
 
 	return rxq;
@@ -8223,13 +8236,18 @@ static void ipw_rx(struct ipw_priv *priv)
 	struct ieee80211_hdr_4addr *header;
 	u32 r, w, i;
 	u8 network_packet;
+	u8 fill_rx = 0;
+	u32 count = 0;
 	DECLARE_MAC_BUF(mac);
 	DECLARE_MAC_BUF(mac2);
 	DECLARE_MAC_BUF(mac3);
 
 	r = ipw_read32(priv, IPW_RX_READ_INDEX);
 	w = ipw_read32(priv, IPW_RX_WRITE_INDEX);
-	i = (priv->rxq->processed + 1) % RX_QUEUE_SIZE;
+	i = priv->rxq->read;
+
+	if (ipw_rx_queue_space (priv->rxq) > (RX_QUEUE_SIZE / 2))
+		fill_rx = 1;
 
 	while (i != r) {
 		rxb = priv->rxq->queue[i];
@@ -8404,11 +8422,21 @@ static void ipw_rx(struct ipw_priv *priv)
 		list_add_tail(&rxb->list, &priv->rxq->rx_used);
 
 		i = (i + 1) % RX_QUEUE_SIZE;
+
+		/* If there are a lot of unsued frames, restock the Rx queue
+		 * so the ucode won't assert */
+		if (fill_rx) {
+			count++;
+			if (count >= 8) {
+				priv->rxq->read = i;
+				ipw_rx_queue_replenish(priv);
+				count = 0;
+			}
+		}
 	}
 
 	/* Backtrack one entry */
-	priv->rxq->processed = (i ? i : RX_QUEUE_SIZE) - 1;
-
+	priv->rxq->read = i;
 	ipw_rx_queue_restock(priv);
 }
 
@@ -10336,7 +10364,7 @@ static int ipw_tx_skb(struct ipw_priv *priv, struct ieee80211_txb *txb,
 	q->first_empty = ipw_queue_inc_wrap(q->first_empty, q->n_bd);
 	ipw_write32(priv, q->reg_w, q->first_empty);
 
-	if (ipw_queue_space(q) < q->high_mark)
+	if (ipw_tx_queue_space(q) < q->high_mark)
 		netif_stop_queue(priv->net_dev);
 
 	return NETDEV_TX_OK;
@@ -10357,7 +10385,7 @@ static int ipw_net_is_queue_full(struct net_device *dev, int pri)
 	struct clx2_tx_queue *txq = &priv->txq[0];
 #endif				/* CONFIG_IPW2200_QOS */
 
-	if (ipw_queue_space(&txq->q) < txq->q.high_mark)
+	if (ipw_tx_queue_space(&txq->q) < txq->q.high_mark)
 		return 1;
 
 	return 0;
diff --git a/drivers/net/wireless/ipw2200.h b/drivers/net/wireless/ipw2200.h
index fdc187e..72884e2 100644
--- a/drivers/net/wireless/ipw2200.h
+++ b/drivers/net/wireless/ipw2200.h
@@ -719,7 +719,6 @@ struct ipw_rx_mem_buffer {
 struct ipw_rx_queue {
 	struct ipw_rx_mem_buffer pool[RX_QUEUE_SIZE + RX_FREE_BUFFERS];
 	struct ipw_rx_mem_buffer *queue[RX_QUEUE_SIZE];
-	u32 processed;		/* Internal index to last handled Rx packet */
 	u32 read;		/* Shared index to newest available Rx buffer */
 	u32 write;		/* Shared index to oldest written Rx packet */
 	u32 free_count;		/* Number of pre-allocated buffers in rx_free */


  reply	other threads:[~2008-02-04 12:37 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-01-26 13:29 ipw2200 stalls on high load Sebastian Siewior
2008-01-28 18:40 ` Chatre, Reinette
2008-01-30 22:57   ` Sebastian Siewior
2008-02-01 22:29     ` Chatre, Reinette
2008-02-04 12:37       ` Dan Williams [this message]
2008-02-04 18:23         ` Chatre, Reinette
2008-02-04 22:45           ` Sebastian Siewior
2008-02-04 23:24             ` Dan Williams
2008-02-05  8:35               ` Sebastian Siewior
2008-02-05 15:09                 ` Dan Williams
2008-02-05 16:50                   ` [Ipw2100-devel] " Cahill, Ben M
2008-02-05 23:53                   ` Sebastian Siewior
2008-02-06  4:56                     ` Dan Williams
2008-02-08  7:50                       ` Sebastian Siewior
2008-01-28 18:53 ` [Ipw2100-devel] " Cahill, Ben M
2008-01-28 22:54   ` Sebastian Siewior
2008-01-30 22:48   ` Sebastian Siewior

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1202128649.10632.4.camel@localhost.localdomain \
    --to=dcbw@redhat.com \
    --cc=ipw+2200@ml.breakpoint.cc \
    --cc=ipw2100-devel@lists.sourceforge.net \
    --cc=jketreno@linux.intel.com \
    --cc=linux-wireless@vger.kernel.org \
    --cc=reinette.chatre@intel.com \
    --cc=yi.zhu@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.