John,
Here are a few fixes for iwlwifi. It's probably too late to get
them into 3.4 but I split them out into their own series anyway.
This is my first ever submission as iwlwifi maintainer, so I
hope it works ok and doesn't get too many CCs :-)
johannes
John,
> The logic that allows to have a short TFD queue was completely wrong.
> We do maintain 256 Transmit Frame Descriptors, but they point to
> recycled buffers. We used to attach and de-attach different TFDs for
> the same buffer and it worked since they pointed to the same buffer.
>
> Also zero the number of BDs after unmapping a TFD. This seems not
> necessary since we don't reclaim the same TFD twice, but I like
> housekeeping.
> Cc: [email protected]
This is Cc stable, can we also get it to 3.4 still? If not I guess Greg
will pick it up
johannes
On Thu, 2012-05-10 at 18:50 +0200, Johannes Berg wrote:
> On Thu, 2012-05-10 at 09:38 -0700, Joe Perches wrote:
> > Add gcc format and argument printf checking
> > to reduce future defect introduction.
[]
> > #if defined(CONFIG_IWLWIFI_DEBUG) || defined(CONFIG_IWLWIFI_DEVICE_TRACING)
> > +__printf(5, 6)
> > void __iwl_dbg(struct device *dev,
> > u32 level, bool limit, const char *function,
> > const char *fmt, ...);
>
> But that looks a bit odd to me? It would seem more natural to me to put
> the attribute after the parameter list?
It's an attribute of the function and
this style is used almost everywhere
in the kernel.
On Thu, May 10, 2012 at 6:05 PM, Johannes Berg
<[email protected]> wrote:
> From: Meenakshi Venkataraman <[email protected]>
>
> We missed passing an argument to the
> debug print. Fix it.
>
> Signed-off-by: Meenakshi Venkataraman <[email protected]>
> Reviewed-by: Emmanuel Grumbach <[email protected]>
> Signed-off-by: Johannes Berg <[email protected]>
>
Tested-by: Sedat Dilek <[email protected]>
Add gcc format and argument printf checking
to reduce future defect introduction.
Signed-off-by: Joe Perches <[email protected]>
---
drivers/net/wireless/iwlwifi/iwl-debug.h | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/drivers/net/wireless/iwlwifi/iwl-debug.h b/drivers/net/wireless/iwlwifi/iwl-debug.h
index 8376b84..d0c7c9e 100644
--- a/drivers/net/wireless/iwlwifi/iwl-debug.h
+++ b/drivers/net/wireless/iwlwifi/iwl-debug.h
@@ -50,10 +50,12 @@ void __iwl_crit(struct device *dev, const char *fmt, ...);
#define IWL_CRIT(m, f, a...) __iwl_crit((m)->dev, f, ## a)
#if defined(CONFIG_IWLWIFI_DEBUG) || defined(CONFIG_IWLWIFI_DEVICE_TRACING)
+__printf(5, 6)
void __iwl_dbg(struct device *dev,
u32 level, bool limit, const char *function,
const char *fmt, ...);
#else
+__printf(5, 6)
static inline void
__iwl_dbg(struct device *dev,
u32 level, bool limit, const char *function,
On Thu, 2012-05-10 at 19:40 +0200, Johannes Berg wrote:
> Checkpatch
> doesn't like it though if I put it in the same line as "static inline
> void" ;-)
checkpatch is both wise and stupid.
On Thu, May 10, 2012 at 6:05 PM, Johannes Berg
<[email protected]> wrote:
> From: Meenakshi Venkataraman <[email protected]>
>
> When adding a station fails in
> iwl_restore_stations, the driver treats it
> like a successful station add and sends a
> link quality command, when it it shouldn't.
>
> This patch fixes one of the potential
> sources for kernel warnings like this one:
>
> WARNING: at drivers/net/wireless/iwlwifi/iwl-agn-sta.c:905 iwl_send_lq_cmd+0x130/0x217 [iwlwifi]()
> Hardware name: 3323A2G
> Modules linked in: ...
> Pid: 17359, comm: kworker/u:2 Tainted: G O 3.3.0-wl+ #1
> Call Trace:
> [<ffffffff81039620>] warn_slowpath_common+0x7e/0x96
> [<ffffffff8103964d>] warn_slowpath_null+0x15/0x17
> [<ffffffffa02a9f0b>] iwl_send_lq_cmd+0x130/0x217 [iwlwifi]
> [<ffffffffa02aa1fb>] iwl_restore_stations+0x209/0x289 [iwlwifi]
> [<ffffffffa02b07c2>] iwlagn_commit_rxon+0x602/0x7bd [iwlwifi]
> [<ffffffffa02b111f>] iwlagn_bss_info_changed+0x247/0x31a [iwlwifi]
> [<ffffffffa0861437>] ieee80211_bss_info_change_notify+0x1a5/0x1ba [mac80211]
> [<ffffffffa088afad>] ieee80211_destroy_auth_data+0x4b/0x70 [mac80211]
> [<ffffffffa088df26>] ieee80211_sta_work+0xb5/0x954 [mac80211]
>
> Signed-off-by: Meenakshi Venkataraman <[email protected]>
> Reviewed-by: Wey-Yi W Guy <[email protected]>
> Reviewed-by: Emmanuel Grumbach <[email protected]>
> Signed-off-by: Johannes Berg <[email protected]>
>
Tested-by: Sedat Dilek <[email protected]>
From: Meenakshi Venkataraman <[email protected]>
When adding a station fails in
iwl_restore_stations, the driver treats it
like a successful station add and sends a
link quality command, when it it shouldn't.
This patch fixes one of the potential
sources for kernel warnings like this one:
WARNING: at drivers/net/wireless/iwlwifi/iwl-agn-sta.c:905 iwl_send_lq_cmd+0x130/0x217 [iwlwifi]()
Hardware name: 3323A2G
Modules linked in: ...
Pid: 17359, comm: kworker/u:2 Tainted: G O 3.3.0-wl+ #1
Call Trace:
[<ffffffff81039620>] warn_slowpath_common+0x7e/0x96
[<ffffffff8103964d>] warn_slowpath_null+0x15/0x17
[<ffffffffa02a9f0b>] iwl_send_lq_cmd+0x130/0x217 [iwlwifi]
[<ffffffffa02aa1fb>] iwl_restore_stations+0x209/0x289 [iwlwifi]
[<ffffffffa02b07c2>] iwlagn_commit_rxon+0x602/0x7bd [iwlwifi]
[<ffffffffa02b111f>] iwlagn_bss_info_changed+0x247/0x31a [iwlwifi]
[<ffffffffa0861437>] ieee80211_bss_info_change_notify+0x1a5/0x1ba [mac80211]
[<ffffffffa088afad>] ieee80211_destroy_auth_data+0x4b/0x70 [mac80211]
[<ffffffffa088df26>] ieee80211_sta_work+0xb5/0x954 [mac80211]
Signed-off-by: Meenakshi Venkataraman <[email protected]>
Reviewed-by: Wey-Yi W Guy <[email protected]>
Reviewed-by: Emmanuel Grumbach <[email protected]>
Signed-off-by: Johannes Berg <[email protected]>
---
drivers/net/wireless/iwlwifi/iwl-agn-sta.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/iwlwifi/iwl-agn-sta.c b/drivers/net/wireless/iwlwifi/iwl-agn-sta.c
index 76c1f72..ca9ac20 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn-sta.c
+++ b/drivers/net/wireless/iwlwifi/iwl-agn-sta.c
@@ -773,7 +773,7 @@ void iwl_restore_stations(struct iwl_priv *priv, struct iwl_rxon_context *ctx)
~IWL_STA_DRIVER_ACTIVE;
priv->stations[i].used &=
~IWL_STA_UCODE_INPROGRESS;
- spin_unlock_bh(&priv->sta_lock);
+ continue;
}
/*
* Rate scaling has already been initialized, send
--
1.7.10
From: Emmanuel Grumbach <[email protected]>
The logic that allows to have a short TFD queue was completely wrong.
We do maintain 256 Transmit Frame Descriptors, but they point to
recycled buffers. We used to attach and de-attach different TFDs for
the same buffer and it worked since they pointed to the same buffer.
Also zero the number of BDs after unmapping a TFD. This seems not
necessary since we don't reclaim the same TFD twice, but I like
housekeeping.
This patch solves this warning:
[ 6427.079855] WARNING: at lib/dma-debug.c:866 check_unmap+0x727/0x7a0()
[ 6427.079859] Hardware name: Latitude E6410
[ 6427.079865] iwlwifi 0000:02:00.0: DMA-API: device driver tries to free DMA memory it has not allocated [device address=0x00000000296d393c] [size=8 bytes]
[ 6427.079870] Modules linked in: ...
[ 6427.079950] Pid: 6613, comm: ifconfig Tainted: G O 3.3.3 #5
[ 6427.079954] Call Trace:
[ 6427.079963] [<c10337a2>] warn_slowpath_common+0x72/0xa0
[ 6427.079982] [<c1033873>] warn_slowpath_fmt+0x33/0x40
[ 6427.079988] [<c12dcb77>] check_unmap+0x727/0x7a0
[ 6427.079995] [<c12dcdaa>] debug_dma_unmap_page+0x5a/0x80
[ 6427.080024] [<fe2312ac>] iwlagn_unmap_tfd+0x12c/0x180 [iwlwifi]
[ 6427.080048] [<fe231349>] iwlagn_txq_free_tfd+0x49/0xb0 [iwlwifi]
[ 6427.080071] [<fe228e37>] iwl_tx_queue_unmap+0x67/0x90 [iwlwifi]
[ 6427.080095] [<fe22d221>] iwl_trans_pcie_stop_device+0x341/0x7b0 [iwlwifi]
[ 6427.080113] [<fe204b0e>] iwl_down+0x17e/0x260 [iwlwifi]
[ 6427.080132] [<fe20efec>] iwlagn_mac_stop+0x6c/0xf0 [iwlwifi]
[ 6427.080168] [<fd8480ce>] ieee80211_stop_device+0x5e/0x190 [mac80211]
[ 6427.080198] [<fd833208>] ieee80211_do_stop+0x288/0x620 [mac80211]
[ 6427.080243] [<fd8335b7>] ieee80211_stop+0x17/0x20 [mac80211]
[ 6427.080250] [<c148dac1>] __dev_close_many+0x81/0xd0
[ 6427.080270] [<c148db3d>] __dev_close+0x2d/0x50
[ 6427.080276] [<c148d152>] __dev_change_flags+0x82/0x150
[ 6427.080282] [<c148e3e3>] dev_change_flags+0x23/0x60
[ 6427.080289] [<c14f6320>] devinet_ioctl+0x6a0/0x770
[ 6427.080296] [<c14f8705>] inet_ioctl+0x95/0xb0
[ 6427.080304] [<c147a0f0>] sock_ioctl+0x70/0x270
Cc: [email protected]
Reported-by: Antonio Quartulli <[email protected]>
Tested-by: Antonio Quartulli <[email protected]>
Signed-off-by: Emmanuel Grumbach <[email protected]>
Reviewed-by: Wey-Yi W Guy <[email protected]>
Signed-off-by: Johannes Berg <[email protected]>
---
drivers/net/wireless/iwlwifi/iwl-trans-pcie-int.h | 2 +-
drivers/net/wireless/iwlwifi/iwl-trans-pcie-tx.c | 22 +++++++++++++--------
drivers/net/wireless/iwlwifi/iwl-trans-pcie.c | 4 +---
3 files changed, 16 insertions(+), 12 deletions(-)
diff --git a/drivers/net/wireless/iwlwifi/iwl-trans-pcie-int.h b/drivers/net/wireless/iwlwifi/iwl-trans-pcie-int.h
index 6213c05..e959207 100644
--- a/drivers/net/wireless/iwlwifi/iwl-trans-pcie-int.h
+++ b/drivers/net/wireless/iwlwifi/iwl-trans-pcie-int.h
@@ -347,7 +347,7 @@ void iwl_trans_tx_queue_set_status(struct iwl_trans *trans,
void iwl_trans_pcie_tx_agg_setup(struct iwl_trans *trans, int queue, int fifo,
int sta_id, int tid, int frame_limit, u16 ssn);
void iwlagn_txq_free_tfd(struct iwl_trans *trans, struct iwl_tx_queue *txq,
- int index, enum dma_data_direction dma_dir);
+ enum dma_data_direction dma_dir);
int iwl_tx_queue_reclaim(struct iwl_trans *trans, int txq_id, int index,
struct sk_buff_head *skbs);
int iwl_queue_space(const struct iwl_queue *q);
diff --git a/drivers/net/wireless/iwlwifi/iwl-trans-pcie-tx.c b/drivers/net/wireless/iwlwifi/iwl-trans-pcie-tx.c
index 21a8a67..a875023 100644
--- a/drivers/net/wireless/iwlwifi/iwl-trans-pcie-tx.c
+++ b/drivers/net/wireless/iwlwifi/iwl-trans-pcie-tx.c
@@ -204,33 +204,39 @@ static void iwlagn_unmap_tfd(struct iwl_trans *trans, struct iwl_cmd_meta *meta,
for (i = 1; i < num_tbs; i++)
dma_unmap_single(trans->dev, iwl_tfd_tb_get_addr(tfd, i),
iwl_tfd_tb_get_len(tfd, i), dma_dir);
+
+ tfd->num_tbs = 0;
}
/**
* iwlagn_txq_free_tfd - Free all chunks referenced by TFD [txq->q.read_ptr]
* @trans - transport private data
* @txq - tx queue
- * @index - the index of the TFD to be freed
- *@dma_dir - the direction of the DMA mapping
+ * @dma_dir - the direction of the DMA mapping
*
* Does NOT advance any TFD circular buffer read/write indexes
* Does NOT free the TFD itself (which is within circular buffer)
*/
void iwlagn_txq_free_tfd(struct iwl_trans *trans, struct iwl_tx_queue *txq,
- int index, enum dma_data_direction dma_dir)
+ enum dma_data_direction dma_dir)
{
struct iwl_tfd *tfd_tmp = txq->tfds;
+ /* rd_ptr is bounded by n_bd and idx is bounded by n_window */
+ int rd_ptr = txq->q.read_ptr;
+ int idx = get_cmd_index(&txq->q, rd_ptr);
+
lockdep_assert_held(&txq->lock);
- iwlagn_unmap_tfd(trans, &txq->entries[index].meta,
- &tfd_tmp[index], dma_dir);
+ /* We have only q->n_window txq->entries, but we use q->n_bd tfds */
+ iwlagn_unmap_tfd(trans, &txq->entries[idx].meta,
+ &tfd_tmp[rd_ptr], dma_dir);
/* free SKB */
if (txq->entries) {
struct sk_buff *skb;
- skb = txq->entries[index].skb;
+ skb = txq->entries[idx].skb;
/* Can be called from irqs-disabled context
* If skb is not NULL, it means that the whole queue is being
@@ -238,7 +244,7 @@ void iwlagn_txq_free_tfd(struct iwl_trans *trans, struct iwl_tx_queue *txq,
*/
if (skb) {
iwl_op_mode_free_skb(trans->op_mode, skb);
- txq->entries[index].skb = NULL;
+ txq->entries[idx].skb = NULL;
}
}
}
@@ -973,7 +979,7 @@ int iwl_tx_queue_reclaim(struct iwl_trans *trans, int txq_id, int index,
iwlagn_txq_inval_byte_cnt_tbl(trans, txq);
- iwlagn_txq_free_tfd(trans, txq, txq->q.read_ptr, DMA_TO_DEVICE);
+ iwlagn_txq_free_tfd(trans, txq, DMA_TO_DEVICE);
freed++;
}
diff --git a/drivers/net/wireless/iwlwifi/iwl-trans-pcie.c b/drivers/net/wireless/iwlwifi/iwl-trans-pcie.c
index b2f27ec..3cdc014 100644
--- a/drivers/net/wireless/iwlwifi/iwl-trans-pcie.c
+++ b/drivers/net/wireless/iwlwifi/iwl-trans-pcie.c
@@ -435,9 +435,7 @@ static void iwl_tx_queue_unmap(struct iwl_trans *trans, int txq_id)
spin_lock_bh(&txq->lock);
while (q->write_ptr != q->read_ptr) {
- /* The read_ptr needs to bound by q->n_window */
- iwlagn_txq_free_tfd(trans, txq, get_cmd_index(q, q->read_ptr),
- dma_dir);
+ iwlagn_txq_free_tfd(trans, txq, dma_dir);
q->read_ptr = iwl_queue_inc_wrap(q->read_ptr, q->n_bd);
}
spin_unlock_bh(&txq->lock);
--
1.7.10
From: Emmanuel Grumbach <[email protected]>
The debug prints were wrong and buggy. The HW pointer wasn't printed
correctly, it was mixed up with the pointer to the rxbuf.
Signed-off-by: Emmanuel Grumbach <[email protected]>
Reviewed-by: Wey-Yi W Guy <[email protected]>
Signed-off-by: Johannes Berg <[email protected]>
---
v2: make patch apply -- looks like some whitespace diff to upstream
crept into our tree, thanks dileks.
--- a/drivers/net/wireless/iwlwifi/iwl-trans-pcie-rx.c
+++ b/drivers/net/wireless/iwlwifi/iwl-trans-pcie-rx.c
@@ -499,7 +499,7 @@ static void iwl_rx_handle(struct iwl_trans *trans)
/* Rx interrupt, but nothing sent from uCode */
if (i == r)
- IWL_DEBUG_RX(trans, "r = %d, i = %d\n", r, i);
+ IWL_DEBUG_RX(trans, "HW = SW = %d\n", r);
/* calculate total frames need to be restock after handling RX */
total_empty = r - rxq->write_actual;
@@ -515,8 +515,8 @@ static void iwl_rx_handle(struct iwl_trans *trans)
rxb = rxq->queue[i];
rxq->queue[i] = NULL;
- IWL_DEBUG_RX(trans, "rxbuf: r = %d, i = %d (%p)\n", rxb);
-
+ IWL_DEBUG_RX(trans, "rxbuf: HW = %d, SW = %d (%p)\n",
+ r, i, rxb);
iwl_rx_handle_rxbuf(trans, rxb);
i = (i + 1) & RX_QUEUE_MASK;
On Fri, May 11, 2012 at 12:59 PM, Johannes Berg
<[email protected]> wrote:
> From: Emmanuel Grumbach <[email protected]>
>
> The debug prints were wrong and buggy. The HW pointer wasn't printed
> correctly, it was mixed up with the pointer to the rxbuf.
>
> Signed-off-by: Emmanuel Grumbach <[email protected]>
> Reviewed-by: Wey-Yi W Guy <[email protected]>
> Signed-off-by: Johannes Berg <[email protected]>
> ---
> v2: make patch apply -- looks like some whitespace diff to upstream
> crept into our tree, thanks dileks.
>
Tested-by: Sedat Dilek <[email protected]>
On Thu, 2012-05-10 at 18:50 +0200, Johannes Berg wrote:
> On Thu, 2012-05-10 at 09:38 -0700, Joe Perches wrote:
> > Add gcc format and argument printf checking
> > to reduce future defect introduction.
>
> Good point, I'll pick this up.
I'll also add it to the other functions __iwl_crit etc.
Thanks for the suggestion!
johannes
From: Emmanuel Grumbach <[email protected]>
The debug prints were wrong and buggy. The HW pointer wasn't printed
correctly, it was mixed up with the pointer to the rxbuf.
Signed-off-by: Emmanuel Grumbach <[email protected]>
Reviewed-by: Wey-Yi W Guy <[email protected]>
Signed-off-by: Johannes Berg <[email protected]>
---
drivers/net/wireless/iwlwifi/iwl-trans-pcie-rx.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/iwlwifi/iwl-trans-pcie-rx.c b/drivers/net/wireless/iwlwifi/iwl-trans-pcie-rx.c
index d7b5e66..9191143 100644
--- a/drivers/net/wireless/iwlwifi/iwl-trans-pcie-rx.c
+++ b/drivers/net/wireless/iwlwifi/iwl-trans-pcie-rx.c
@@ -499,7 +499,7 @@ static void iwl_rx_handle(struct iwl_trans *trans)
/* Rx interrupt, but nothing sent from uCode */
if (i == r)
- IWL_DEBUG_RX(trans, "r = %d, i = %d\n", r, i);
+ IWL_DEBUG_RX(trans, "HW = SW = %d\n", r);
/* calculate total frames need to be restock after handling RX */
total_empty = r - rxq->write_actual;
@@ -515,7 +515,8 @@ static void iwl_rx_handle(struct iwl_trans *trans)
rxb = rxq->queue[i];
rxq->queue[i] = NULL;
- IWL_DEBUG_RX(trans, "rxbuf: r = %d, i = %d (%p)\n", rxb);
+ IWL_DEBUG_RX(trans, "rxbuf: HW = %d, SW = %d (%p)\n",
+ r, i, rxb);
iwl_rx_handle_rxbuf(trans, rxb);
i = (i + 1) & RX_QUEUE_MASK;
--
1.7.10
On Thu, 2012-05-10 at 10:34 -0700, Joe Perches wrote:
> On Thu, 2012-05-10 at 18:50 +0200, Johannes Berg wrote:
> > On Thu, 2012-05-10 at 09:38 -0700, Joe Perches wrote:
> > > Add gcc format and argument printf checking
> > > to reduce future defect introduction.
> []
> > > #if defined(CONFIG_IWLWIFI_DEBUG) || defined(CONFIG_IWLWIFI_DEVICE_TRACING)
> > > +__printf(5, 6)
> > > void __iwl_dbg(struct device *dev,
> > > u32 level, bool limit, const char *function,
> > > const char *fmt, ...);
> >
> > But that looks a bit odd to me? It would seem more natural to me to put
> > the attribute after the parameter list?
>
> It's an attribute of the function and
> this style is used almost everywhere
> in the kernel.
I also found that for the inline, it *has* to be there. Checkpatch
doesn't like it though if I put it in the same line as "static inline
void" ;-)
johannes
On Fri, May 11, 2012 at 2:50 PM, Johannes Berg
<[email protected]> wrote:
> From: Emmanuel Grumbach <[email protected]>
>
> The logic that allows to have a short TFD queue was completely wrong.
> We do maintain 256 Transmit Frame Descriptors, but they point to
> recycled buffers. We used to attach and de-attach different TFDs for
> the same buffer and it worked since they pointed to the same buffer.
>
> Also zero the number of BDs after unmapping a TFD. This seems not
> necessary since we don't reclaim the same TFD twice, but I like
> housekeeping.
>
> This patch solves this warning:
>
> [ 6427.079855] WARNING: at lib/dma-debug.c:866 check_unmap+0x727/0x7a0()
> [ 6427.079859] Hardware name: Latitude E6410
> [ 6427.079865] iwlwifi 0000:02:00.0: DMA-API: device driver tries to free DMA memory it has not allocated [device address=0x00000000296d393c] [size=8 bytes]
> [ 6427.079870] Modules linked in: ...
> [ 6427.079950] Pid: 6613, comm: ifconfig Tainted: G O 3.3.3 #5
> [ 6427.079954] Call Trace:
> [ 6427.079963] [<c10337a2>] warn_slowpath_common+0x72/0xa0
> [ 6427.079982] [<c1033873>] warn_slowpath_fmt+0x33/0x40
> [ 6427.079988] [<c12dcb77>] check_unmap+0x727/0x7a0
> [ 6427.079995] [<c12dcdaa>] debug_dma_unmap_page+0x5a/0x80
> [ 6427.080024] [<fe2312ac>] iwlagn_unmap_tfd+0x12c/0x180 [iwlwifi]
> [ 6427.080048] [<fe231349>] iwlagn_txq_free_tfd+0x49/0xb0 [iwlwifi]
> [ 6427.080071] [<fe228e37>] iwl_tx_queue_unmap+0x67/0x90 [iwlwifi]
> [ 6427.080095] [<fe22d221>] iwl_trans_pcie_stop_device+0x341/0x7b0 [iwlwifi]
> [ 6427.080113] [<fe204b0e>] iwl_down+0x17e/0x260 [iwlwifi]
> [ 6427.080132] [<fe20efec>] iwlagn_mac_stop+0x6c/0xf0 [iwlwifi]
> [ 6427.080168] [<fd8480ce>] ieee80211_stop_device+0x5e/0x190 [mac80211]
> [ 6427.080198] [<fd833208>] ieee80211_do_stop+0x288/0x620 [mac80211]
> [ 6427.080243] [<fd8335b7>] ieee80211_stop+0x17/0x20 [mac80211]
> [ 6427.080250] [<c148dac1>] __dev_close_many+0x81/0xd0
> [ 6427.080270] [<c148db3d>] __dev_close+0x2d/0x50
> [ 6427.080276] [<c148d152>] __dev_change_flags+0x82/0x150
> [ 6427.080282] [<c148e3e3>] dev_change_flags+0x23/0x60
> [ 6427.080289] [<c14f6320>] devinet_ioctl+0x6a0/0x770
> [ 6427.080296] [<c14f8705>] inet_ioctl+0x95/0xb0
> [ 6427.080304] [<c147a0f0>] sock_ioctl+0x70/0x270
>
> Cc: [email protected]
> Reported-by: Antonio Quartulli <[email protected]>
> Tested-by: Antonio Quartulli <[email protected]>
> Signed-off-by: Emmanuel Grumbach <[email protected]>
> Reviewed-by: Wey-Yi W Guy <[email protected]>
> Signed-off-by: Johannes Berg <[email protected]>
> ---
> v2: rebased onto wireless.git rather than wireless-testing.git, sorry!
>
Tested-by: Sedat Dilek <[email protected]>
On Thu, 2012-05-10 at 09:38 -0700, Joe Perches wrote:
> Add gcc format and argument printf checking
> to reduce future defect introduction.
Good point, I'll pick this up.
> Signed-off-by: Joe Perches <[email protected]>
> ---
> drivers/net/wireless/iwlwifi/iwl-debug.h | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/net/wireless/iwlwifi/iwl-debug.h b/drivers/net/wireless/iwlwifi/iwl-debug.h
> index 8376b84..d0c7c9e 100644
> --- a/drivers/net/wireless/iwlwifi/iwl-debug.h
> +++ b/drivers/net/wireless/iwlwifi/iwl-debug.h
> @@ -50,10 +50,12 @@ void __iwl_crit(struct device *dev, const char *fmt, ...);
> #define IWL_CRIT(m, f, a...) __iwl_crit((m)->dev, f, ## a)
>
> #if defined(CONFIG_IWLWIFI_DEBUG) || defined(CONFIG_IWLWIFI_DEVICE_TRACING)
> +__printf(5, 6)
> void __iwl_dbg(struct device *dev,
> u32 level, bool limit, const char *function,
> const char *fmt, ...);
But that looks a bit odd to me? It would seem more natural to me to put
the attribute after the parameter list?
johannes
From: Meenakshi Venkataraman <[email protected]>
We missed passing an argument to the
debug print. Fix it.
Signed-off-by: Meenakshi Venkataraman <[email protected]>
Reviewed-by: Emmanuel Grumbach <[email protected]>
Signed-off-by: Johannes Berg <[email protected]>
---
drivers/net/wireless/iwlwifi/iwl-agn-sta.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/wireless/iwlwifi/iwl-agn-sta.c b/drivers/net/wireless/iwlwifi/iwl-agn-sta.c
index 67e6f1d..76c1f72 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn-sta.c
+++ b/drivers/net/wireless/iwlwifi/iwl-agn-sta.c
@@ -236,6 +236,7 @@ static void iwl_sta_calc_ht_flags(struct iwl_priv *priv,
mimo_ps_mode = (sta_ht_inf->cap & IEEE80211_HT_CAP_SM_PS) >> 2;
IWL_DEBUG_INFO(priv, "STA %pM SM PS mode: %s\n",
+ sta->addr,
(mimo_ps_mode == WLAN_HT_CAP_SM_PS_STATIC) ?
"static" :
(mimo_ps_mode == WLAN_HT_CAP_SM_PS_DYNAMIC) ?
--
1.7.10
On Thu, May 10, 2012 at 06:05:48PM +0200, Johannes Berg wrote:
> John,
>
> Here are a few fixes for iwlwifi. It's probably too late to get
> them into 3.4 but I split them out into their own series anyway.
>
> This is my first ever submission as iwlwifi maintainer, so I
> hope it works ok and doesn't get too many CCs :-)
>
> johannes
I didn't think they were suitable for post-rc7. I tried to apply
them to wireless-next, but they required more rework than I thought
I should be trusted to give them. :-)
John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.
From: Emmanuel Grumbach <[email protected]>
The logic that allows to have a short TFD queue was completely wrong.
We do maintain 256 Transmit Frame Descriptors, but they point to
recycled buffers. We used to attach and de-attach different TFDs for
the same buffer and it worked since they pointed to the same buffer.
Also zero the number of BDs after unmapping a TFD. This seems not
necessary since we don't reclaim the same TFD twice, but I like
housekeeping.
This patch solves this warning:
[ 6427.079855] WARNING: at lib/dma-debug.c:866 check_unmap+0x727/0x7a0()
[ 6427.079859] Hardware name: Latitude E6410
[ 6427.079865] iwlwifi 0000:02:00.0: DMA-API: device driver tries to free DMA memory it has not allocated [device address=0x00000000296d393c] [size=8 bytes]
[ 6427.079870] Modules linked in: ...
[ 6427.079950] Pid: 6613, comm: ifconfig Tainted: G O 3.3.3 #5
[ 6427.079954] Call Trace:
[ 6427.079963] [<c10337a2>] warn_slowpath_common+0x72/0xa0
[ 6427.079982] [<c1033873>] warn_slowpath_fmt+0x33/0x40
[ 6427.079988] [<c12dcb77>] check_unmap+0x727/0x7a0
[ 6427.079995] [<c12dcdaa>] debug_dma_unmap_page+0x5a/0x80
[ 6427.080024] [<fe2312ac>] iwlagn_unmap_tfd+0x12c/0x180 [iwlwifi]
[ 6427.080048] [<fe231349>] iwlagn_txq_free_tfd+0x49/0xb0 [iwlwifi]
[ 6427.080071] [<fe228e37>] iwl_tx_queue_unmap+0x67/0x90 [iwlwifi]
[ 6427.080095] [<fe22d221>] iwl_trans_pcie_stop_device+0x341/0x7b0 [iwlwifi]
[ 6427.080113] [<fe204b0e>] iwl_down+0x17e/0x260 [iwlwifi]
[ 6427.080132] [<fe20efec>] iwlagn_mac_stop+0x6c/0xf0 [iwlwifi]
[ 6427.080168] [<fd8480ce>] ieee80211_stop_device+0x5e/0x190 [mac80211]
[ 6427.080198] [<fd833208>] ieee80211_do_stop+0x288/0x620 [mac80211]
[ 6427.080243] [<fd8335b7>] ieee80211_stop+0x17/0x20 [mac80211]
[ 6427.080250] [<c148dac1>] __dev_close_many+0x81/0xd0
[ 6427.080270] [<c148db3d>] __dev_close+0x2d/0x50
[ 6427.080276] [<c148d152>] __dev_change_flags+0x82/0x150
[ 6427.080282] [<c148e3e3>] dev_change_flags+0x23/0x60
[ 6427.080289] [<c14f6320>] devinet_ioctl+0x6a0/0x770
[ 6427.080296] [<c14f8705>] inet_ioctl+0x95/0xb0
[ 6427.080304] [<c147a0f0>] sock_ioctl+0x70/0x270
Cc: [email protected]
Reported-by: Antonio Quartulli <[email protected]>
Tested-by: Antonio Quartulli <[email protected]>
Signed-off-by: Emmanuel Grumbach <[email protected]>
Reviewed-by: Wey-Yi W Guy <[email protected]>
Signed-off-by: Johannes Berg <[email protected]>
---
v2: rebased onto wireless.git rather than wireless-testing.git, sorry!
diff --git a/drivers/net/wireless/iwlwifi/iwl-trans-pcie-int.h b/drivers/net/wireless/iwlwifi/iwl-trans-pcie-int.h
index 1c2fe87..3b844b7 100644
--- a/drivers/net/wireless/iwlwifi/iwl-trans-pcie-int.h
+++ b/drivers/net/wireless/iwlwifi/iwl-trans-pcie-int.h
@@ -342,7 +342,7 @@ void iwl_trans_pcie_tx_agg_setup(struct iwl_trans *trans,
enum iwl_rxon_context_id ctx,
int sta_id, int tid, int frame_limit, u16 ssn);
void iwlagn_txq_free_tfd(struct iwl_trans *trans, struct iwl_tx_queue *txq,
- int index, enum dma_data_direction dma_dir);
+ enum dma_data_direction dma_dir);
int iwl_tx_queue_reclaim(struct iwl_trans *trans, int txq_id, int index,
struct sk_buff_head *skbs);
int iwl_queue_space(const struct iwl_queue *q);
diff --git a/drivers/net/wireless/iwlwifi/iwl-trans-pcie-tx.c b/drivers/net/wireless/iwlwifi/iwl-trans-pcie-tx.c
index e92972f..d7964b1 100644
--- a/drivers/net/wireless/iwlwifi/iwl-trans-pcie-tx.c
+++ b/drivers/net/wireless/iwlwifi/iwl-trans-pcie-tx.c
@@ -237,32 +237,38 @@ static void iwlagn_unmap_tfd(struct iwl_trans *trans, struct iwl_cmd_meta *meta,
for (i = 1; i < num_tbs; i++)
dma_unmap_single(trans->dev, iwl_tfd_tb_get_addr(tfd, i),
iwl_tfd_tb_get_len(tfd, i), dma_dir);
+
+ tfd->num_tbs = 0;
}
/**
* iwlagn_txq_free_tfd - Free all chunks referenced by TFD [txq->q.read_ptr]
* @trans - transport private data
* @txq - tx queue
- * @index - the index of the TFD to be freed
- *@dma_dir - the direction of the DMA mapping
+ * @dma_dir - the direction of the DMA mapping
*
* Does NOT advance any TFD circular buffer read/write indexes
* Does NOT free the TFD itself (which is within circular buffer)
*/
void iwlagn_txq_free_tfd(struct iwl_trans *trans, struct iwl_tx_queue *txq,
- int index, enum dma_data_direction dma_dir)
+ enum dma_data_direction dma_dir)
{
struct iwl_tfd *tfd_tmp = txq->tfds;
+ /* rd_ptr is bounded by n_bd and idx is bounded by n_window */
+ int rd_ptr = txq->q.read_ptr;
+ int idx = get_cmd_index(&txq->q, rd_ptr);
+
lockdep_assert_held(&txq->lock);
- iwlagn_unmap_tfd(trans, &txq->meta[index], &tfd_tmp[index], dma_dir);
+ /* We have only q->n_window txq->entries, but we use q->n_bd tfds */
+ iwlagn_unmap_tfd(trans, &txq->meta[idx], &tfd_tmp[rd_ptr], dma_dir);
/* free SKB */
if (txq->skbs) {
struct sk_buff *skb;
- skb = txq->skbs[index];
+ skb = txq->skbs[idx];
/* Can be called from irqs-disabled context
* If skb is not NULL, it means that the whole queue is being
@@ -270,7 +276,7 @@ void iwlagn_txq_free_tfd(struct iwl_trans *trans, struct iwl_tx_queue *txq,
*/
if (skb) {
iwl_op_mode_free_skb(trans->op_mode, skb);
- txq->skbs[index] = NULL;
+ txq->skbs[idx] = NULL;
}
}
}
@@ -1100,7 +1106,7 @@ int iwl_tx_queue_reclaim(struct iwl_trans *trans, int txq_id, int index,
iwlagn_txq_inval_byte_cnt_tbl(trans, txq);
- iwlagn_txq_free_tfd(trans, txq, txq->q.read_ptr, DMA_TO_DEVICE);
+ iwlagn_txq_free_tfd(trans, txq, DMA_TO_DEVICE);
freed++;
}
return freed;
diff --git a/drivers/net/wireless/iwlwifi/iwl-trans-pcie.c b/drivers/net/wireless/iwlwifi/iwl-trans-pcie.c
index 4d7b30d..74b49ca 100644
--- a/drivers/net/wireless/iwlwifi/iwl-trans-pcie.c
+++ b/drivers/net/wireless/iwlwifi/iwl-trans-pcie.c
@@ -430,9 +430,7 @@ static void iwl_tx_queue_unmap(struct iwl_trans *trans, int txq_id)
spin_lock_bh(&txq->lock);
while (q->write_ptr != q->read_ptr) {
- /* The read_ptr needs to bound by q->n_window */
- iwlagn_txq_free_tfd(trans, txq, get_cmd_index(q, q->read_ptr),
- dma_dir);
+ iwlagn_txq_free_tfd(trans, txq, dma_dir);
q->read_ptr = iwl_queue_inc_wrap(q->read_ptr, q->n_bd);
}
spin_unlock_bh(&txq->lock);