ath10k: Hold tx-lock while detaching htt.

PATCH In review [email protected] 3 participants 5 messages Apr 17, 2014 – May 14, 2014 Qualcomm Atheros ATH10K wireless driver

lore

Share & tools

lore

[email protected]

author

2014-04-17 00:43 UTC #0 lore raw

PATCH 1/1 ath10k: Hold tx-lock while detaching htt.

From: Ben Greear <[email protected]>

And check for null pool in the tx path (which would indicate we are detaching).

Signed-off-by Ben Greear <[email protected]>

---
 drivers/net/wireless/ath/ath10k/htt_tx.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

drivers/net/wireless/ath/ath10k/htt_tx.c +17 -1

index 22a4542..0698d6f 100644
@@ -143,10 +143,19 @@ static void ath10k_htt_tx_cleanup_pending(struct ath10k_htt *htt)
 
 void ath10k_htt_tx_detach(struct ath10k_htt *htt)
 {
+	struct dma_pool *tx_pool_tmp;
+
 	ath10k_htt_tx_cleanup_pending(htt);
+
+	spin_lock_bh(&htt->tx_lock);
 	kfree(htt->pending_tx);
 	kfree(htt->used_msdu_ids);
-	dma_pool_destroy(htt->tx_pool);
+	tx_pool_tmp = htt->tx_pool;
+	htt->tx_pool = NULL;
+	spin_unlock_bh(&htt->tx_lock);
+
+	dma_pool_destroy(tx_pool_tmp);
+
 	return;
 }

@@ -403,6 +412,13 @@ int ath10k_htt_tx(struct ath10k_htt *htt, struct sk_buff *msdu)
 		goto err;
 
 	spin_lock_bh(&htt->tx_lock);
