[PATCH 1/3] iwlwifi: fix enqueue hcmd race conditions

[PATCH 1/3] iwlwifi: fix enqueue hcmd race conditions

[Stanislaw Gruszka]

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 <sgruszka@redhat.com>
---
 drivers/net/wireless/iwlwifi/iwl-dev.h |    1 +
 drivers/net/wireless/iwlwifi/iwl-tx.c  |   62 +++++++++++++++++---------------
 2 files changed, 34 insertions(+), 29 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-dev.h b/drivers/net/wireless/iwlwifi/iwl-dev.h
index 68b953f..9e63bd8 100644
--- a/drivers/net/wireless/iwlwifi/iwl-dev.h
+++ b/drivers/net/wireless/iwlwifi/iwl-dev.h
@@ -309,6 +309,7 @@ enum {
 	CMD_SIZE_HUGE = (1 << 0),
 	CMD_ASYNC = (1 << 1),
 	CMD_WANT_SKB = (1 << 2),
+	CMD_MAPPED = (1 << 3),
 };
 
 #define DEF_CMD_PAYLOAD_SIZE 320
diff --git a/drivers/net/wireless/iwlwifi/iwl-tx.c b/drivers/net/wireless/iwlwifi/iwl-tx.c
index 277c917..39a4180 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);
 }
-- 
1.7.1

[PATCH 2/3] iwlwifi: more priv->mutex serialization

[Stanislaw Gruszka]

Check status bits with mutex taken, because when we wait for mutex
unlock, status can change. Patch should also make remaining sync
commands be send with priv->mutex taken. That will prevent execute
these commands when we are currently reset firmware, what could
possibly cause troubles.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/iwlwifi/iwl-agn.c |   34 ++++++++++++++++++-------------
 1 files changed, 20 insertions(+), 14 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-agn.c b/drivers/net/wireless/iwlwifi/iwl-agn.c
index 7adc60e..0c49d1a 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn.c
+++ b/drivers/net/wireless/iwlwifi/iwl-agn.c
@@ -483,12 +483,14 @@ static void iwl_bg_bt_full_concurrency(struct work_struct *work)
 		container_of(work, struct iwl_priv, bt_full_concurrency);
 	struct iwl_rxon_context *ctx;
 
+	mutex_lock(&priv->mutex);
+
 	if (test_bit(STATUS_EXIT_PENDING, &priv->status))
-		return;
+		goto out;
 
 	/* dont send host command if rf-kill is on */
 	if (!iwl_is_ready_rf(priv))
-		return;
+		goto out;
 
 	IWL_DEBUG_INFO(priv, "BT coex in %s mode\n",
 		       priv->bt_full_concurrent ?
@@ -498,15 +500,15 @@ static void iwl_bg_bt_full_concurrency(struct work_struct *work)
 	 * LQ & RXON updated cmds must be sent before BT Config cmd
 	 * to avoid 3-wire collisions
 	 */
-	mutex_lock(&priv->mutex);
 	for_each_context(priv, ctx) {
 		if (priv->cfg->ops->hcmd->set_rxon_chain)
 			priv->cfg->ops->hcmd->set_rxon_chain(priv, ctx);
 		iwlcore_commit_rxon(priv, ctx);
 	}
-	mutex_unlock(&priv->mutex);
 
 	priv->cfg->ops->hcmd->send_bt_config(priv);
+out:
+	mutex_unlock(&priv->mutex);
 }
 
 /**
@@ -2814,10 +2816,13 @@ static void iwl_bg_init_alive_start(struct work_struct *data)
 	struct iwl_priv *priv =
 	    container_of(data, struct iwl_priv, init_alive_start.work);
 
-	if (test_bit(STATUS_EXIT_PENDING, &priv->status))
+	mutex_lock(&priv->mutex);
+
+	if (test_bit(STATUS_EXIT_PENDING, &priv->status)) {
+		mutex_unlock(&priv->mutex);
 		return;
+	}
 
-	mutex_lock(&priv->mutex);
 	priv->cfg->ops->lib->init_alive_start(priv);
 	mutex_unlock(&priv->mutex);
 }
@@ -2827,15 +2832,16 @@ static void iwl_bg_alive_start(struct work_struct *data)
 	struct iwl_priv *priv =
 	    container_of(data, struct iwl_priv, alive_start.work);
 
+	mutex_lock(&priv->mutex);
 	if (test_bit(STATUS_EXIT_PENDING, &priv->status))
-		return;
+		goto unlock;
 
 	/* enable dram interrupt */
 	if (priv->cfg->ops->lib->isr_ops.reset)
 		priv->cfg->ops->lib->isr_ops.reset(priv);
 
