[PATCH 0/5] misc ath5k fixes

[PATCH 0/5] misc ath5k fixes

[Bob Copeland]

Here are various fixes for ath5k.  Patch 1 is the result of a script to
compare our initval tables against the ones in the open source HALs.
Patches 2 and 3 are self-explanatory.  Patches 4 and 5 may help make a
use-after-free bug more rare (though it isn't fully solved - both need
some testing so are 2.6.31 material).

Bob Copeland (5):
  ath5k: fix initvals errors
  ath5k: use tasklet_hi_schedule for beacon queue
  ath5k: use bool for modparams
  ath5k: use rx hw descriptor pointer for self-linked check
  ath5k: manipulate rxlink and descriptor address under rxbuf lock

 drivers/net/wireless/ath/ath5k/base.c     |   35 +++++++---------------------
 drivers/net/wireless/ath/ath5k/base.h     |    1 -
 drivers/net/wireless/ath/ath5k/dma.c      |    2 -
 drivers/net/wireless/ath/ath5k/initvals.c |    8 ++----
 4 files changed, 12 insertions(+), 34 deletions(-)

[PATCH 1/5] ath5k: fix initvals errors

[Bob Copeland]

This patch corrects a few errors in the initvals tables to match those
in the HAL tables.  Namely, remove a couple of repetitions, fix some
turbo mode errors, and correct a register for the CCK rate power table.

Changes-licensed-under: ISC

Signed-off-by: Bob Copeland <me@bobcopeland.com>
---
 drivers/net/wireless/ath/ath5k/initvals.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/initvals.c b/drivers/net/wireless/ath/ath5k/initvals.c
