| Message ID | 1397695357-14028-1-git-send-email-greearb@candelatech.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
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? -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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?
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 -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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);