On 2018-10-02 16:07, Rajkumar Manoharan wrote:
On 2018-10-02 12:00, Toke Høiland-Jørgensen wrote:
Rajkumar Manoharan <rmano...@codeaurora.org> writes:
I noticed a race condition b/w sta cleanup and kick_airtime tasklet.
How do you plan to exit kick_airtime gracefully during sta_cleanup?

Ah, right, there's a lot of stuff going on before we get to purge_txq.
Hmm, I guess we should either make sure we remove the station from
active_txqs earlier in the sta cleanup process, or maybe it'd enough to
just check the removed flag in the tasklet?

Does the below patch fix the issue?


No. Attaching backtrace. Any clue?

Ah, that's my bad. Just having a 'continue' there can make the function
loop forever. Oops. Try something like this instead?


But 'continue' also used in other places. Will give a try but I think that
calling drv_wake_tx_queue within iteration is dangerous as it alters
the list. no?

How about below change? Just schedule first txq and remaining will be
scheduled later by driver upon tx-compl.

diff --git a/net/mac80211/util.c b/net/mac80211/util.c
index 0bb590928dd0..2dbfd1d8eb5f 100644
--- a/net/mac80211/util.c
+++ b/net/mac80211/util.c
@@ -242,6 +242,7 @@ EXPORT_SYMBOL(ieee80211_ctstoself_duration);

static void __ieee80211_kick_airtime(struct ieee80211_local *local, int ac)
 {
+       struct ieee80211_txq *txq;
        bool seen_eligible = false;
        struct txq_info *txqi;
        struct sta_info *sta;
@@ -261,14 +262,7 @@ static void __ieee80211_kick_airtime(struct ieee80211_local *local, int ac)

                if (sta->airtime[ac].deficit >= 0) {
                        seen_eligible = true;
-
- if (!test_and_clear_bit(IEEE80211_TXQ_AIRTIME_THROTTLE,
-                                               &txqi->flags))
-                               continue;
-
-                       spin_unlock_bh(&local->active_txq_lock[ac]);
-                       drv_wake_tx_queue(local, txqi);
-                       spin_lock_bh(&local->active_txq_lock[ac]);
+ clear_bit(IEEE80211_TXQ_AIRTIME_THROTTLE, &txqi->flags);
                }
        }

@@ -289,8 +283,10 @@ static void __ieee80211_kick_airtime(struct ieee80211_local *local, int ac)
        }
  out:
        rcu_read_unlock();
+       txq = ieee80211_next_txq(&local->hw, ac);
        spin_unlock_bh(&local->active_txq_lock[ac]);
-
+       if (txq)
+               drv_wake_tx_queue(local, to_txq_info(txq));
 }

-Rajkumar

Reply via email to