+
+	/* Check if we are detached... */
+	if (! htt->tx_pool) {
+		spin_unlock_bh(&htt->tx_lock);
+		goto err_tx_dec;
+	}
+
 	res = ath10k_htt_tx_alloc_msdu_id(htt);
 	if (res < 0) {
 		spin_unlock_bh(&htt->tx_lock);

Signature · 7 lines

1.7.11.7


_______________________________________________
ath10k mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/ath10k

Michal Kazior

2014-04-17 06:12 UTC #1 lore raw

Quoted 4 lines · On 17 April 2014 02:42, <[email protected]> wrote:

From: Ben Greear <[email protected]>
And check for null pool in the tx path (which would indicate we are detaching).

Shouldn't we just make sure tx is stopped before we try to detach htt? Stoping mac80211 queues and synchronize_net() should be sufficient I think.

drivers/net/wireless/ath/ath10k/htt_tx.c int ath10k_htt_tx(struct ath10k_htt *htt, struct sk_buff *msdu) · line 418

Quoted 8 lines

drivers/net/wireless/ath/ath10k/htt_tx.c +4 -0

@@ -403,6 +412,13 @@ int ath10k_htt_tx(struct ath10k_htt *htt, struct sk_buff *msdu)
                goto err;
 
        spin_lock_bh(&htt->tx_lock);
+
+       /* Check if we are detached... */
+       if (! htt->tx_pool) {
+               spin_unlock_bh(&htt->tx_lock);

Setting res would be nice to indicate an error. And there's a space after !.

Michał

Kalle Valo

2014-05-14 12:52 UTC #2 lore raw

Quoted 14 lines · Michal Kazior

Quoted 4 lines · On 17 April 2014 02:42, <[email protected]> wrote:
From: Ben Greear <[email protected]>
And check for null pool in the tx path (which would indicate we are detaching).
Shouldn't we just make sure tx is stopped before we try to detach htt? Stoping mac80211 queues and synchronize_net() should be sufficient I think.
Quoted 3 lines
drivers/net/wireless/ath/ath10k/htt_tx.c +0 -0
@@ -403,6 +412,13 @@ int ath10k_htt_tx(struct ath10k_htt *htt, struct sk_buff *msdu)
                goto err;
 

Quoted 7 lines

drivers/net/wireless/ath/ath10k/htt_tx.c int ath10k_htt_tx(struct ath10k_htt *htt, struct sk_buff *msdu) · line 418
Quoted 5 lines
drivers/net/wireless/ath/ath10k/htt_tx.c +4 -0
        spin_lock_bh(&htt->tx_lock);
+
+       /* Check if we are detached... */
+       if (! htt->tx_pool) {
+               spin_unlock_bh(&htt->tx_lock);
Setting res would be nice to indicate an error. And there's a space after !.

Ben, are you planning to send v2? Or what should we do with this patch?

Signature · 1 line

Kalle Valo

Michal Kazior

2014-05-14 12:59 UTC #3 lore raw

Quoted 24 lines · Kalle Valo

Quoted 14 lines · Michal Kazior
Quoted 4 lines · On 17 April 2014 02:42, <[email protected]> wrote:
From: Ben Greear <[email protected]>
And check for null pool in the tx path (which would indicate we are detaching).
Shouldn't we just make sure tx is stopped before we try to detach htt? Stoping mac80211 queues and synchronize_net() should be sufficient I think.
Quoted 3 lines
drivers/net/wireless/ath/ath10k/htt_tx.c +0 -0
@@ -403,6 +412,13 @@ int ath10k_htt_tx(struct ath10k_htt *htt, struct sk_buff *msdu)
                goto err;
 
Quoted 7 lines
drivers/net/wireless/ath/ath10k/htt_tx.c int ath10k_htt_tx(struct ath10k_htt *htt, struct sk_buff *msdu) · line 418
Quoted 5 lines
drivers/net/wireless/ath/ath10k/htt_tx.c +4 -0
        spin_lock_bh(&htt->tx_lock);
+
+       /* Check if we are detached... */
+       if (! htt->tx_pool) {
+               spin_unlock_bh(&htt->tx_lock);
Setting res would be nice to indicate an error. And there's a space after !.

Quoted 1 line

Ben, are you planning to send v2? Or what should we do with this patch?

My patchset ath10k: recovery fixes 2014-05-09 fixes the problem by avoiding the tx to happen when it shouldn't in the first place so Ben's patch isn't necessary anymore I suppose.

Michal

Kalle Valo

2014-05-14 13:01 UTC #4 lore raw

Quoted 25 lines · Michal Kazior

Quoted 24 lines · Kalle Valo
Quoted 21 lines · Michal Kazior
Quoted 4 lines · On 17 April 2014 02:42, <[email protected]> wrote:
From: Ben Greear <[email protected]>
And check for null pool in the tx path (which would indicate we are detaching).
Shouldn't we just make sure tx is stopped before we try to detach htt? Stoping mac80211 queues and synchronize_net() should be sufficient I think.
drivers/net/wireless/ath/ath10k/htt_tx.c int ath10k_htt_tx(struct ath10k_htt *htt, struct sk_buff *msdu) · line 418
Quoted 8 lines
drivers/net/wireless/ath/ath10k/htt_tx.c +4 -0
@@ -403,6 +412,13 @@ int ath10k_htt_tx(struct ath10k_htt *htt, struct sk_buff *msdu)
                goto err;
 
        spin_lock_bh(&htt->tx_lock);
+
+       /* Check if we are detached... */
+       if (! htt->tx_pool) {
+               spin_unlock_bh(&htt->tx_lock);
Setting res would be nice to indicate an error. And there's a space after !.

Quoted 5 lines

Quoted 1 line
Ben, are you planning to send v2? Or what should we do with this patch?
My patchset ath10k: recovery fixes 2014-05-09 fixes the problem by avoiding the tx to happen when it shouldn't in the first place so Ben's patch isn't necessary anymore I suppose.

Ah, that's great. I'll drop this patch then.

Signature · 1 line

Kalle Valo