On 2018-09-28 00:51, Toke Høiland-Jørgensen wrote:
On 28 September 2018 07:29:03 CEST, Rajkumar Manoharan
<rmanohar@xxxxxxxxxxxxxx> wrote:
On 2018-09-26 17:09, Rajkumar Manoharan wrote:
On 2018-09-26 02:22, Toke Høiland-Jørgensen wrote:
Rajkumar Manoharan <rmanohar@xxxxxxxxxxxxxx> writes:
:( Yeah... I got confused with attached soft lockup in ARM platform.
Toke,
Cause for the soft lockup exposed in multi client scenario is due to
mixed order of fq_lock and active_txqs_lock. In wake_tx_queue or
push_pending
case, driver acquires active_txq_lock first by schedule_start and
followed by
fq_lock in tx_dequeue. The same order should be maintained in sta
cleanup.
Below change fixed the issue.
Ah, great find! I'll fold this into the next version, thanks!
One more thing. As I mentioned earlier, scheduling wake_txqs_tasklet
is heavy load and causing random rcu stall issue. Hence I added
another API to schedule throttled txqs once for all. Also I did
a cleanup in kick_airtime by traversing list only once. With these
changes I don't see rcu stall issue. Please review and fold them as
well.
-Rajkumar
single_iter - clean up kick_airtime
sched_throttle - new API and separate tasklet for throttled txqs
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 404c5e82e4ca..023bc81bd4a0 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -242,13 +242,11 @@ EXPORT_SYMBOL(ieee80211_ctstoself_duration);
static void __ieee80211_kick_airtime(struct ieee80211_local *local, int ac)
{
- bool seen_eligible = false;
struct txq_info *txqi;
struct sta_info *sta;
spin_lock_bh(&local->active_txq_lock[ac]);
- begin:
if (list_empty(&local->active_txqs[ac]))
goto out;
@@ -258,12 +256,12 @@ static void __ieee80211_kick_airtime(struct ieee80211_local *local, int ac)
sta = container_of(txqi->txq.sta, struct sta_info, sta);
- if (sta->airtime[ac].deficit >= 0) {
- seen_eligible = true;
-
- if (!test_and_clear_bit(IEEE80211_TXQ_AIRTIME_THROTTLE,
- &txqi->flags))
+ if (test_bit(IEEE80211_TXQ_AIRTIME_THROTTLE, &txqi->flags)) {
+ clear_bit(IEEE80211_TXQ_AIRTIME_THROTTLE, &txqi->flags);
+ if (sta->airtime[ac].deficit < 0) {
+ sta->airtime[ac].deficit += sta->airtime_weight;
continue;
+ }
spin_unlock_bh(&local->active_txq_lock[ac]);
drv_wake_tx_queue(local, txqi);
@@ -271,21 +269,6 @@ static void __ieee80211_kick_airtime(struct ieee80211_local *local, int ac)
}
}
- /* No active queues have positive deficit, which means TX is stuck;
- * increase all deficits to get things unstuck.
- */
- if (!seen_eligible) {
- list_for_each_entry(txqi, &local->active_txqs[ac], schedule_order) {
- if (!txqi->txq.sta)
- continue;
-
- sta = container_of(txqi->txq.sta, struct sta_info, sta);
-
- sta->airtime[ac].deficit += sta->airtime_weight;
- }
-
- goto begin;
- }
out:
spin_unlock_bh(&local->active_txq_lock[ac]);
diff --git a/drivers/net/wireless/ath/ath10k/htt_rx.c b/drivers/net/wireless/ath/ath10k/htt_rx.c
index 625a4ab37ea0..b1bb369b0971 100644
--- a/drivers/net/wireless/ath/ath10k/htt_rx.c
+++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
@@ -2362,6 +2362,7 @@ static void ath10k_htt_rx_tx_fetch_ind(struct ath10k *ar, struct sk_buff *skb)
}
rcu_read_unlock();
+ ieee80211_txq_schedule_throttled(hw);
resp_ids = ath10k_htt_get_tx_fetch_ind_resp_ids(&resp->tx_fetch_ind);
ath10k_htt_rx_tx_fetch_resp_id_confirm(ar, resp_ids, num_resp_ids);
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 50c1082c86d3..8101936a9c9c 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -6124,6 +6124,7 @@ void ieee80211_txq_schedule_start(struct ieee80211_hw *hw, u8 ac);
*/
void ieee80211_txq_schedule_end(struct ieee80211_hw *hw, u8 ac);
+void ieee80211_txq_schedule_throttled(struct ieee80211_hw *hw);
/**
* ieee80211_txq_may_transmit - check whether TXQ is allowed to transmit
*
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index c138b9d47453..329ebd2e1594 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1254,6 +1254,7 @@ struct ieee80211_local {
struct sk_buff_head pending[IEEE80211_MAX_QUEUES];
struct tasklet_struct tx_pending_tasklet;
struct tasklet_struct wake_txqs_tasklet;
+ struct tasklet_struct kick_airtime;
atomic_t agg_queue_stop[IEEE80211_MAX_QUEUES];
@@ -2068,6 +2069,7 @@ void ieee80211_txq_purge(struct ieee80211_local *local,
void ieee80211_txq_remove_vlan(struct ieee80211_local *local,
struct ieee80211_sub_if_data *sdata);
void ieee80211_wake_txqs(unsigned long data);
+void ieee80211_kick_airtime(unsigned long data);
void ieee80211_send_auth(struct ieee80211_sub_if_data *sdata,
u16 transaction, u16 auth_alg, u16 status,
const u8 *extra, size_t extra_len, const u8 *bssid,
diff --git a/net/mac80211/main.c b/net/mac80211/main.c
index c4c3feaea336..f2772d15a541 100644
--- a/net/mac80211/main.c
+++ b/net/mac80211/main.c
@@ -675,9 +675,12 @@ struct ieee80211_hw *ieee80211_alloc_hw_nm(size_t priv_data_len,
tasklet_init(&local->tx_pending_tasklet, ieee80211_tx_pending,
(unsigned long)local);
- if (ops->wake_tx_queue)
+ if (ops->wake_tx_queue) {
tasklet_init(&local->wake_txqs_tasklet, ieee80211_wake_txqs,
(unsigned long)local);
+ tasklet_init(&local->kick_airtime, ieee80211_kick_airtime,
+ (unsigned long)local);
+ }
tasklet_init(&local->tasklet,
ieee80211_tasklet_handler,
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 5d70f64a205c..73287f5eedd8 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -3696,6 +3696,14 @@ bool ieee80211_txq_may_transmit(struct ieee80211_hw *hw,
}
EXPORT_SYMBOL(ieee80211_txq_may_transmit);
+void ieee80211_txq_schedule_throttled(struct ieee80211_hw *hw)
+{
+ struct ieee80211_local *local = hw_to_local(hw);
+
+ tasklet_schedule(&local->wake_txqs_tasklet);
+}
+EXPORT_SYMBOL(ieee80211_txq_schedule_throttled);
+
void ieee80211_txq_schedule_start(struct ieee80211_hw *hw, u8 ac)
{
struct ieee80211_local *local = hw_to_local(hw);
@@ -3710,7 +3718,6 @@ void ieee80211_txq_schedule_end(struct ieee80211_hw *hw, u8 ac)
struct ieee80211_local *local = hw_to_local(hw);
spin_unlock_bh(&local->active_txq_lock[ac]);
- tasklet_schedule(&local->wake_txqs_tasklet);
}
EXPORT_SYMBOL(ieee80211_txq_schedule_end);
diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 023bc81bd4a0..69fc3c723add 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -246,6 +246,7 @@ static void __ieee80211_kick_airtime(struct ieee80211_local *local, int ac)
struct sta_info *sta;
spin_lock_bh(&local->active_txq_lock[ac]);
+ rcu_read_lock();
if (list_empty(&local->active_txqs[ac]))
goto out;
@@ -270,6 +271,7 @@ static void __ieee80211_kick_airtime(struct ieee80211_local *local, int ac)
}
out:
+ rcu_read_unlock();
spin_unlock_bh(&local->active_txq_lock[ac]);
}
@@ -284,10 +286,6 @@ static void __ieee80211_wake_txqs(struct ieee80211_sub_if_data *sdata, int ac)
struct sta_info *sta;
int i;
- if (wiphy_ext_feature_isset(local->hw.wiphy,
- NL80211_EXT_FEATURE_AIRTIME_FAIRNESS))
- __ieee80211_kick_airtime(local, ac);
-
spin_lock_bh(&fq->lock);
if (sdata->vif.type == NL80211_IFTYPE_AP)
@@ -334,6 +332,15 @@ out:
spin_unlock_bh(&fq->lock);
}
+void ieee80211_kick_airtime(unsigned long data)
+{
+ struct ieee80211_local *local = (struct ieee80211_local *)data;
+ int i;
+
+ for (i = 0; i < IEEE80211_NUM_ACS; i++)
+ __ieee80211_kick_airtime(local, i);
+}
+
void ieee80211_wake_txqs(unsigned long data)
{
struct ieee80211_local *local = (struct ieee80211_local *)data;
