From: Ben Greear <greearb@candelatech.com>
To: greearb@candelatech.com
Cc: linux-wireless@vger.kernel.org, ath9k-devel@venema.h4ckr.net
Subject: Re: [PATCH] ath9k: Clear DMA addresses when buffers are freed.
Date: Thu, 28 Oct 2010 16:33:28 -0700 [thread overview]
Message-ID: <4CCA0848.7080001@candelatech.com> (raw)
In-Reply-To: <1287178199-7506-1-git-send-email-greearb@candelatech.com>
On 10/15/2010 02:29 PM, greearb@candelatech.com wrote:
> From: Ben Greear<greearb@candelatech.com>
>
> Previous patches cleared the pointer to the SKB, but did not
> clear the DMA addresses. This patch moves the clear logic into
> a method and makes sure that the DMA addresses are cleared as well.
>
> This does not obviously fix any bug, but may make it harder to
> cause bugs in the future.
I wanted to follow up to see if there was any interest in this patch,
as it received no comment and has not been accepted upstream.
Thanks,
Ben
>
> Signed-off-by: Ben Greear<greearb@candelatech.com>
> ---
> :100644 100644 0c917e5... 8fba13d... M drivers/net/wireless/ath/ath9k/ath9k.h
> :100644 100644 97d471f... cc406f9... M drivers/net/wireless/ath/ath9k/beacon.c
> :100644 100644 62294da... 1560280... M drivers/net/wireless/ath/ath9k/main.c
> :100644 100644 fd778d2... 5afb46f... M drivers/net/wireless/ath/ath9k/recv.c
> :100644 100644 a5e5f27... e86f59c... M drivers/net/wireless/ath/ath9k/xmit.c
> drivers/net/wireless/ath/ath9k/ath9k.h | 2 ++
> drivers/net/wireless/ath/ath9k/beacon.c | 14 +++++---------
> drivers/net/wireless/ath/ath9k/main.c | 11 +++++++++++
> drivers/net/wireless/ath/ath9k/recv.c | 12 ++++--------
> drivers/net/wireless/ath/ath9k/xmit.c | 6 ++----
> 5 files changed, 24 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
> index 0c917e5..8fba13d 100644
> --- a/drivers/net/wireless/ath/ath9k/ath9k.h
> +++ b/drivers/net/wireless/ath/ath9k/ath9k.h
> @@ -737,4 +737,6 @@ bool ath_mac80211_start_queue(struct ath_softc *sc, u16 skb_queue);
> void ath_start_rfkill_poll(struct ath_softc *sc);
> extern void ath9k_rfkill_poll_state(struct ieee80211_hw *hw);
>
> +void ath_clear_dma_ptrs(struct ath_buf *bf);
> +
> #endif /* ATH9K_H */
> diff --git a/drivers/net/wireless/ath/ath9k/beacon.c b/drivers/net/wireless/ath/ath9k/beacon.c
> index 97d471f..cc406f9 100644
> --- a/drivers/net/wireless/ath/ath9k/beacon.c
> +++ b/drivers/net/wireless/ath/ath9k/beacon.c
> @@ -139,7 +139,7 @@ static struct ath_buf *ath_beacon_generate(struct ieee80211_hw *hw,
> dma_unmap_single(sc->dev, bf->bf_buf_addr,
> skb->len, DMA_TO_DEVICE);
> dev_kfree_skb_any(skb);
> - bf->bf_buf_addr = 0;
> + ath_clear_dma_ptrs(bf);
> }
>
> /* Get a new beacon from mac80211 */
> @@ -167,8 +167,7 @@ static struct ath_buf *ath_beacon_generate(struct ieee80211_hw *hw,
> skb->len, DMA_TO_DEVICE);
> if (unlikely(dma_mapping_error(sc->dev, bf->bf_buf_addr))) {
> dev_kfree_skb_any(skb);
> - bf->bf_mpdu = NULL;
> - bf->bf_buf_addr = 0;
> + ath_clear_dma_ptrs(bf);
> ath_print(common, ATH_DBG_FATAL,
> "dma_mapping_error on beaconing\n");
> return NULL;
> @@ -256,8 +255,7 @@ int ath_beacon_alloc(struct ath_wiphy *aphy, struct ieee80211_vif *vif)
> dma_unmap_single(sc->dev, bf->bf_buf_addr,
> skb->len, DMA_TO_DEVICE);
> dev_kfree_skb_any(skb);
> - bf->bf_mpdu = NULL;
> - bf->bf_buf_addr = 0;
> + ath_clear_dma_ptrs(bf);
> }
>
> /* NB: the beacon data buffer must be 32-bit aligned. */
> @@ -302,8 +300,7 @@ int ath_beacon_alloc(struct ath_wiphy *aphy, struct ieee80211_vif *vif)
> skb->len, DMA_TO_DEVICE);
> if (unlikely(dma_mapping_error(sc->dev, bf->bf_buf_addr))) {
> dev_kfree_skb_any(skb);
> - bf->bf_mpdu = NULL;
> - bf->bf_buf_addr = 0;
> + ath_clear_dma_ptrs(bf);
> ath_print(common, ATH_DBG_FATAL,
> "dma_mapping_error on beacon alloc\n");
> return -ENOMEM;
> @@ -329,8 +326,7 @@ void ath_beacon_return(struct ath_softc *sc, struct ath_vif *avp)
> dma_unmap_single(sc->dev, bf->bf_buf_addr,
> skb->len, DMA_TO_DEVICE);
> dev_kfree_skb_any(skb);
> - bf->bf_mpdu = NULL;
> - bf->bf_buf_addr = 0;
> + ath_clear_dma_ptrs(bf);
> }
> list_add_tail(&bf->list,&sc->beacon.bbuf);
>
> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
> index 62294da..1560280 100644
> --- a/drivers/net/wireless/ath/ath9k/main.c
> +++ b/drivers/net/wireless/ath/ath9k/main.c
> @@ -213,6 +213,17 @@ static void ath_update_survey_stats(struct ath_softc *sc)
> ath_update_survey_nf(sc, pos);
> }
>
> +void ath_clear_dma_ptrs(struct ath_buf *bf)
> +{
> + struct ath_desc *ds = bf->bf_desc;
> + bf->bf_buf_addr = 0;
> + bf->bf_mpdu = NULL;
> + if (ds) {
> + ds->ds_data = 0;
> + ds->ds_vdata = NULL;
> + }
> +}
> +
> /*
> * Set/change channels. If the channel is really being changed, it's done
> * by reseting the chip. To accomplish this we must first cleanup any pending
> diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
> index fd778d2..5afb46f 100644
> --- a/drivers/net/wireless/ath/ath9k/recv.c
> +++ b/drivers/net/wireless/ath/ath9k/recv.c
> @@ -269,8 +269,7 @@ static int ath_rx_edma_init(struct ath_softc *sc, int nbufs)
> if (unlikely(dma_mapping_error(sc->dev,
> bf->bf_buf_addr))) {
> dev_kfree_skb_any(skb);
> - bf->bf_mpdu = NULL;
> - bf->bf_buf_addr = 0;
> + ath_clear_dma_ptrs(bf);
> ath_print(common, ATH_DBG_FATAL,
> "dma_mapping_error() on RX init\n");
> error = -ENOMEM;
> @@ -360,8 +359,7 @@ int ath_rx_init(struct ath_softc *sc, int nbufs)
> if (unlikely(dma_mapping_error(sc->dev,
> bf->bf_buf_addr))) {
> dev_kfree_skb_any(skb);
> - bf->bf_mpdu = NULL;
> - bf->bf_buf_addr = 0;
> + ath_clear_dma_ptrs(bf);
> ath_print(common, ATH_DBG_FATAL,
> "dma_mapping_error() on RX init\n");
> error = -ENOMEM;
> @@ -396,8 +394,7 @@ void ath_rx_cleanup(struct ath_softc *sc)
> common->rx_bufsize,
> DMA_FROM_DEVICE);
> dev_kfree_skb(skb);
> - bf->bf_buf_addr = 0;
> - bf->bf_mpdu = NULL;
> + ath_clear_dma_ptrs(bf);
> }
> }
>
> @@ -1741,8 +1738,7 @@ int ath_rx_tasklet(struct ath_softc *sc, int flush, bool hp)
> if (unlikely(dma_mapping_error(sc->dev,
> bf->bf_buf_addr))) {
> dev_kfree_skb_any(requeue_skb);
> - bf->bf_mpdu = NULL;
> - bf->bf_buf_addr = 0;
> + ath_clear_dma_ptrs(bf);
> ath_print(common, ATH_DBG_FATAL,
> "dma_mapping_error() on RX\n");
> ath_rx_send_to_mac80211(hw, sc, skb, rxs);
> diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
> index a5e5f27..e86f59c 100644
> --- a/drivers/net/wireless/ath/ath9k/xmit.c
> +++ b/drivers/net/wireless/ath/ath9k/xmit.c
> @@ -1644,8 +1644,7 @@ static int ath_tx_setup_buffer(struct ieee80211_hw *hw, struct ath_buf *bf,
> bf->bf_buf_addr = dma_map_single(sc->dev, skb->data,
> skb->len, DMA_TO_DEVICE);
> if (unlikely(dma_mapping_error(sc->dev, bf->bf_buf_addr))) {
> - bf->bf_mpdu = NULL;
> - bf->bf_buf_addr = 0;
> + ath_clear_dma_ptrs(bf);
> ath_print(ath9k_hw_common(sc->sc_ah), ATH_DBG_FATAL,
> "dma_mapping_error() on TX\n");
> return -ENOMEM;
> @@ -1915,7 +1914,6 @@ static void ath_tx_complete_buf(struct ath_softc *sc, struct ath_buf *bf,
> }
>
> dma_unmap_single(sc->dev, bf->bf_buf_addr, skb->len, DMA_TO_DEVICE);
> - bf->bf_buf_addr = 0;
>
> if (bf->bf_state.bfs_paprd) {
> if (time_after(jiffies,
> @@ -1931,7 +1929,7 @@ static void ath_tx_complete_buf(struct ath_softc *sc, struct ath_buf *bf,
> /* At this point, skb (bf->bf_mpdu) is consumed...make sure we don't
> * accidentally reference it later.
> */
> - bf->bf_mpdu = NULL;
> + ath_clear_dma_ptrs(bf);
>
> /*
> * Return the list of ath_buf of this mpdu to free queue
--
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc http://www.candelatech.com
next prev parent reply other threads:[~2010-10-28 23:33 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-10-15 21:29 [PATCH] ath9k: Clear DMA addresses when buffers are freed greearb
2010-10-28 23:33 ` Ben Greear [this message]
2010-10-28 23:36 ` [ath9k-devel] " Luis R. Rodriguez
2010-10-28 23:44 ` Ben Greear
2010-10-29 13:21 ` John W. Linville
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4CCA0848.7080001@candelatech.com \
--to=greearb@candelatech.com \
--cc=ath9k-devel@venema.h4ckr.net \
--cc=linux-wireless@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).