From 3598e1774c94e55c71b585340e7dc4538f310e3f Mon Sep 17 00:00:00 2001 From: Stanislaw Gruszka Date: Thu, 31 Mar 2011 17:36:26 +0200 Subject: iwlwifi: fix enqueue hcmd race conditions We mark command as huge by using meta->flags from other (non huge) command, but flags can be possibly overridden, when non huge command is enqueued, what can lead to: WARNING: at lib/dma-debug.c:696 dma_debug_device_change+0x1a3/0x1f0() DMA-API: device driver has pending DMA allocations while released from device [count=1] To fix introduce additional CMD_MAPPED to mark command as mapped and serialize iwl_enqueue_hcmd() with iwl_tx_cmd_complete() using hcmd_lock. Serialization will also fix possible race conditions, because q->read_ptr, q->write_ptr are modified/used in parallel. On the way fix whitespace. Signed-off-by: Stanislaw Gruszka Acked-by: Wey-Yi Guy Signed-off-by: John W. Linville --- drivers/net/wireless/iwlwifi/iwl-tx.c | 62 +++++++++++++++++++---------------- 1 file changed, 33 insertions(+), 29 deletions(-) (limited to 'drivers/net/wireless/iwlwifi/iwl-tx.c') diff --git a/drivers/net/wireless/iwlwifi/iwl-tx.c b/drivers/net/wireless/iwlwifi/iwl-tx.c index 277c9175dcf6..39a4180ee854 100644 --- a/drivers/net/wireless/iwlwifi/iwl-tx.c +++ b/drivers/net/wireless/iwlwifi/iwl-tx.c @@ -149,32 +149,31 @@ void iwl_cmd_queue_unmap(struct iwl_priv *priv) struct iwl_tx_queue *txq = &priv->txq[priv->cmd_queue]; struct iwl_queue *q = &txq->q; int i; - bool huge = false; if (q->n_bd == 0) return; while (q->read_ptr != q->write_ptr) { - /* we have no way to tell if it is a huge cmd ATM */ i = get_cmd_index(q, q->read_ptr, 0); - if (txq->meta[i].flags & CMD_SIZE_HUGE) - huge = true; - else + if (txq->meta[i].flags & CMD_MAPPED) { pci_unmap_single(priv->pci_dev, dma_unmap_addr(&txq->meta[i], mapping), dma_unmap_len(&txq->meta[i], len), PCI_DMA_BIDIRECTIONAL); + txq->meta[i].flags = 0; + } - q->read_ptr = iwl_queue_inc_wrap(q->read_ptr, q->n_bd); + q->read_ptr = iwl_queue_inc_wrap(q->read_ptr, q->n_bd); } - if (huge) { - i = q->n_window; + i = q->n_window; + if (txq->meta[i].flags & CMD_MAPPED) { pci_unmap_single(priv->pci_dev, dma_unmap_addr(&txq->meta[i], mapping), dma_unmap_len(&txq->meta[i], len), PCI_DMA_BIDIRECTIONAL); + txq->meta[i].flags = 0; } } @@ -463,7 +462,11 @@ int iwl_enqueue_hcmd(struct iwl_priv *priv, struct iwl_host_cmd *cmd) return -EIO; } + spin_lock_irqsave(&priv->hcmd_lock, flags); + if (iwl_queue_space(q) < ((cmd->flags & CMD_ASYNC) ? 2 : 1)) { + spin_unlock_irqrestore(&priv->hcmd_lock, flags); + IWL_ERR(priv, "No space in command queue\n"); if (priv->cfg->ops->lib->tt_ops.ct_kill_check) { is_ct_kill = @@ -476,22 +479,17 @@ int iwl_enqueue_hcmd(struct iwl_priv *priv, struct iwl_host_cmd *cmd) return -ENOSPC; } - spin_lock_irqsave(&priv->hcmd_lock, flags); - - /* If this is a huge cmd, mark the huge flag also on the meta.flags - * of the _original_ cmd. This is used for DMA mapping clean up. - */ - if (cmd->flags & CMD_SIZE_HUGE) { - idx = get_cmd_index(q, q->write_ptr, 0); - txq->meta[idx].flags = CMD_SIZE_HUGE; - } - idx = get_cmd_index(q, q->write_ptr, cmd->flags & CMD_SIZE_HUGE); out_cmd = txq->cmd[idx]; out_meta = &txq->meta[idx]; + if (WARN_ON(out_meta->flags & CMD_MAPPED)) { + spin_unlock_irqrestore(&priv->hcmd_lock, flags); + return -ENOSPC; + } + memset(out_meta, 0, sizeof(*out_meta)); /* re-initialize to NULL */ - out_meta->flags = cmd->flags; + out_meta->flags = cmd->flags | CMD_MAPPED; if (cmd->flags & CMD_WANT_SKB) out_meta->source = cmd; if (cmd->flags & CMD_ASYNC) @@ -609,6 +607,10 @@ void iwl_tx_cmd_complete(struct iwl_priv *priv, struct iwl_rx_mem_buffer *rxb) struct iwl_device_cmd *cmd; struct iwl_cmd_meta *meta; struct iwl_tx_queue *txq = &priv->txq[priv->cmd_queue]; + unsigned long flags; + void (*callback) (struct iwl_priv *priv, struct iwl_device_cmd *cmd, + struct iwl_rx_packet *pkt); + /* If a Tx command is being handled and it isn't in the actual * command queue then there a command routing bug has been introduced @@ -622,14 +624,8 @@ void iwl_tx_cmd_complete(struct iwl_priv *priv, struct iwl_rx_mem_buffer *rxb) return; } - /* If this is a huge cmd, clear the huge flag on the meta.flags - * of the _original_ cmd. So that iwl_cmd_queue_free won't unmap - * the DMA buffer for the scan (huge) command. - */ - if (huge) { - cmd_index = get_cmd_index(&txq->q, index, 0); - txq->meta[cmd_index].flags = 0; - } + spin_lock_irqsave(&priv->hcmd_lock, flags); + cmd_index = get_cmd_index(&txq->q, index, huge); cmd = txq->cmd[cmd_index]; meta = &txq->meta[cmd_index]; @@ -639,12 +635,13 @@ void iwl_tx_cmd_complete(struct iwl_priv *priv, struct iwl_rx_mem_buffer *rxb) dma_unmap_len(meta, len), PCI_DMA_BIDIRECTIONAL); + callback = NULL; /* Input error checking is done when commands are added to queue. */ if (meta->flags & CMD_WANT_SKB) { meta->source->reply_page = (unsigned long)rxb_addr(rxb); rxb->page = NULL; - } else if (meta->callback) - meta->callback(priv, cmd, pkt); + } else + callback = meta->callback; iwl_hcmd_queue_reclaim(priv, txq_id, index, cmd_index); @@ -654,5 +651,12 @@ void iwl_tx_cmd_complete(struct iwl_priv *priv, struct iwl_rx_mem_buffer *rxb) get_cmd_string(cmd->hdr.cmd)); wake_up_interruptible(&priv->wait_command_queue); } + + /* Mark as unmapped */ meta->flags = 0; + + spin_unlock_irqrestore(&priv->hcmd_lock, flags); + + if (callback) + callback(priv, cmd, pkt); } -- cgit From e649437fd6e2bae6f7b8a36a302a1ec4faa5d906 Mon Sep 17 00:00:00 2001 From: Johannes Berg Date: Tue, 5 Apr 2011 09:41:58 -0700 Subject: iwlagn: centralize and fix ucode restart The ucode restart has to take into account a number of things, like clearing the HCMD_ACTIVE and other status bits, and waking up the wait_command_queue. Currently, however, there are a number of places that neither do that, nor actually set the FW error bit that leads to proper restart handling, which means that in those cases things will probably just hang completely. To clean this up, make all ucode restart go through a single function, except for the cases where it's called during firmware loading. Also fix a bug in wimax coexist restart avoidance, it needs to first clear the status bits (and it has to clear the HCMD_ACTIVE one as well) and then wake up anything waiting on wait_command_queue. Signed-off-by: Johannes Berg Signed-off-by: Wey-Yi Guy Signed-off-by: John W. Linville --- drivers/net/wireless/iwlwifi/iwl-tx.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/net/wireless/iwlwifi/iwl-tx.c') diff --git a/drivers/net/wireless/iwlwifi/iwl-tx.c b/drivers/net/wireless/iwlwifi/iwl-tx.c index 39a4180ee854..fa81df22a103 100644 --- a/drivers/net/wireless/iwlwifi/iwl-tx.c +++ b/drivers/net/wireless/iwlwifi/iwl-tx.c @@ -474,7 +474,7 @@ int iwl_enqueue_hcmd(struct iwl_priv *priv, struct iwl_host_cmd *cmd) } if (!is_ct_kill) { IWL_ERR(priv, "Restarting adapter due to queue full\n"); - queue_work(priv->workqueue, &priv->restart); + iwlagn_fw_error(priv, false); } return -ENOSPC; } @@ -582,7 +582,7 @@ static void iwl_hcmd_queue_reclaim(struct iwl_priv *priv, int txq_id, if (nfreed++ > 0) { IWL_ERR(priv, "HCMD skipped: index (%d) %d %d\n", idx, q->write_ptr, q->read_ptr); - queue_work(priv->workqueue, &priv->restart); + iwlagn_fw_error(priv, false); } } -- cgit From 901069c71415a76d731857ccda814e18ded062f7 Mon Sep 17 00:00:00 2001 From: Wey-Yi Guy Date: Tue, 5 Apr 2011 09:42:00 -0700 Subject: iwlagn: change Copyright to 2011 Signed-off-by: Wey-Yi Guy Signed-off-by: John W. Linville --- drivers/net/wireless/iwlwifi/iwl-tx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/net/wireless/iwlwifi/iwl-tx.c') diff --git a/drivers/net/wireless/iwlwifi/iwl-tx.c b/drivers/net/wireless/iwlwifi/iwl-tx.c index fa81df22a103..565980fbb591 100644 --- a/drivers/net/wireless/iwlwifi/iwl-tx.c +++ b/drivers/net/wireless/iwlwifi/iwl-tx.c @@ -1,6 +1,6 @@ /****************************************************************************** * - * Copyright(c) 2003 - 2010 Intel Corporation. All rights reserved. + * Copyright(c) 2003 - 2011 Intel Corporation. All rights reserved. * * Portions of this file are derived from the ipw3945 project, as well * as portions of the ieee80211 subsystem header files. -- cgit