Hello
On Wed, Apr 20, 2011 at 07:05:48AM -0700, Guy, Wey-Yi wrote:
> > - 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.
get_cmd_index() is simple mathematical function of index local variable
(provided by firmware) and globally canst q->n_window, not need to be
protected.
What need to be protected is iwl_hcdm_queue_reclaim() as is touch
q->read_ptr and meta->flags to make assure is synchronized across
different cpus, when new huge command come instantly.
Note circular queue management could be done lock-less, but need
trickery described in Documentation/circular-buffers.txt to synchronize
q->read_ptr and q->write_ptr properly. What is probably too complex to
be worth to consider instead of simply using a spin lock.
Stanislaw
--
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
