[PATCH 97/99] cw1200: Avoid sleeping on TX lock for more than 1.5 sec.

Dmitry Tarnyagin dmitry.tarnyagin at stericsson.com
Wed Feb 29 14:15:44 UTC 2012


The patch protects against possible timeout in system suspend
code. It implements timestamps for TX frames, and do not wait more
than 1.5 sec in total since the oldest frame is sent to firmware.
Firmware has to reply within this interval, otherwise driver commits
suicide by terminating device communication thread.

ST-Ericsson ID: 418642

Change-Id: Ifb5abf1147cb48665ffebf56254ce7a21c9a7d10
Signed-off-by: Dmitry Tarnyagin <dmitry.tarnyagin at stericsson.com>
---
 drivers/staging/cw1200/bh.c    |    4 +-
 drivers/staging/cw1200/queue.c |   28 ++++++++++++++--
 drivers/staging/cw1200/queue.h |    3 ++
 drivers/staging/cw1200/wsm.c   |   67 +++++++++++++++++++++++++++++-----------
 drivers/staging/cw1200/wsm.h   |    2 +-
 5 files changed, 79 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/cw1200/bh.c b/drivers/staging/cw1200/bh.c
index fe8ecc1..22b43a5 100644
--- a/drivers/staging/cw1200/bh.c
+++ b/drivers/staging/cw1200/bh.c
@@ -294,10 +294,10 @@ static int cw1200_bh(void *arg)
 				term = atomic_xchg(&priv->bh_term, 0);
 				suspend = pending_tx ?
 					0 : atomic_read(&priv->bh_suspend);
-				(rx || tx || term || suspend);
+				(rx || tx || term || suspend || priv->bh_error);
 			}), status);
 
-		if (status < 0 || term)
+		if (status < 0 || term || priv->bh_error)
 			break;
 
 		if (!status && priv->hw_bufs_used) {
diff --git a/drivers/staging/cw1200/queue.c b/drivers/staging/cw1200/queue.c
index ba2cc43..014e6b2 100644
--- a/drivers/staging/cw1200/queue.c
+++ b/drivers/staging/cw1200/queue.c
@@ -20,7 +20,8 @@
 	struct list_head	head;
 	struct sk_buff		*skb;
 	u32			packetID;
-	unsigned long		timestamp;
+	unsigned long		queue_timestamp;
+	unsigned long		xmit_timestamp;
 	struct cw1200_txpriv	txpriv;
 	u8			generation;
 };
