On 2018-09-28 02:58, Toke Høiland-Jørgensen wrote:
Rajkumar Manoharan <rmanohar@xxxxxxxxxxxxxx> writes:
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;
+ }
This is going to break fairness; we only want to increase deficits when
all stations' deficits are negative. Hence the two loops. Did you see
any problems with those specifically?
No. I didn't see any issue but the loop won't exit until one txq becomes
positive.
Till then the lock won't be released. Hence I converged into single
iteration.
-Rajkumar
