* [PATCH] ath5k: fix requested allocated RX skb size for DMA
@ 2009-08-12 16:58 Luis R. Rodriguez
2009-08-12 18:13 ` Jiri Slaby
0 siblings, 1 reply; 5+ messages in thread
From: Luis R. Rodriguez @ 2009-08-12 16:58 UTC (permalink / raw)
To: linville
Cc: linux-wireless, ath9k-devel, ath5k-devel, Luis R. Rodriguez,
Jiri Slaby, Nick Kossifidis, Bob Copeland
When we dev_alloc_skb() our RX buffers we were asking
for them to be of size:
sc->rxbufsize + sc->common.cachelsz - 1
but when mapping processor virtual memeory for access by the
hardware we were only giving it the sc->rxbufsize, therefore
always preventing the device from accessing the fully allocated
skb.
This patch fixes the disparity. It is unclear to me if there are
other consequences to having this mismatch other than perhaps never
allowing the device to use the full capacity of the allocated skb.
Cc: Jiri Slaby <jirislaby@gmail.com>
Cc: Nick Kossifidis <mickflemm@gmail.com>
Cc: Bob Copeland <me@bobcopeland.com>
Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com>
---
drivers/net/wireless/ath/ath5k/base.c | 8 +++++---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c
index 2b3cf39..75843f2 100644
--- a/drivers/net/wireless/ath/ath5k/base.c
+++ b/drivers/net/wireless/ath/ath5k/base.c
@@ -1157,17 +1157,18 @@ struct sk_buff *ath5k_rx_skb_alloc(struct ath5k_softc *sc, dma_addr_t *skb_addr)
* fake physical layer header at the start.
*/
skb = ath_rxbuf_alloc(&sc->common,
- sc->rxbufsize + sc->common.cachelsz - 1,
+ sc->rxbufsize,
GFP_ATOMIC);
if (!skb) {
ATH5K_ERR(sc, "can't alloc skbuff of size %u\n",
- sc->rxbufsize + sc->common.cachelsz - 1);
+ sc->rxbufsize);
return NULL;
}
*skb_addr = pci_map_single(sc->pdev,
skb->data, sc->rxbufsize, PCI_DMA_FROMDEVICE);
+
if (unlikely(pci_dma_mapping_error(sc->pdev, *skb_addr))) {
ATH5K_ERR(sc, "%s: DMA mapping failed\n", __func__);
dev_kfree_skb(skb);
@@ -1607,9 +1608,10 @@ ath5k_rx_start(struct ath5k_softc *sc)
int ret;
sc->rxbufsize = roundup(IEEE80211_MAX_LEN, sc->common.cachelsz);
+ sc->rxbufsize += sc->common.cachelsz - 1;
ATH5K_DBG(sc, ATH5K_DEBUG_RESET, "cachelsz %u rxbufsize %u\n",
- sc->common.cachelsz, sc->rxbufsize);
+ sc->common.cachelsz, sc->rxbufsize);
spin_lock_bh(&sc->rxbuflock);
sc->rxlink = NULL;
--
1.6.3.3
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] ath5k: fix requested allocated RX skb size for DMA
2009-08-12 16:58 [PATCH] ath5k: fix requested allocated RX skb size for DMA Luis R. Rodriguez
@ 2009-08-12 18:13 ` Jiri Slaby
2009-08-12 18:48 ` [ath9k-devel] " Luis R. Rodriguez
0 siblings, 1 reply; 5+ messages in thread
From: Jiri Slaby @ 2009-08-12 18:13 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: linville, linux-wireless, ath9k-devel, ath5k-devel,
Nick Kossifidis, Bob Copeland
On 08/12/2009 06:58 PM, Luis R. Rodriguez wrote:
> --- a/drivers/net/wireless/ath/ath5k/base.c
> +++ b/drivers/net/wireless/ath/ath5k/base.c
> @@ -1157,17 +1157,18 @@ struct sk_buff *ath5k_rx_skb_alloc(struct ath5k_softc *sc, dma_addr_t *skb_addr)
> * fake physical layer header at the start.
> */
> skb = ath_rxbuf_alloc(&sc->common,
> - sc->rxbufsize + sc->common.cachelsz - 1,
What was the exact purpose of this? My guess is that we should map
starting at a next cache line boundary and put this aligned address to
the device instead?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [ath9k-devel] [PATCH] ath5k: fix requested allocated RX skb size for DMA
2009-08-12 18:13 ` Jiri Slaby
@ 2009-08-12 18:48 ` Luis R. Rodriguez
2009-08-12 21:08 ` Bob Copeland
0 siblings, 1 reply; 5+ messages in thread
From: Luis R. Rodriguez @ 2009-08-12 18:48 UTC (permalink / raw)
To: Jiri Slaby
Cc: linux-wireless, ath5k-devel, Bob Copeland, linville, ath9k-devel,
Nick Kossifidis
On Wed, Aug 12, 2009 at 11:13 AM, Jiri Slaby<jirislaby@gmail.com> wrote:
> On 08/12/2009 06:58 PM, Luis R. Rodriguez wrote:
>> --- a/drivers/net/wireless/ath/ath5k/base.c
>> +++ b/drivers/net/wireless/ath/ath5k/base.c
>> @@ -1157,17 +1157,18 @@ struct sk_buff *ath5k_rx_skb_alloc(struct ath5k_softc *sc, dma_addr_t *skb_addr)
>> * fake physical layer header at the start.
>> */
>> skb = ath_rxbuf_alloc(&sc->common,
>> - sc->rxbufsize + sc->common.cachelsz - 1,
>
> What was the exact purpose of this? My guess is that we should map
> starting at a next cache line boundary and put this aligned address to
> the device instead?
I've heard two theories:
1) AR5210 *required* it otherwise bad we would get bad data
2) Performance considerations
I've tried checking internally but haven't found out yet the exact
answer and this seems purely historical. I poked Sam Leffler to see if
he recalls. But as we the 0x41 reset which I removed it seems to be
best to leave unless we can be 100% sure we can remove this.
Luis
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [ath9k-devel] [PATCH] ath5k: fix requested allocated RX skb size for DMA
2009-08-12 18:48 ` [ath9k-devel] " Luis R. Rodriguez
@ 2009-08-12 21:08 ` Bob Copeland
2009-08-13 3:33 ` Luis R. Rodriguez
0 siblings, 1 reply; 5+ messages in thread
From: Bob Copeland @ 2009-08-12 21:08 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Jiri Slaby, linux-wireless, ath5k-devel, linville, ath9k-devel,
Nick Kossifidis
On Wed, Aug 12, 2009 at 2:48 PM, Luis R.
Rodriguez<lrodriguez@atheros.com> wrote:
> On Wed, Aug 12, 2009 at 11:13 AM, Jiri Slaby<jirislaby@gmail.com> wrote:
>> What was the exact purpose of this? My guess is that we should map
>> starting at a next cache line boundary and put this aligned address to
>> the device instead?
>
> I've heard two theories:
>
> 1) AR5210 *required* it otherwise bad we would get bad data
> 2) Performance considerations
>From what I can tell Jiri has the right guess:
// ?? nice pointer arithmetic... should use PTR_ALIGN here?
off = ((unsigned long) skb->data) % sc->cachelsz;
if (off != 0)
skb_reserve(skb, sc->cachelsz);
in other words, when we allocate, round up to the next cache line
greater than IEEE80211_MAX_LEN, then add an extra cache_line-1
bytes so we can map starting from it.
dev_alloc_skb already does some padding and alignment, and it's
configurable on a per-arch basis (though looks like only powerpc
sets it to L1 cache size, everywhere else it's 32 bytes.)
I guess if someone is bored some benchmarking would be useful.
--
Bob Copeland %% www.bobcopeland.com
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [ath9k-devel] [PATCH] ath5k: fix requested allocated RX skb size for DMA
2009-08-12 21:08 ` Bob Copeland
@ 2009-08-13 3:33 ` Luis R. Rodriguez
0 siblings, 0 replies; 5+ messages in thread
From: Luis R. Rodriguez @ 2009-08-13 3:33 UTC (permalink / raw)
To: Bob Copeland
Cc: Jiri Slaby, linux-wireless, ath5k-devel, linville, ath9k-devel,
Nick Kossifidis
John please ignore this patch, its busted, as Bob noted on IRC.
Luis
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2009-08-13 3:33 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-08-12 16:58 [PATCH] ath5k: fix requested allocated RX skb size for DMA Luis R. Rodriguez
2009-08-12 18:13 ` Jiri Slaby
2009-08-12 18:48 ` [ath9k-devel] " Luis R. Rodriguez
2009-08-12 21:08 ` Bob Copeland
2009-08-13 3:33 ` Luis R. Rodriguez
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).