@@ -102,7 +103,7 @@ static void __cw1200_queue_gc(struct cw1200_queue *queue,
 	while (!list_empty(&queue->queue)) {
 		item = list_first_entry(
 			&queue->queue, struct cw1200_queue_item, head);
-		if (jiffies - item->timestamp < queue->ttl)
+		if (jiffies - item->queue_timestamp < queue->ttl)
 			break;
 		--queue->num_queued;
 		--queue->link_map_cache[item->txpriv.link_id];
@@ -126,7 +127,7 @@ static void __cw1200_queue_gc(struct cw1200_queue *queue,
 			if (unlock)
 				__cw1200_queue_unlock(queue);
 		} else {
-			unsigned long tmo = item->timestamp + queue->ttl;
+			unsigned long tmo = item->queue_timestamp + queue->ttl;
 			mod_timer(&queue->gc, tmo);
 			cw1200_pm_stay_awake(&stats->priv->pm_state,
 					tmo - jiffies);
@@ -309,7 +310,7 @@ int cw1200_queue_put(struct cw1200_queue *queue,
 		item->packetID = cw1200_queue_make_packet_id(
 			queue->generation, queue->queue_id,
 			item->generation, item - queue->pool);
-		item->timestamp = jiffies;
+		item->queue_timestamp = jiffies;
 
 		++queue->num_queued;
 		++queue->link_map_cache[txpriv->link_id];
@@ -364,6 +365,7 @@ int cw1200_queue_get(struct cw1200_queue *queue,
 		list_move_tail(&item->head, &queue->pending);
 		++queue->num_pending;
 		--queue->link_map_cache[item->txpriv.link_id];
+		item->xmit_timestamp = jiffies;
 
 		spin_lock_bh(&stats->lock);
 		--stats->num_queued;
@@ -539,6 +541,24 @@ void cw1200_queue_unlock(struct cw1200_queue *queue)
 	spin_unlock_bh(&queue->lock);
 }
 
+bool cw1200_queue_get_xmit_timestamp(struct cw1200_queue *queue,
+				     unsigned long *timestamp)
+{
+	struct cw1200_queue_item *item;
+	bool ret;
+
+	spin_lock_bh(&queue->lock);
+	ret = !list_empty(&queue->pending);
+	if (ret) {
+		list_for_each_entry(item, &queue->pending, head) {
+			if (time_before(item->xmit_timestamp, *timestamp))
+				*timestamp = item->xmit_timestamp;
+		}
+	}
+	spin_unlock_bh(&queue->lock);
+	return ret;
+}
+
 bool cw1200_queue_stats_is_empty(struct cw1200_queue_stats *stats,
 				 u32 link_id_map)
 {
diff --git a/drivers/staging/cw1200/queue.h b/drivers/staging/cw1200/queue.h
index 2403b25..aa9a7f9 100644
--- a/drivers/staging/cw1200/queue.h
+++ b/drivers/staging/cw1200/queue.h
@@ -96,6 +96,9 @@ int cw1200_queue_get_skb(struct cw1200_queue *queue, u32 packetID,
 			 const struct cw1200_txpriv **txpriv);
 void cw1200_queue_lock(struct cw1200_queue *queue);
 void cw1200_queue_unlock(struct cw1200_queue *queue);
+bool cw1200_queue_get_xmit_timestamp(struct cw1200_queue *queue,
+				     unsigned long *timestamp);
+
 
 bool cw1200_queue_stats_is_empty(struct cw1200_queue_stats *stats,
 				 u32 link_id_map);
diff --git a/drivers/staging/cw1200/wsm.c b/drivers/staging/cw1200/wsm.c
index 21df2bf..d1ea95c 100644
--- a/drivers/staging/cw1200/wsm.c
+++ b/drivers/staging/cw1200/wsm.c
@@ -33,7 +33,7 @@
 #define WSM_CMD_JOIN_TIMEOUT	(7 * HZ) /* Join timeout is 5 sec. in FW   */
 #define WSM_CMD_START_TIMEOUT	(7 * HZ)
 #define WSM_CMD_RESET_TIMEOUT	(3 * HZ) /* 2 sec. timeout was observed.   */
-#define WSM_CMD_LAST_CHANCE_TIMEOUT (10 * HZ)
+#define WSM_CMD_LAST_CHANCE_TIMEOUT (HZ * 3 / 2)
 
 #define WSM_SKIP(buf, size)						\
 	do {								\
@@ -1082,6 +1082,10 @@ int wsm_cmd_send(struct cw1200_common *priv,
 					priv->wsm_cmd_wq, priv->wsm_cmd.done,
 					WSM_CMD_LAST_CHANCE_TIMEOUT) <= 0);
 		}
+
+		/* Kill BH thread to report the error to the top layer. */
+		priv->bh_error = 1;
+		wake_up(&priv->bh_wq);
 		ret = -ETIMEDOUT;
 	} else {
 		spin_lock(&priv->wsm_cmd.lock);
@@ -1100,15 +1104,8 @@ void wsm_lock_tx(struct cw1200_common *priv)
 {
 	wsm_cmd_lock(priv);
 	if (atomic_add_return(1, &priv->tx_lock) == 1) {
-		if (priv->bh_error) {
-			wsm_printk(KERN_ERR "fatal error occured, "
-					"could not take lock\n");
-		} else {
-			WARN_ON(wait_event_timeout(priv->bh_evt_wq,
-				!priv->hw_bufs_used,
-				WSM_CMD_LAST_CHANCE_TIMEOUT) <= 0);
+		if (wsm_flush_tx(priv))
 			wsm_printk(KERN_DEBUG "[WSM] TX is locked.\n");
-		}
 	}
 	wsm_cmd_unlock(priv);
 }
@@ -1116,18 +1113,52 @@ void wsm_lock_tx(struct cw1200_common *priv)
 void wsm_lock_tx_async(struct cw1200_common *priv)
 {
 	if (atomic_add_return(1, &priv->tx_lock) == 1)
-		wsm_printk(KERN_DEBUG "[WSM] TX is locked.\n");
+		wsm_printk(KERN_DEBUG "[WSM] TX is locked (async).\n");
 }
 
-void wsm_flush_tx(struct cw1200_common *priv)
+bool wsm_flush_tx(struct cw1200_common *priv)
 {
-	if (priv->bh_error)
-		wsm_printk(KERN_ERR "fatal error occured, will not flush\n");
-	else {
-		BUG_ON(!atomic_read(&priv->tx_lock));
-		WARN_ON(wait_event_timeout(priv->bh_evt_wq,
-			!priv->hw_bufs_used,
-			WSM_CMD_LAST_CHANCE_TIMEOUT) <= 0);
+	unsigned long timestamp = jiffies;
+	bool pending = false;
+	long timeout;
+	int i;
+
+	/* Flush must be called with TX lock held. */
+	BUG_ON(!atomic_read(&priv->tx_lock));
+
+	/* First check if we really need to do something.
+	 * It is safe to use unprotected access, as hw_bufs_used
+	 * can only decrements. */
+	if (!priv->hw_bufs_used)
+		return true;
+
+	if (priv->bh_error) {
+		/* In case of failure do not wait for magic. */
+		wsm_printk(KERN_ERR "[WSM] Fatal error occured, "
+				"will not flush TX.\n");
+		return false;
+	} else {
+		/* Get a timestamp of "oldest" frame */
+		for (i = 0; i < 4; ++i)
+			pending |= cw1200_queue_get_xmit_timestamp(
+					&priv->tx_queue[i],
+					&timestamp);
+		/* It is allowed to lock TX with only a command in the pipe. */
+		if (!pending)
+			return true;
+
+		timeout = timestamp + WSM_CMD_LAST_CHANCE_TIMEOUT - jiffies;
+		if (timeout < 0 || wait_event_timeout(priv->bh_evt_wq,
+				!priv->hw_bufs_used,
+				timeout) <= 0) {
+			/* Hmmm... Not good. Frame had stuck in firmware. */
+			priv->bh_error = 1;
+			wake_up(&priv->bh_wq);
+			return false;
+		}
+
+		/* Ok, everything is flushed. */
+		return true;
 	}
 }
 
diff --git a/drivers/staging/cw1200/wsm.h b/drivers/staging/cw1200/wsm.h
index 9c66c1a..fb15ef5 100644
--- a/drivers/staging/cw1200/wsm.h
+++ b/drivers/staging/cw1200/wsm.h
@@ -1765,7 +1765,7 @@ static inline int wsm_set_override_internal_txrate(struct cw1200_common *priv,
 
 void wsm_lock_tx(struct cw1200_common *priv);
 void wsm_lock_tx_async(struct cw1200_common *priv);
-void wsm_flush_tx(struct cw1200_common *priv);
+bool wsm_flush_tx(struct cw1200_common *priv);
 void wsm_unlock_tx(struct cw1200_common *priv);
 
 /* ******************************************************************** */
-- 
1.7.8.3



More information about the kernel mailing list