2014-04-17 00:43:28

by Ben Greear

[permalink] [raw]
Subject: [PATCH] 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(-)

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



2014-04-17 06:12:07

by Michal Kazior

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

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.


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

2014-05-14 12:59:21

by Michal Kazior

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

On 14 May 2014 14:52, Kalle Valo <[email protected]> wrote:
> Michal Kazior <[email protected]> writes:
>
>> 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.
>>
>>
>>> @@ -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

2014-05-14 13:01:20

by Kalle Valo

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

Michal Kazior <[email protected]> writes:

> On 14 May 2014 14:52, Kalle Valo <[email protected]> wrote:
>> Michal Kazior <[email protected]> writes:
>>
>>> 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.
>>>
>>>
>>>> @@ -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

2014-05-14 12:52:51

by Kalle Valo

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

Michal Kazior <[email protected]> writes:

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