From: Ben Greear <greearb@candelatech.com>
To: ath9k-devel@lists.ath9k.org
Subject: [ath9k-devel] [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 at 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
WARNING: multiple messages have this Message-ID (diff)
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: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-10-15 21:29 [ath9k-devel] [PATCH] ath9k: Clear DMA addresses when buffers are freed greearb at candelatech.com
2010-10-15 21:29 ` greearb
2010-10-28 23:33 ` Ben Greear [this message]
2010-10-28 23:33 ` Ben Greear
2010-10-28 23:36 ` [ath9k-devel] " Luis R. Rodriguez
2010-10-28 23:36 ` Luis R. Rodriguez
2010-10-28 23:44 ` Ben Greear
2010-10-28 23:44 ` Ben Greear
2010-10-29 5:12 ` Peter Stuge
2010-10-29 13:21 ` John W. Linville
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@lists.ath9k.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.