-	mutex_lock(&priv->mutex);
 	iwl_alive_start(priv);
+unlock:
 	mutex_unlock(&priv->mutex);
 }
 
@@ -3457,21 +3463,22 @@ void iwlagn_mac_channel_switch(struct ieee80211_hw *hw,
 
 	IWL_DEBUG_MAC80211(priv, "enter\n");
 
+	mutex_lock(&priv->mutex);
+
 	if (iwl_is_rfkill(priv))
-		goto out_exit;
+		goto out;
 
 	if (test_bit(STATUS_EXIT_PENDING, &priv->status) ||
 	    test_bit(STATUS_SCANNING, &priv->status))
-		goto out_exit;
+		goto out;
 
 	if (!iwl_is_associated_ctx(ctx))
-		goto out_exit;
+		goto out;
 
 	/* channel switch in progress */
 	if (priv->switch_rxon.switch_in_progress == true)
-		goto out_exit;
+		goto out;
 
-	mutex_lock(&priv->mutex);
 	if (priv->cfg->ops->lib->set_channel_switch) {
 
 		ch = channel->hw_value;
@@ -3527,7 +3534,6 @@ void iwlagn_mac_channel_switch(struct ieee80211_hw *hw,
 	}
 out:
 	mutex_unlock(&priv->mutex);
-out_exit:
 	if (!priv->switch_rxon.switch_in_progress)
 		ieee80211_chswitch_done(ctx->vif, false);
 	IWL_DEBUG_MAC80211(priv, "leave\n");
-- 
1.7.1

[PATCH 3/3] iwlwifi: remove sync_cmd_mutex

[Stanislaw Gruszka]

We now use priv->mutex to serialize sync command, remove old
priv->sync_cmd_mutex and add assertion that priv->mutex must be locked.

Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
 drivers/net/wireless/iwlwifi/iwl-agn.c  |    4 ----
 drivers/net/wireless/iwlwifi/iwl-dev.h  |    1 -
 drivers/net/wireless/iwlwifi/iwl-hcmd.c |   13 +++++--------
 3 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/drivers/net/wireless/iwlwifi/iwl-agn.c b/drivers/net/wireless/iwlwifi/iwl-agn.c
index 0c49d1a..6f188fd 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn.c
+++ b/drivers/net/wireless/iwlwifi/iwl-agn.c
@@ -2565,9 +2565,6 @@ static void __iwl_down(struct iwl_priv *priv)
 	priv->bt_full_concurrent = false;
 	priv->bt_ci_compliance = 0;
 
-	/* Unblock any waiting calls */
-	wake_up_interruptible_all(&priv->wait_command_queue);
-
 	/* Wipe out the EXIT_PENDING status bit if we are not actually
 	 * exiting the module */
 	if (!exit_pending)
@@ -3814,7 +3811,6 @@ static int iwl_init_drv(struct iwl_priv *priv)
 	INIT_LIST_HEAD(&priv->free_frames);
 
 	mutex_init(&priv->mutex);
-	mutex_init(&priv->sync_cmd_mutex);
 
 	priv->ieee_channels = NULL;
 	priv->ieee_rates = NULL;
diff --git a/drivers/net/wireless/iwlwifi/iwl-dev.h b/drivers/net/wireless/iwlwifi/iwl-dev.h
index 9e63bd8..ca30f95 100644
--- a/drivers/net/wireless/iwlwifi/iwl-dev.h
+++ b/drivers/net/wireless/iwlwifi/iwl-dev.h
@@ -1306,7 +1306,6 @@ struct iwl_priv {
 	spinlock_t hcmd_lock;	/* protect hcmd */
 	spinlock_t reg_lock;	/* protect hw register access */
 	struct mutex mutex;
-	struct mutex sync_cmd_mutex; /* enable serialization of sync commands */
 
 	/* basic pci-network driver stuff */
 	struct pci_dev *pci_dev;
diff --git a/drivers/net/wireless/iwlwifi/iwl-hcmd.c b/drivers/net/wireless/iwlwifi/iwl-hcmd.c
index 02499f6..c71c0a4 100644
--- a/drivers/net/wireless/iwlwifi/iwl-hcmd.c
+++ b/drivers/net/wireless/iwlwifi/iwl-hcmd.c
@@ -171,14 +171,13 @@ int iwl_send_cmd_sync(struct iwl_priv *priv, struct iwl_host_cmd *cmd)
 	int cmd_idx;
 	int ret;
 
-	BUG_ON(cmd->flags & CMD_ASYNC);
+	lockdep_assert_held(&priv->mutex);
 
 	 /* A synchronous command can not have a callback set. */
-	BUG_ON(cmd->callback);
+	BUG_ON((cmd->flags & CMD_ASYNC) || cmd->callback);
 
 	IWL_DEBUG_INFO(priv, "Attempting to send sync command %s\n",
 			get_cmd_string(cmd->id));
-	mutex_lock(&priv->sync_cmd_mutex);
 
 	set_bit(STATUS_HCMD_ACTIVE, &priv->status);
 	IWL_DEBUG_INFO(priv, "Setting HCMD_ACTIVE for command %s\n",
@@ -189,7 +188,7 @@ int iwl_send_cmd_sync(struct iwl_priv *priv, struct iwl_host_cmd *cmd)
 		ret = cmd_idx;
 		IWL_ERR(priv, "Error sending %s: enqueue_hcmd failed: %d\n",
 			  get_cmd_string(cmd->id), ret);
-		goto out;
+		return ret;
 	}
 
 	ret = wait_event_interruptible_timeout(priv->wait_command_queue,
@@ -229,8 +228,7 @@ int iwl_send_cmd_sync(struct iwl_priv *priv, struct iwl_host_cmd *cmd)
 		goto cancel;
 	}
 
-	ret = 0;
-	goto out;
+	return 0;
 
 cancel:
 	if (cmd->flags & CMD_WANT_SKB) {
@@ -248,8 +246,7 @@ fail:
 		iwl_free_pages(priv, cmd->reply_page);
 		cmd->reply_page = 0;
 	}
-out:
-	mutex_unlock(&priv->sync_cmd_mutex);
+
 	return ret;
 }
 
-- 
1.7.1

Re: [PATCH 2/3] iwlwifi: more priv->mutex serialization

[wwguy]

On Thu, 2011-03-31 at 08:36 -0700, Stanislaw Gruszka wrote:
> Check status bits with mutex taken, because when we wait for mutex
> unlock, status can change. Patch should also make remaining sync
> commands be send with priv->mutex taken. That will prevent execute
> these commands when we are currently reset firmware, what could
> possibly cause troubles.
> 
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
Acked-by: Wey-Yi Guy <wey-yi.w.guy@intel.com>
> ---
>  drivers/net/wireless/iwlwifi/iwl-agn.c |   34 ++++++++++++++++++-------------
>  1 files changed, 20 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/net/wireless/iwlwifi/iwl-agn.c b/drivers/net/wireless/iwlwifi/iwl-agn.c
> index 7adc60e..0c49d1a 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-agn.c
> +++ b/drivers/net/wireless/iwlwifi/iwl-agn.c
> @@ -483,12 +483,14 @@ static void iwl_bg_bt_full_concurrency(struct work_struct *work)
>  		container_of(work, struct iwl_priv, bt_full_concurrency);
>  	struct iwl_rxon_context *ctx;
>  
> +	mutex_lock(&priv->mutex);
> +
>  	if (test_bit(STATUS_EXIT_PENDING, &priv->status))
> -		return;
> +		goto out;
>  
>  	/* dont send host command if rf-kill is on */
>  	if (!iwl_is_ready_rf(priv))
> -		return;
> +		goto out;
>  
>  	IWL_DEBUG_INFO(priv, "BT coex in %s mode\n",
>  		       priv->bt_full_concurrent ?
> @@ -498,15 +500,15 @@ static void iwl_bg_bt_full_concurrency(struct work_struct *work)
>  	 * LQ & RXON updated cmds must be sent before BT Config cmd
>  	 * to avoid 3-wire collisions
>  	 */
> -	mutex_lock(&priv->mutex);
>  	for_each_context(priv, ctx) {
>  		if (priv->cfg->ops->hcmd->set_rxon_chain)
>  			priv->cfg->ops->hcmd->set_rxon_chain(priv, ctx);
>  		iwlcore_commit_rxon(priv, ctx);
>  	}
> -	mutex_unlock(&priv->mutex);
>  
>  	priv->cfg->ops->hcmd->send_bt_config(priv);
> +out:
> +	mutex_unlock(&priv->mutex);
>  }
>  
>  /**
> @@ -2814,10 +2816,13 @@ static void iwl_bg_init_alive_start(struct work_struct *data)
>  	struct iwl_priv *priv =
>  	    container_of(data, struct iwl_priv, init_alive_start.work);
>  
> -	if (test_bit(STATUS_EXIT_PENDING, &priv->status))
> +	mutex_lock(&priv->mutex);
> +
> +	if (test_bit(STATUS_EXIT_PENDING, &priv->status)) {
> +		mutex_unlock(&priv->mutex);
>  		return;
> +	}
>  
> -	mutex_lock(&priv->mutex);
>  	priv->cfg->ops->lib->init_alive_start(priv);
>  	mutex_unlock(&priv->mutex);
>  }
> @@ -2827,15 +2832,16 @@ static void iwl_bg_alive_start(struct work_struct *data)
>  	struct iwl_priv *priv =
>  	    container_of(data, struct iwl_priv, alive_start.work);
>  
> +	mutex_lock(&priv->mutex);
>  	if (test_bit(STATUS_EXIT_PENDING, &priv->status))
> -		return;
> +		goto unlock;
>  
>  	/* enable dram interrupt */
>  	if (priv->cfg->ops->lib->isr_ops.reset)
>  		priv->cfg->ops->lib->isr_ops.reset(priv);
>  
> -	mutex_lock(&priv->mutex);
>  	iwl_alive_start(priv);
> +unlock:
>  	mutex_unlock(&priv->mutex);
>  }
>  
> @@ -3457,21 +3463,22 @@ void iwlagn_mac_channel_switch(struct ieee80211_hw *hw,
>  
>  	IWL_DEBUG_MAC80211(priv, "enter\n");
>  
> +	mutex_lock(&priv->mutex);
> +
>  	if (iwl_is_rfkill(priv))
> -		goto out_exit;
> +		goto out;
>  
>  	if (test_bit(STATUS_EXIT_PENDING, &priv->status) ||
>  	    test_bit(STATUS_SCANNING, &priv->status))
> -		goto out_exit;
> +		goto out;
>  
>  	if (!iwl_is_associated_ctx(ctx))
> -		goto out_exit;
> +		goto out;
>  
>  	/* channel switch in progress */
>  	if (priv->switch_rxon.switch_in_progress == true)
> -		goto out_exit;
> +		goto out;
>  
> -	mutex_lock(&priv->mutex);
>  	if (priv->cfg->ops->lib->set_channel_switch) {
>  
>  		ch = channel->hw_value;
> @@ -3527,7 +3534,6 @@ void iwlagn_mac_channel_switch(struct ieee80211_hw *hw,
>  	}
>  out:
>  	mutex_unlock(&priv->mutex);
> -out_exit:
>  	if (!priv->switch_rxon.switch_in_progress)
>  		ieee80211_chswitch_done(ctx->vif, false);
>  	IWL_DEBUG_MAC80211(priv, "leave\n");

Re: [PATCH 3/3] iwlwifi: remove sync_cmd_mutex

[wwguy]

On Thu, 2011-03-31 at 08:36 -0700, Stanislaw Gruszka wrote:
> We now use priv->mutex to serialize sync command, remove old
> priv->sync_cmd_mutex and add assertion that priv->mutex must be locked.
> 
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
Acked-by: Wey-Yi Guy <wey-yi.w.guy@intel.com>
> ---
>  drivers/net/wireless/iwlwifi/iwl-agn.c  |    4 ----
>  drivers/net/wireless/iwlwifi/iwl-dev.h  |    1 -
>  drivers/net/wireless/iwlwifi/iwl-hcmd.c |   13 +++++--------
>  3 files changed, 5 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/net/wireless/iwlwifi/iwl-agn.c b/drivers/net/wireless/iwlwifi/iwl-agn.c
> index 0c49d1a..6f188fd 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-agn.c
> +++ b/drivers/net/wireless/iwlwifi/iwl-agn.c
> @@ -2565,9 +2565,6 @@ static void __iwl_down(struct iwl_priv *priv)
>  	priv->bt_full_concurrent = false;
>  	priv->bt_ci_compliance = 0;
>  
> -	/* Unblock any waiting calls */
> -	wake_up_interruptible_all(&priv->wait_command_queue);
> -
>  	/* Wipe out the EXIT_PENDING status bit if we are not actually
>  	 * exiting the module */
>  	if (!exit_pending)
> @@ -3814,7 +3811,6 @@ static int iwl_init_drv(struct iwl_priv *priv)
>  	INIT_LIST_HEAD(&priv->free_frames);
>  
>  	mutex_init(&priv->mutex);
> -	mutex_init(&priv->sync_cmd_mutex);
>  
>  	priv->ieee_channels = NULL;
>  	priv->ieee_rates = NULL;
> diff --git a/drivers/net/wireless/iwlwifi/iwl-dev.h b/drivers/net/wireless/iwlwifi/iwl-dev.h
> index 9e63bd8..ca30f95 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-dev.h
> +++ b/drivers/net/wireless/iwlwifi/iwl-dev.h
> @@ -1306,7 +1306,6 @@ struct iwl_priv {
>  	spinlock_t hcmd_lock;	/* protect hcmd */
>  	spinlock_t reg_lock;	/* protect hw register access */
>  	struct mutex mutex;
> -	struct mutex sync_cmd_mutex; /* enable serialization of sync commands */
>  
>  	/* basic pci-network driver stuff */
>  	struct pci_dev *pci_dev;
> diff --git a/drivers/net/wireless/iwlwifi/iwl-hcmd.c b/drivers/net/wireless/iwlwifi/iwl-hcmd.c
> index 02499f6..c71c0a4 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-hcmd.c
> +++ b/drivers/net/wireless/iwlwifi/iwl-hcmd.c
> @@ -171,14 +171,13 @@ int iwl_send_cmd_sync(struct iwl_priv *priv, struct iwl_host_cmd *cmd)
>  	int cmd_idx;
>  	int ret;
>  
> -	BUG_ON(cmd->flags & CMD_ASYNC);
> +	lockdep_assert_held(&priv->mutex);
>  
>  	 /* A synchronous command can not have a callback set. */
> -	BUG_ON(cmd->callback);
> +	BUG_ON((cmd->flags & CMD_ASYNC) || cmd->callback);
>  
>  	IWL_DEBUG_INFO(priv, "Attempting to send sync command %s\n",
>  			get_cmd_string(cmd->id));
> -	mutex_lock(&priv->sync_cmd_mutex);
>  
>  	set_bit(STATUS_HCMD_ACTIVE, &priv->status);
>  	IWL_DEBUG_INFO(priv, "Setting HCMD_ACTIVE for command %s\n",
> @@ -189,7 +188,7 @@ int iwl_send_cmd_sync(struct iwl_priv *priv, struct iwl_host_cmd *cmd)
>  		ret = cmd_idx;
>  		IWL_ERR(priv, "Error sending %s: enqueue_hcmd failed: %d\n",
>  			  get_cmd_string(cmd->id), ret);
> -		goto out;
> +		return ret;
>  	}
>  
>  	ret = wait_event_interruptible_timeout(priv->wait_command_queue,
> @@ -229,8 +228,7 @@ int iwl_send_cmd_sync(struct iwl_priv *priv, struct iwl_host_cmd *cmd)
>  		goto cancel;
>  	}
>  
> -	ret = 0;
> -	goto out;
> +	return 0;
>  
>  cancel:
>  	if (cmd->flags & CMD_WANT_SKB) {
> @@ -248,8 +246,7 @@ fail:
>  		iwl_free_pages(priv, cmd->reply_page);
>  		cmd->reply_page = 0;
>  	}
> -out:
> -	mutex_unlock(&priv->sync_cmd_mutex);
> +
>  	return ret;
>  }
>  

RE: [PATCH 1/3] iwlwifi: fix enqueue hcmd race conditions

[Guy, Wey-Yi W]

Stanislaw,

-----Original Message-----
From: Stanislaw Gruszka [mailto:sgruszka@redhat.com] 
Sent: Thursday, March 31, 2011 8:36 AM
To: Guy, Wey-Yi W
Cc: Intel Linux Wireless; linux-wireless@vger.kernel.org; Stanislaw Gruszka
Subject: [PATCH 1/3] 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.

Do you have a bug# against this?

Thanks
Wey

Re: [PATCH 1/3] iwlwifi: fix enqueue hcmd race conditions

[Stanislaw Gruszka]

Hi Wey

> Do you have a bug# against this?
No, I was fixing problems happened on my local system.

Stanislaw

Re: [PATCH 1/3] iwlwifi: fix enqueue hcmd race conditions

[wwguy]

On Fri, 2011-04-01 at 00:22 -0700, Stanislaw Gruszka wrote:
> Hi Wey
> 
> > Do you have a bug# against this?
> No, I was fixing problems happened on my local system.
> 
fair enough :-), thanks for the update

Wey

Re: [PATCH 1/3] iwlwifi: fix enqueue hcmd race conditions

[Stanislaw Gruszka]

On Fri, Apr 01, 2011 at 08:07:25AM -0700, wwguy wrote:
> > > Do you have a bug# against this?
> > No, I was fixing problems happened on my local system.
> > 
> fair enough :-), thanks for the update

So we could consider patch as acked.

Stanislaw

Re: [PATCH 1/3] iwlwifi: fix enqueue hcmd race conditions

[Guy, Wey-Yi]

On Wed, 2011-04-06 at 07:18 -0700, Stanislaw Gruszka wrote:
> On Fri, Apr 01, 2011 at 08:07:25AM -0700, wwguy wrote:
> > > > Do you have a bug# against this?
> > > No, I was fixing problems happened on my local system.
> > > 
> > fair enough :-), thanks for the update
> 
> So we could consider patch as acked.

forgot to "ack", sorry :-)

Wey