index 61fb621..18eb519 100644
--- a/drivers/net/wireless/ath/ath5k/initvals.c
+++ b/drivers/net/wireless/ath/ath5k/initvals.c
@@ -537,8 +537,6 @@ static const struct ath5k_ini ar5212_ini_common_start[] = {
 	{ AR5K_DCU_TX_FILTER_1(15), 0x00000000 },
 	{ AR5K_DCU_TX_FILTER_CLR, 0x00000000 },
 	{ AR5K_DCU_TX_FILTER_SET, 0x00000000 },
-	{ AR5K_DCU_TX_FILTER_CLR, 0x00000000 },
-	{ AR5K_DCU_TX_FILTER_SET, 0x00000000 },
 	{ AR5K_STA_ID1,		0x00000000 },
 	{ AR5K_BSS_ID0,		0x00000000 },
 	{ AR5K_BSS_ID1,		0x00000000 },
@@ -669,7 +667,7 @@ static const struct ath5k_ini ar5212_ini_common_start[] = {
 	/*{ AR5K_PHY(650), 0x000001b5 },*/
 	{ AR5K_PHY(651),	0x00000000 },
 	{ AR5K_PHY_TXPOWER_RATE3, 0x20202020 },
-	{ AR5K_PHY_TXPOWER_RATE2, 0x20202020 },
+	{ AR5K_PHY_TXPOWER_RATE4, 0x20202020 },
 	/*{ AR5K_PHY(655), 0x13c889af },*/
 	{ AR5K_PHY(656),	0x38490a20 },
 	{ AR5K_PHY(657),	0x00007bb6 },
@@ -718,7 +716,7 @@ static const struct ath5k_ini_mode ar5212_ini_mode_start[] = {
 	{ AR5K_PHY_SETTLING,
 	   { 0x1372161c, 0x13721c25, 0x13721722, 0x137216a2, 0x13721c25 } },
 	{ AR5K_PHY_AGCCTL,
-	   { 0x00009d10, 0x00009d10, 0x00009d18, 0x00009d18, 0x00009d18 } },
+	   { 0x00009d10, 0x00009d10, 0x00009d18, 0x00009d18, 0x00009d10 } },
 	{ AR5K_PHY_NF,
 	   { 0x0001ce00, 0x0001ce00, 0x0001ce00, 0x0001ce00, 0x0001ce00 } },
 	{ AR5K_PHY_WEAK_OFDM_HIGH_THR,
@@ -799,7 +797,7 @@ static const struct ath5k_ini_mode rf5112_ini_mode_end[] = {
 	{ AR5K_PHY_DESIRED_SIZE,
 	   { 0x0de8b4e0, 0x0de8b4e0, 0x0de8b4e0, 0x0de8b4e0, 0x0de8b4e0 } },
 	{ AR5K_PHY_SIG,
-	   { 0x7e800d2e, 0x7e800d2e, 0x7ee80d2e, 0x7ee80d2e, 0x7ee80d2e } },
+	   { 0x7e800d2e, 0x7e800d2e, 0x7ee80d2e, 0x7ee80d2e, 0x7e800d2e } },
 	{ AR5K_PHY_AGCCOARSE,
 	   { 0x3137665e, 0x3137665e, 0x3137665e, 0x3137665e, 0x3137665e } },
 	{ AR5K_PHY_WEAK_OFDM_LOW_THR,
-- 
1.6.0.6

Re: [PATCH 1/5] ath5k: fix initvals errors

[Nick Kossifidis]

2009/4/15 Bob Copeland <me@bobcopeland.com>:
> This patch corrects a few errors in the initvals tables to match thos=
e
> in the HAL tables. =C2=A0Namely, remove a couple of repetitions, fix =
some
> turbo mode errors, and correct a register for the CCK rate power tabl=
e.
>
> Changes-licensed-under: ISC
>
> Signed-off-by: Bob Copeland <me@bobcopeland.com>
> ---

Acked-by: Nick Kossifidis <mickflemm@gmail.com>



--=20
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/5] ath5k: use tasklet_hi_schedule for beacon queue

[Bob Copeland]

For embedded platforms, beacon transmission can be starved when
flooded with data packets.  Prioritize beacons by giving the beacon
queue the first shot when the isr completes.

Changes-licensed-under: 3-Clause-BSD

Signed-off-by: Bob Copeland <me@bobcopeland.com>
---
 drivers/net/wireless/ath/ath5k/base.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c
index ff6d4f8..ef8523e 100644
--- a/drivers/net/wireless/ath/ath5k/base.c
+++ b/drivers/net/wireless/ath/ath5k/base.c
@@ -2496,7 +2496,7 @@ ath5k_intr(int irq, void *dev_id)
 			tasklet_schedule(&sc->restq);
 		} else {
 			if (status & AR5K_INT_SWBA) {
-				tasklet_schedule(&sc->beacontq);
+				tasklet_hi_schedule(&sc->beacontq);
 			}
 			if (status & AR5K_INT_RXEOL) {
 				/*
-- 
1.6.0.6

Re: [PATCH 2/5] ath5k: use tasklet_hi_schedule for beacon queue

[Nick Kossifidis]

2009/4/15 Bob Copeland <me@bobcopeland.com>:
> For embedded platforms, beacon transmission can be starved when
> flooded with data packets. =C2=A0Prioritize beacons by giving the bea=
con
> queue the first shot when the isr completes.
>
> Changes-licensed-under: 3-Clause-BSD
>
> Signed-off-by: Bob Copeland <me@bobcopeland.com>


Acked-by: Nick Kossifidis <mickflemm@gmail.com>





--=20
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 3/5] ath5k: use bool for modparams

[Bob Copeland]

Current code uses int types, but both modparams are boolean values.

Changes-licensed-under: 3-Clause-BSD

Signed-off-by: Bob Copeland <me@bobcopeland.com>
---
 drivers/net/wireless/ath/ath5k/base.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c
index ef8523e..3bde18f 100644
--- a/drivers/net/wireless/ath/ath5k/base.c
+++ b/drivers/net/wireless/ath/ath5k/base.c
@@ -61,11 +61,11 @@
 
 static int ath5k_calinterval = 10; /* Calibrate PHY every 10 secs (TODO: Fixme) */
 static int modparam_nohwcrypt;
-module_param_named(nohwcrypt, modparam_nohwcrypt, int, 0444);
+module_param_named(nohwcrypt, modparam_nohwcrypt, bool, S_IRUGO);
 MODULE_PARM_DESC(nohwcrypt, "Disable hardware encryption.");
 
 static int modparam_all_channels;
-module_param_named(all_channels, modparam_all_channels, int, 0444);
+module_param_named(all_channels, modparam_all_channels, bool, S_IRUGO);
 MODULE_PARM_DESC(all_channels, "Expose all channels the device can use.");
 
 
-- 
1.6.0.6

Re: [PATCH 3/5] ath5k: use bool for modparams

[Nick Kossifidis]

2009/4/15 Bob Copeland <me@bobcopeland.com>:
> Current code uses int types, but both modparams are boolean values.
>
> Changes-licensed-under: 3-Clause-BSD
>
> Signed-off-by: Bob Copeland <me@bobcopeland.com>
> ---


Acked-by: Nick Kossifidis <mickflemm@gmail.com>




-- 
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick

[PATCH 4/5] ath5k: use rx hw descriptor pointer for self-linked check

[Bob Copeland]

This patch simplifies the code used to detect when the
self-linked DMA buffer is still in use by hardware, by
checking the hardware's rxdp register instead of looking
at the software buffer list.

Signed-off-by: Bob Copeland <me@bobcopeland.com>
---
 drivers/net/wireless/ath/ath5k/base.c |   24 ++++--------------------
 drivers/net/wireless/ath/ath5k/base.h |    1 -
 drivers/net/wireless/ath/ath5k/dma.c  |    2 --
 3 files changed, 4 insertions(+), 23 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c
index 3bde18f..1a6e72f 100644
--- a/drivers/net/wireless/ath/ath5k/base.c
+++ b/drivers/net/wireless/ath/ath5k/base.c
@@ -1780,7 +1780,7 @@ ath5k_tasklet_rx(unsigned long data)
 	struct sk_buff *skb, *next_skb;
 	dma_addr_t next_skb_addr;
 	struct ath5k_softc *sc = (void *)data;
-	struct ath5k_buf *bf, *bf_last;
+	struct ath5k_buf *bf;
 	struct ath5k_desc *ds;
 	int ret;
 	int hdrlen;
@@ -1791,7 +1791,6 @@ ath5k_tasklet_rx(unsigned long data)
 		ATH5K_WARN(sc, "empty rx buf pool\n");
 		goto unlock;
 	}
-	bf_last = list_entry(sc->rxbuf.prev, struct ath5k_buf, list);
 	do {
 		rxs.flag = 0;
 
@@ -1800,24 +1799,9 @@ ath5k_tasklet_rx(unsigned long data)
 		skb = bf->skb;
 		ds = bf->desc;
 
-		/*
-		 * last buffer must not be freed to ensure proper hardware
-		 * function. When the hardware finishes also a packet next to
-		 * it, we are sure, it doesn't use it anymore and we can go on.
-		 */
-		if (bf_last == bf)
-			bf->flags |= 1;
-		if (bf->flags) {
-			struct ath5k_buf *bf_next = list_entry(bf->list.next,
-					struct ath5k_buf, list);
-			ret = sc->ah->ah_proc_rx_desc(sc->ah, bf_next->desc,
-					&rs);
-			if (ret)
-				break;
-			bf->flags &= ~1;
-			/* skip the overwritten one (even status is martian) */
-			goto next;
-		}
+		/* bail if HW is still using self-linked descriptor */
+		if (ath5k_hw_get_rxdp(sc->ah) == bf->daddr)
+			break;
 
 		ret = sc->ah->ah_proc_rx_desc(sc->ah, ds, &rs);
 		if (unlikely(ret == -EINPROGRESS))
diff --git a/drivers/net/wireless/ath/ath5k/base.h b/drivers/net/wireless/ath/ath5k/base.h
index 8229561..852b2c1 100644
--- a/drivers/net/wireless/ath/ath5k/base.h
+++ b/drivers/net/wireless/ath/ath5k/base.h
@@ -56,7 +56,6 @@
 
 struct ath5k_buf {
 	struct list_head	list;
-	unsigned int		flags;	/* rx descriptor flags */
 	struct ath5k_desc	*desc;	/* virtual addr of desc */
 	dma_addr_t		daddr;	/* physical addr of desc */
 	struct sk_buff		*skb;	/* skbuff for buf */
diff --git a/drivers/net/wireless/ath/ath5k/dma.c b/drivers/net/wireless/ath/ath5k/dma.c
index b65b4fe..941b511 100644
--- a/drivers/net/wireless/ath/ath5k/dma.c
+++ b/drivers/net/wireless/ath/ath5k/dma.c
@@ -80,8 +80,6 @@ int ath5k_hw_stop_rx_dma(struct ath5k_hw *ah)
  * ath5k_hw_get_rxdp - Get RX Descriptor's address
  *
  * @ah: The &struct ath5k_hw
- *
- * XXX: Is RXDP read and clear ?
  */
 u32 ath5k_hw_get_rxdp(struct ath5k_hw *ah)
 {
-- 
1.6.0.6

Re: [PATCH 4/5] ath5k: use rx hw descriptor pointer for self-linked check

[Nick Kossifidis]

2009/4/15 Bob Copeland <me@bobcopeland.com>:
> This patch simplifies the code used to detect when the
> self-linked DMA buffer is still in use by hardware, by
> checking the hardware's rxdp register instead of looking
> at the software buffer list.
>
> Signed-off-by: Bob Copeland <me@bobcopeland.com>
> ---

Acked-by: Nick Kossifidis <mickflemm@gmail.com>



-- 
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick

Re: [ath5k-devel] [PATCH 4/5] ath5k: use rx hw descriptor pointer for self-linked check

[Daniel J Blueman]

Hi Bob,

On Wed, Apr 15, 2009 at 12:57 PM, Bob Copeland <me@bobcopeland.com> wro=
te:
> This patch simplifies the code used to detect when the
> self-linked DMA buffer is still in use by hardware, by
> checking the hardware's rxdp register instead of looking
> at the software buffer list.

Just an observation: Each synchronous read over PCI will take at
minimum 0.9us to return. Accessing tables in the host's cache
hierarchy/memory will likely be much faster - possibly why it is done
like this already.

Is this a worthwhile tradeoff?

Many thanks,
  Daniel

> Signed-off-by: Bob Copeland <me@bobcopeland.com>
> ---
> =A0drivers/net/wireless/ath/ath5k/base.c | =A0 24 ++++---------------=
-----
> =A0drivers/net/wireless/ath/ath5k/base.h | =A0 =A01 -
> =A0drivers/net/wireless/ath/ath5k/dma.c =A0| =A0 =A02 --
> =A03 files changed, 4 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wire=
less/ath/ath5k/base.c
> index 3bde18f..1a6e72f 100644
> --- a/drivers/net/wireless/ath/ath5k/base.c
> +++ b/drivers/net/wireless/ath/ath5k/base.c
> @@ -1780,7 +1780,7 @@ ath5k_tasklet_rx(unsigned long data)
> =A0 =A0 =A0 =A0struct sk_buff *skb, *next_skb;
> =A0 =A0 =A0 =A0dma_addr_t next_skb_addr;
> =A0 =A0 =A0 =A0struct ath5k_softc *sc =3D (void *)data;
> - =A0 =A0 =A0 struct ath5k_buf *bf, *bf_last;
> + =A0 =A0 =A0 struct ath5k_buf *bf;
> =A0 =A0 =A0 =A0struct ath5k_desc *ds;
> =A0 =A0 =A0 =A0int ret;
> =A0 =A0 =A0 =A0int hdrlen;
> @@ -1791,7 +1791,6 @@ ath5k_tasklet_rx(unsigned long data)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0ATH5K_WARN(sc, "empty rx buf pool\n");
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0goto unlock;
> =A0 =A0 =A0 =A0}
> - =A0 =A0 =A0 bf_last =3D list_entry(sc->rxbuf.prev, struct ath5k_buf=
, list);
> =A0 =A0 =A0 =A0do {
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0rxs.flag =3D 0;
>
> @@ -1800,24 +1799,9 @@ ath5k_tasklet_rx(unsigned long data)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0skb =3D bf->skb;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0ds =3D bf->desc;
>
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 /*
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* last buffer must not be freed to e=
nsure proper hardware
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* function. When the hardware finish=
es also a packet next to
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0* it, we are sure, it doesn't use it=
 anymore and we can go on.
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0*/
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (bf_last =3D=3D bf)
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 bf->flags |=3D 1;
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (bf->flags) {
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 struct ath5k_buf *bf_ne=
xt =3D list_entry(bf->list.next,
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 =A0 struct ath5k_buf, list);
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 ret =3D sc->ah->ah_proc=
_rx_desc(sc->ah, bf_next->desc,
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
 =A0 =A0 &rs);
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (ret)
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 break;
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 bf->flags &=3D ~1;
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* skip the overwritten=
 one (even status is martian) */
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto next;
> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 }
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 /* bail if HW is still using self-linke=
d descriptor */
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (ath5k_hw_get_rxdp(sc->ah) =3D=3D bf=
->daddr)
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 break;
>
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0ret =3D sc->ah->ah_proc_rx_desc(sc->ah=
, ds, &rs);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (unlikely(ret =3D=3D -EINPROGRESS))
> diff --git a/drivers/net/wireless/ath/ath5k/base.h b/drivers/net/wire=
less/ath/ath5k/base.h
> index 8229561..852b2c1 100644
> --- a/drivers/net/wireless/ath/ath5k/base.h
> +++ b/drivers/net/wireless/ath/ath5k/base.h
> @@ -56,7 +56,6 @@
>
> =A0struct ath5k_buf {
> =A0 =A0 =A0 =A0struct list_head =A0 =A0 =A0 =A0list;
> - =A0 =A0 =A0 unsigned int =A0 =A0 =A0 =A0 =A0 =A0flags; =A0/* rx des=
criptor flags */
> =A0 =A0 =A0 =A0struct ath5k_desc =A0 =A0 =A0 *desc; =A0/* virtual add=
r of desc */
> =A0 =A0 =A0 =A0dma_addr_t =A0 =A0 =A0 =A0 =A0 =A0 =A0daddr; =A0/* phy=
sical addr of desc */
> =A0 =A0 =A0 =A0struct sk_buff =A0 =A0 =A0 =A0 =A0*skb; =A0 /* skbuff =
for buf */
> diff --git a/drivers/net/wireless/ath/ath5k/dma.c b/drivers/net/wirel=
ess/ath/ath5k/dma.c
> index b65b4fe..941b511 100644
> --- a/drivers/net/wireless/ath/ath5k/dma.c
> +++ b/drivers/net/wireless/ath/ath5k/dma.c
> @@ -80,8 +80,6 @@ int ath5k_hw_stop_rx_dma(struct ath5k_hw *ah)
> =A0* ath5k_hw_get_rxdp - Get RX Descriptor's address
> =A0*
> =A0* @ah: The &struct ath5k_hw
> - *
> - * XXX: Is RXDP read and clear ?
> =A0*/
> =A0u32 ath5k_hw_get_rxdp(struct ath5k_hw *ah)
> =A0{
--=20
Daniel J Blueman
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [ath5k-devel] [PATCH 4/5] ath5k: use rx hw descriptor pointer for self-linked check

[Bob Copeland]

On Wed, Apr 15, 2009 at 01:20:09PM +0100, Daniel J Blueman wrote:
> Just an observation: Each synchronous read over PCI will take at
> minimum 0.9us to return. Accessing tables in the host's cache
> hierarchy/memory will likely be much faster - possibly why it is done
> like this already.
> 
> Is this a worthwhile tradeoff?

This is a good point -- but I think in this case, it is worth it
from a correctness rather than performance standpoint.

See http://lkml.org/lkml/2009/3/8/99 for why I think the current
code doesn't handle every case.

FWIW, madwifi does this too.  We could eliminate it if we dropped the
self-linked buffer thing (but then we'd need to handle EOL interrupts).

-- 
Bob Copeland %% www.bobcopeland.com

[PATCH 5/5] ath5k: manipulate rxlink and descriptor address under rxbuf lock

[Bob Copeland]

Grabbing an ath5k_buf then dropping the lock is racy because the
referenced descriptor can be obtained in another thread and released
before the buffer is handed to the hardware.  Likewise, manipulating
sc->rxlink without the lock can lead to having multiple self-linked
hardware descriptors.

Changes-licensed-under: 3-Clause-BSD

Signed-off-by: Bob Copeland <me@bobcopeland.com>
---
 drivers/net/wireless/ath/ath5k/base.c |    5 ++---
 1 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c
index 1a6e72f..c8c658b 100644
--- a/drivers/net/wireless/ath/ath5k/base.c
+++ b/drivers/net/wireless/ath/ath5k/base.c
@@ -1618,9 +1618,8 @@ ath5k_rx_start(struct ath5k_softc *sc)
 	ATH5K_DBG(sc, ATH5K_DEBUG_RESET, "cachelsz %u rxbufsize %u\n",
 		sc->cachelsz, sc->rxbufsize);
 
-	sc->rxlink = NULL;
-
 	spin_lock_bh(&sc->rxbuflock);
+	sc->rxlink = NULL;
 	list_for_each_entry(bf, &sc->rxbuf, list) {
 		ret = ath5k_rxbuf_setup(sc, bf);
 		if (ret != 0) {
@@ -1629,9 +1628,9 @@ ath5k_rx_start(struct ath5k_softc *sc)
 		}
 	}
 	bf = list_first_entry(&sc->rxbuf, struct ath5k_buf, list);
+	ath5k_hw_set_rxdp(ah, bf->daddr);
 	spin_unlock_bh(&sc->rxbuflock);
 
-	ath5k_hw_set_rxdp(ah, bf->daddr);
 	ath5k_hw_start_rx_dma(ah);	/* enable recv descriptors */
 	ath5k_mode_setup(sc);		/* set filters, etc. */
 	ath5k_hw_start_rx_pcu(ah);	/* re-enable PCU/DMA engine */
-- 
1.6.0.6

Re: [PATCH 5/5] ath5k: manipulate rxlink and descriptor address under rxbuf lock

[Nick Kossifidis]

2009/4/15 Bob Copeland <me@bobcopeland.com>:
> Grabbing an ath5k_buf then dropping the lock is racy because the
> referenced descriptor can be obtained in another thread and released
> before the buffer is handed to the hardware. =C2=A0Likewise, manipula=
ting
> sc->rxlink without the lock can lead to having multiple self-linked
> hardware descriptors.
>
> Changes-licensed-under: 3-Clause-BSD
>
> Signed-off-by: Bob Copeland <me@bobcopeland.com>
> ---

Acked-by: Nick Kossifidis <mickflemm@gmail.com>


--=20
GPG ID: 0xD21DB2DB
As you read this post global entropy rises. Have Fun ;-)
Nick
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html