On Mon, Oct 06, 2008 at 08:30:29PM +0200, Tomas Winkler wrote:
> On Mon, Oct 6, 2008 at 6:54 PM, Johannes Berg <johannes@xxxxxxxxxxxxxxxx> wrote:
> > On Mon, 2008-10-06 at 18:51 +0200, Johannes Berg wrote:
> >
> >> > > * therefore, all the is_odd junk goes away
> >> > > * fix a bug in iwl_hcmd_queue_reclaim where it would reclaim all the
> >> > > fragments of a descriptor rather than all descriptors (this may be
> >> > > the cause of the dma unmapping problem I reported)
> >> > > * some more cleanups
> >> >
> >> > > Signed-off-by: Johannes Berg <johannes@xxxxxxxxxxxxxxxx>
> >> > > --
> >> >
> >> >
> >> > > Tested on 5000 hw, please apply.
> >> >
> >> > Great job, however do not apply this before I review it I had strong
> >> > feeling this will not
> >> > work with aggregation flows.
> >>
> >> I cannot imagine why you think that, care to explain?
>
> Yep, no connection I thought at the first glance you've thatched more code.
> >
> > Of course, I would very much appreciate you review the actual bug fix in
> > iwl_hcmd_queue_reclaim, which consist of the addition of the line
> >
> > + bd = &txq->bd[index];
>
> Okay I found the source of the evil, with your big pointer :)
> The bug caused by this memory optimization juggling we've done and
> very bad cut and paste coding
> We moved for command buffers from pcI_alloc consitent to kmalloc which
> required pci mapping that that was coded wrongly. Now this kmalloc
> stuff is not good as well since we have this 36 bit memory limitation
> on 64 bit platforms.
> This is the issue I'm trying to address recently I'm not sure what yet
> what allocation schema would be best yet.
What if we just revert that patch?
---
From: John W. Linville <linville@xxxxxxxxxxxxx>
Date: Mon, 6 Oct 2008 15:22:27 -0400
Subject: [PATCH] Revert "iwlwifi: memory allocation optimization"
This reverts commit da99c4b6c25964b90c79f19beccda208df1a865a.
Conflicts:
drivers/net/wireless/iwlwifi/iwl-tx.c
---
drivers/net/wireless/iwlwifi/iwl-5000.c | 6 +-
drivers/net/wireless/iwlwifi/iwl-dev.h | 3 +-
drivers/net/wireless/iwlwifi/iwl-hcmd.c | 2 +-
drivers/net/wireless/iwlwifi/iwl-tx.c | 94 +++++++++----------------------
4 files changed, 33 insertions(+), 72 deletions(-)
diff --git a/drivers/net/wireless/iwlwifi/iwl-5000.c b/drivers/net/wireless/iwlwifi/iwl-5000.c
index b08036a..429f839 100644
--- a/drivers/net/wireless/iwlwifi/iwl-5000.c
+++ b/drivers/net/wireless/iwlwifi/iwl-5000.c
@@ -938,8 +938,8 @@ static void iwl5000_txq_update_byte_cnt_tbl(struct iwl_priv *priv,
len = byte_cnt + IWL_TX_CRC_SIZE + IWL_TX_DELIMITER_SIZE;
if (txq_id != IWL_CMD_QUEUE_NUM) {
- sta = txq->cmd[txq->q.write_ptr]->cmd.tx.sta_id;
- sec_ctl = txq->cmd[txq->q.write_ptr]->cmd.tx.sec_ctl;
+ sta = txq->cmd[txq->q.write_ptr].cmd.tx.sta_id;
+ sec_ctl = txq->cmd[txq->q.write_ptr].cmd.tx.sec_ctl;
switch (sec_ctl & TX_CMD_SEC_MSK) {
case TX_CMD_SEC_CCM:
@@ -978,7 +978,7 @@ static void iwl5000_txq_inval_byte_cnt_tbl(struct iwl_priv *priv,
u8 sta = 0;
if (txq_id != IWL_CMD_QUEUE_NUM)
- sta = txq->cmd[txq->q.read_ptr]->cmd.tx.sta_id;
+ sta = txq->cmd[txq->q.read_ptr].cmd.tx.sta_id;
shared_data->queues_byte_cnt_tbls[txq_id].tfd_offset[txq->q.read_ptr].
val = cpu_to_le16(1 | (sta << 12));
diff --git a/drivers/net/wireless/iwlwifi/iwl-dev.h b/drivers/net/wireless/iwlwifi/iwl-dev.h
index cdfb343..8416cab 100644
--- a/drivers/net/wireless/iwlwifi/iwl-dev.h
+++ b/drivers/net/wireless/iwlwifi/iwl-dev.h
@@ -135,7 +135,8 @@ struct iwl_tx_info {
struct iwl_tx_queue {
struct iwl_queue q;
struct iwl_tfd_frame *bd;
- struct iwl_cmd *cmd[TFD_TX_CMD_SLOTS];
+ struct iwl_cmd *cmd;
+ dma_addr_t dma_addr_cmd;
struct iwl_tx_info *txb;
int need_update;
int sched_retry;
diff --git a/drivers/net/wireless/iwlwifi/iwl-hcmd.c b/drivers/net/wireless/iwlwifi/iwl-hcmd.c
index 2eb03ee..986d79c 100644
--- a/drivers/net/wireless/iwlwifi/iwl-hcmd.c
+++ b/drivers/net/wireless/iwlwifi/iwl-hcmd.c
@@ -227,7 +227,7 @@ cancel:
* TX cmd queue. Otherwise in case the cmd comes
* in later, it will possibly set an invalid
* address (cmd->meta.source). */
- qcmd = priv->txq[IWL_CMD_QUEUE_NUM].cmd[cmd_idx];
+ qcmd = &priv->txq[IWL_CMD_QUEUE_NUM].cmd[cmd_idx];
qcmd->meta.flags &= ~CMD_WANT_SKB;
}
fail:
diff --git a/drivers/net/wireless/iwlwifi/iwl-tx.c b/drivers/net/wireless/iwlwifi/iwl-tx.c
index 78b1a7a..fa88d8e 100644
--- a/drivers/net/wireless/iwlwifi/iwl-tx.c
+++ b/drivers/net/wireless/iwlwifi/iwl-tx.c
@@ -208,12 +208,11 @@ EXPORT_SYMBOL(iwl_txq_update_write_ptr);
* Free all buffers.
* 0-fill, but do not free "txq" descriptor structure.
*/
-static void iwl_tx_queue_free(struct iwl_priv *priv, int txq_id)
+static void iwl_tx_queue_free(struct iwl_priv *priv, struct iwl_tx_queue *txq)
{
- struct iwl_tx_queue *txq = &priv->txq[txq_id];
struct iwl_queue *q = &txq->q;
struct pci_dev *dev = priv->pci_dev;
- int i, slots_num, len;
+ int len;
if (q->n_bd == 0)
return;
@@ -228,12 +227,7 @@ static void iwl_tx_queue_free(struct iwl_priv *priv, int txq_id)
len += IWL_MAX_SCAN_SIZE;
/* De-alloc array of command/tx buffers */
- slots_num = (txq_id == IWL_CMD_QUEUE_NUM) ?
- TFD_CMD_SLOTS : TFD_TX_CMD_SLOTS;
- for (i = 0; i < slots_num; i++)
- kfree(txq->cmd[i]);
- if (txq_id == IWL_CMD_QUEUE_NUM)
- kfree(txq->cmd[slots_num]);
+ pci_free_consistent(dev, len, txq->cmd, txq->dma_addr_cmd);
/* De-alloc circular buffer of TFDs */
if (txq->q.n_bd)
@@ -405,8 +399,9 @@ static int iwl_hw_tx_queue_init(struct iwl_priv *priv,
static int iwl_tx_queue_init(struct iwl_priv *priv, struct iwl_tx_queue *txq,
int slots_num, u32 txq_id)
{
- int i, len;
- int ret;
+ struct pci_dev *dev = priv->pci_dev;
+ int len;
+ int rc = 0;
/*
* Alloc buffer array for commands (Tx or other types of commands).
@@ -416,24 +411,19 @@ static int iwl_tx_queue_init(struct iwl_priv *priv, struct iwl_tx_queue *txq,
* For normal Tx queues (all other queues), no super-size command
* space is needed.
*/
- len = sizeof(struct iwl_cmd);
- for (i = 0; i <= slots_num; i++) {
- if (i == slots_num) {
- if (txq_id == IWL_CMD_QUEUE_NUM)
- len += IWL_MAX_SCAN_SIZE;
- else
- continue;
- }
-
- txq->cmd[i] = kmalloc(len, GFP_KERNEL);
- if (!txq->cmd[i])
- goto err;
- }
+ len = sizeof(struct iwl_cmd) * slots_num;
+ if (txq_id == IWL_CMD_QUEUE_NUM)
+ len += IWL_MAX_SCAN_SIZE;
+ txq->cmd = pci_alloc_consistent(dev, len, &txq->dma_addr_cmd);
+ if (!txq->cmd)
+ return -ENOMEM;
/* Alloc driver data array and TFD circular buffer */
- ret = iwl_tx_queue_alloc(priv, txq, txq_id);
- if (ret)
- goto err;
+ rc = iwl_tx_queue_alloc(priv, txq, txq_id);
+ if (rc) {
+ pci_free_consistent(dev, len, txq->cmd, txq->dma_addr_cmd);
+ return -ENOMEM;
+ }
txq->need_update = 0;
@@ -448,17 +438,6 @@ static int iwl_tx_queue_init(struct iwl_priv *priv, struct iwl_tx_queue *txq,
iwl_hw_tx_queue_init(priv, txq);
return 0;
-err:
- for (i = 0; i < slots_num; i++) {
- kfree(txq->cmd[i]);
- txq->cmd[i] = NULL;
- }
-
- if (txq_id == IWL_CMD_QUEUE_NUM) {
- kfree(txq->cmd[slots_num]);
- txq->cmd[slots_num] = NULL;
- }
- return -ENOMEM;
}
/**
* iwl_hw_txq_ctx_free - Free TXQ Context
@@ -471,7 +450,7 @@ void iwl_hw_txq_ctx_free(struct iwl_priv *priv)
/* Tx queues */
for (txq_id = 0; txq_id < priv->hw_params.max_txq_num; txq_id++)
- iwl_tx_queue_free(priv, txq_id);
+ iwl_tx_queue_free(priv, &priv->txq[txq_id]);
/* Keep-warm buffer */
iwl_kw_free(priv);
@@ -878,7 +857,7 @@ int iwl_tx_skb(struct iwl_priv *priv, struct sk_buff *skb)
txq->txb[q->write_ptr].skb[0] = skb;
/* Set up first empty entry in queue's array of Tx/cmd buffers */
- out_cmd = txq->cmd[idx];
+ out_cmd = &txq->cmd[idx];
tx_cmd = &out_cmd->cmd.tx;
memset(&out_cmd->hdr, 0, sizeof(out_cmd->hdr));
memset(tx_cmd, 0, sizeof(struct iwl_tx_cmd));
@@ -918,9 +897,8 @@ int iwl_tx_skb(struct iwl_priv *priv, struct sk_buff *skb)
/* Physical address of this Tx command's header (not MAC header!),
* within command buffer array. */
- txcmd_phys = pci_map_single(priv->pci_dev, out_cmd,
- sizeof(struct iwl_cmd), PCI_DMA_TODEVICE);
- txcmd_phys += offsetof(struct iwl_cmd, hdr);
+ txcmd_phys = txq->dma_addr_cmd + sizeof(struct iwl_cmd) * idx +
+ offsetof(struct iwl_cmd, hdr);
/* Add buffer containing Tx command and MAC(!) header to TFD's
* first entry */
@@ -1021,7 +999,7 @@ int iwl_enqueue_hcmd(struct iwl_priv *priv, struct iwl_host_cmd *cmd)
struct iwl_cmd *out_cmd;
dma_addr_t phys_addr;
unsigned long flags;
- int len, ret;
+ int ret;
u32 idx;
u16 fix_size;
@@ -1051,7 +1029,7 @@ int iwl_enqueue_hcmd(struct iwl_priv *priv, struct iwl_host_cmd *cmd)
idx = get_cmd_index(q, q->write_ptr, cmd->meta.flags & CMD_SIZE_HUGE);
- out_cmd = txq->cmd[idx];
+ out_cmd = &txq->cmd[idx];
out_cmd->hdr.cmd = cmd->id;
memcpy(&out_cmd->meta, &cmd->meta, sizeof(cmd->meta));
@@ -1065,11 +1043,9 @@ int iwl_enqueue_hcmd(struct iwl_priv *priv, struct iwl_host_cmd *cmd)
INDEX_TO_SEQ(q->write_ptr));
if (out_cmd->meta.flags & CMD_SIZE_HUGE)
out_cmd->hdr.sequence |= cpu_to_le16(SEQ_HUGE_FRAME);
- len = (idx == TFD_CMD_SLOTS) ?
- IWL_MAX_SCAN_SIZE : sizeof(struct iwl_cmd);
- phys_addr = pci_map_single(priv->pci_dev, out_cmd, len,
- PCI_DMA_TODEVICE);
- phys_addr += offsetof(struct iwl_cmd, hdr);
+
+ phys_addr = txq->dma_addr_cmd + sizeof(txq->cmd[0]) * idx +
+ offsetof(struct iwl_cmd, hdr);
iwl_hw_txq_attach_buf_to_tfd(priv, tfd, phys_addr, fix_size);
IWL_DEBUG_HC("Sending command %s (#%x), seq: 0x%04X, "
@@ -1134,9 +1110,6 @@ static void iwl_hcmd_queue_reclaim(struct iwl_priv *priv, int txq_id, int index)
{
struct iwl_tx_queue *txq = &priv->txq[txq_id];
struct iwl_queue *q = &txq->q;
- struct iwl_tfd_frame *bd = &txq->bd[index];
- dma_addr_t dma_addr;
- int is_odd, buf_len;
int nfreed = 0;
if ((index >= q->n_bd) || (iwl_queue_used(q, index) == 0)) {
@@ -1154,19 +1127,6 @@ static void iwl_hcmd_queue_reclaim(struct iwl_priv *priv, int txq_id, int index)
q->write_ptr, q->read_ptr);
queue_work(priv->workqueue, &priv->restart);
}
- is_odd = (index/2) & 0x1;
- if (is_odd) {
- dma_addr = IWL_GET_BITS(bd->pa[index], tb2_addr_lo16) |
- (IWL_GET_BITS(bd->pa[index],
- tb2_addr_hi20) << 16);
- buf_len = IWL_GET_BITS(bd->pa[index], tb2_len);
- } else {
- dma_addr = le32_to_cpu(bd->pa[index].tb1_addr);
- buf_len = IWL_GET_BITS(bd->pa[index], tb1_len);
- }
-
- pci_unmap_single(priv->pci_dev, dma_addr, buf_len,
- PCI_DMA_TODEVICE);
nfreed++;
}
}
@@ -1198,7 +1158,7 @@ void iwl_tx_cmd_complete(struct iwl_priv *priv, struct iwl_rx_mem_buffer *rxb)
BUG_ON(txq_id != IWL_CMD_QUEUE_NUM);
cmd_index = get_cmd_index(&priv->txq[IWL_CMD_QUEUE_NUM].q, index, huge);
- cmd = priv->txq[IWL_CMD_QUEUE_NUM].cmd[cmd_index];
+ cmd = &priv->txq[IWL_CMD_QUEUE_NUM].cmd[cmd_index];
/* Input error checking is done when commands are added to queue. */
if (cmd->meta.flags & CMD_WANT_SKB) {
--
1.5.4.3
--
John W. Linville Linux should be at the core
linville@xxxxxxxxxxxxx of your literate lifestyle.
--
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
