Re: [Make-wifi-fast] [PATCH RFC v4 3/4] mac80211: Add airtime accounting and scheduling to TXQs

[Rajkumar Manoharan <rmanohar@codeaurora.org>]

On 2018-10-02 16:07, Rajkumar Manoharan wrote:
> On 2018-10-02 12:00, Toke Høiland-Jørgensen wrote:
>> Rajkumar Manoharan <rmanohar@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