Hi Stanislaw,
On Wed, 2011-04-20 at 07:02 -0700, Stanislaw Gruszka wrote:
> My commit 3598e1774c94e55c71b585340e7dc4538f310e3f
> "iwlwifi: fix enqueue hcmd race conditions" move hcmd callback after
> command queue reclaim, to avoid call it with hcmd_lock. But since
> queue read index was updated, cmd data can be overwritten. Fix problem
> by calling callback before taking hcmd_lock and queue reclaim.
>
> Signed-off-by: Stanislaw Gruszka <sgruszka@xxxxxxxxxx>
> ---
> drivers/net/wireless/iwlwifi/iwl-tx.c | 15 ++++-----------
> 1 files changed, 4 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/wireless/iwlwifi/iwl-tx.c b/drivers/net/wireless/iwlwifi/iwl-tx.c
> index 3732380..efff189 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-tx.c
> +++ b/drivers/net/wireless/iwlwifi/iwl-tx.c
> @@ -607,9 +607,6 @@ void iwl_tx_cmd_complete(struct iwl_priv *priv, struct iwl_rx_mem_buffer *rxb)
> 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
> @@ -623,8 +620,6 @@ void iwl_tx_cmd_complete(struct iwl_priv *priv, struct iwl_rx_mem_buffer *rxb)
> return;
> }
>
> - 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];
> @@ -634,13 +629,14 @@ 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
> - callback = meta->callback;
> + } else if (meta->callback)
> + meta->callback(priv, cmd, pkt);
> +
> + spin_lock_irqsave(&priv->hcmd_lock, flags);
>
> iwl_hcmd_queue_reclaim(priv, txq_id, index, cmd_index);
>
> @@ -655,7 +651,4 @@ void iwl_tx_cmd_complete(struct iwl_priv *priv, struct iwl_rx_mem_buffer *rxb)
> meta->flags = 0;
>
> spin_unlock_irqrestore(&priv->hcmd_lock, flags);
> -
> - if (callback)
> - callback(priv, cmd, pkt);
> }
Could you elaborate a bit more, why you do not need to protect getting
the cmd index.
Wey
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
