[PATCH] ath10k: Hold tx-lock while detaching htt.

[PATCH] ath10k: Hold tx-lock while detaching htt.

[greearb]

From: Ben Greear <greearb@candelatech.com>

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

Signed-off-by: Ben Greear <greearb@candelatech.com>
---
 drivers/net/wireless/ath/ath10k/htt_tx.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c b/drivers/net/wireless/ath/ath10k/htt_tx.c
index 22a4542..0698d6f 100644
--- a/drivers/net/wireless/ath/ath10k/htt_tx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_tx.c
@@ -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);
-- 
1.7.11.7

Re: [PATCH] ath10k: Hold tx-lock while detaching htt.

[Michal Kazior]

On 17 April 2014 02:42,  <greearb@candelatech.com> wrote:
> From: Ben Greear <greearb@candelatech.com>
>
> 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.


> @@ -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ł

Re: [PATCH] ath10k: Hold tx-lock while detaching htt.

[Kalle Valo]

Michal Kazior <michal.kazior@tieto.com> writes:

> On 17 April 2014 02:42,  <greearb@candelatech.com> wrote:
>> From: Ben Greear <greearb@candelatech.com>
>>
>> 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.
>
>
>> @@ -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 !.

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

-- 
Kalle Valo

Re: [PATCH] ath10k: Hold tx-lock while detaching htt.

[Michal Kazior]

On 14 May 2014 14:52, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
> Michal Kazior <michal.kazior@tieto.com> writes:
>
>> On 17 April 2014 02:42,  <greearb@candelatech.com> wrote:
>>> From: Ben Greear <greearb@candelatech.com>
>>>
>>> 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.
>>
>>
>>> @@ -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 !.
>
> 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

Re: [PATCH] ath10k: Hold tx-lock while detaching htt.

[Kalle Valo]

Michal Kazior <michal.kazior@tieto.com> writes:

> On 14 May 2014 14:52, Kalle Valo <kvalo@qca.qualcomm.com> wrote:
>> Michal Kazior <michal.kazior@tieto.com> writes:
>>
>>> On 17 April 2014 02:42,  <greearb@candelatech.com> wrote:
>>>> From: Ben Greear <greearb@candelatech.com>
>>>>
>>>> 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.
>>>
>>>
>>>> @@ -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 !.
>>
>> 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.

-- 
Kalle Valo