| Message ID | 1576221593-1086-3-git-send-email-yiboz@codeaurora.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Johannes Berg |
| Headers | show |
| Series | Enable virtual time-based airtime scheduler support on ath10k | expand |
On Fri, 2019-12-13 at 15:19 +0800, Yibo Zhao wrote: > In a loop txqs dequeue scenario, if the first txq in the rbtree gets > removed from rbtree immediately in the ieee80211_return_txq(), the > loop will break soon in the ieee80211_next_txq() due to schedule_pos > not leading to the second txq in the rbtree. Thus update schedule_pos > to previous node once the node of schedule_pos is either removed from > rbtree or move to other location in rbtree due to airtime update. For my understanding - this is a fix to the first patch in the series? I guess you didn't squash it because that's Toke's patch or something? I tend to think you still should, and annotate the changes, but I wanted to understand it first. johannes
在 2019-12-13 17:56，Johannes Berg 写道： > On Fri, 2019-12-13 at 15:19 +0800, Yibo Zhao wrote: >> In a loop txqs dequeue scenario, if the first txq in the rbtree gets >> removed from rbtree immediately in the ieee80211_return_txq(), the >> loop will break soon in the ieee80211_next_txq() due to schedule_pos >> not leading to the second txq in the rbtree. Thus update schedule_pos >> to previous node once the node of schedule_pos is either removed from >> rbtree or move to other location in rbtree due to airtime update. > > For my understanding - this is a fix to the first patch in the series? > > I guess you didn't squash it because that's Toke's patch or something? > > I tend to think you still should, and annotate the changes, but I > wanted > to understand it first. Hi Johannes, Yes, this is a fix to the first patch. Actually, the rest of two patches are also serve the same. So, are you suggesting to merge them to the first patch? Previouly, I had added Toke's signature in this patch but Toke advised to delete it. I feel a little bit confused about how to handle it. Appreciate any detail guide. > > johannes
On Wed, 2019-12-18 at 18:12 +0800, yiboz@codeaurora.org wrote: > > Yes, this is a fix to the first patch. Actually, the rest of two patches > are also serve the same. So, are you suggesting to merge them to the > first patch? Yes. > Previouly, I had added Toke's signature in this patch but Toke advised > to delete it. I feel a little bit confused about how to handle it. > Appreciate any detail guide. I guess that you have to discuss that with Toke, how he wants to handle it ... If he gave you a patch with his signed-off-by, then you should probably keep it? There's also "Co-developed-by" tag to give some sort of non-author developer credits. johannes
Johannes Berg <johannes@sipsolutions.net> writes: > On Wed, 2019-12-18 at 18:12 +0800, yiboz@codeaurora.org wrote: >> >> Yes, this is a fix to the first patch. Actually, the rest of two patches >> are also serve the same. So, are you suggesting to merge them to the >> first patch? > > Yes. > >> Previouly, I had added Toke's signature in this patch but Toke advised >> to delete it. I feel a little bit confused about how to handle it. >> Appreciate any detail guide. > > I guess that you have to discuss that with Toke, how he wants to handle > it ... If he gave you a patch with his signed-off-by, then you should > probably keep it? There's also "Co-developed-by" tag to give some sort > of non-author developer credits. I'll do some squashing and send a new version; in which case I think it makes sense to have both our s-o-b tags, and maybe a co-developed-by. I'm hoping to get this done before the holidays (i.e., this week). Already got everything rebased to current mac80211-next, just need to do the squashing and fix up the other outstanding issues from last time. -Toke
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h index a4556f9..ed85400 100644 --- a/net/mac80211/ieee80211_i.h +++ b/net/mac80211/ieee80211_i.h @@ -847,6 +847,7 @@ struct txq_info { struct codel_stats cstats; struct sk_buff_head frags; struct rb_node schedule_order; + u16 schedule_round; unsigned long flags; /* keep last! */ @@ -1144,6 +1145,7 @@ struct ieee80211_local { struct rb_node *schedule_pos[IEEE80211_NUM_ACS]; u64 airtime_v_t[IEEE80211_NUM_ACS]; u64 airtime_weight_sum[IEEE80211_NUM_ACS]; + u16 schedule_round[IEEE80211_NUM_ACS]; u16 airtime_flags; diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c index d00baaa..c1444e7 100644 --- a/net/mac80211/tx.c +++ b/net/mac80211/tx.c @@ -3644,6 +3644,7 @@ struct ieee80211_txq *ieee80211_next_txq(struct ieee80211_hw *hw, u8 ac) lockdep_assert_held(&local->active_txq_lock[ac]); +begin: if (!node) { node = rb_first_cached(&local->active_txqs[ac]); first = true; @@ -3668,7 +3669,10 @@ struct ieee80211_txq *ieee80211_next_txq(struct ieee80211_hw *hw, u8 ac) } } + if (txqi->schedule_round == local->schedule_round[ac]) + goto begin; + txqi->schedule_round = local->schedule_round[ac]; local->schedule_pos[ac] = node; return &txqi->txq; } @@ -3752,6 +3756,9 @@ void ieee80211_resort_txq(struct ieee80211_hw *hw, u8 ac = txq->ac; if (!RB_EMPTY_NODE(&txqi->schedule_order)) { + if (local->schedule_pos[ac] == &txqi->schedule_order) + local->schedule_pos[ac] = rb_prev(&txqi->schedule_order); + rb_erase_cached(&txqi->schedule_order, &local->active_txqs[ac]); RB_CLEAR_NODE(&txqi->schedule_order); @@ -3771,6 +3778,9 @@ static void __ieee80211_unschedule_txq(struct ieee80211_hw *hw, if (RB_EMPTY_NODE(&txqi->schedule_order)) return; + if (local->schedule_pos[ac] == &txqi->schedule_order) + local->schedule_pos[ac] = rb_prev(&txqi->schedule_order); + if (txq->sta) { struct sta_info *sta = container_of(txq->sta, struct sta_info, sta); @@ -3803,7 +3813,7 @@ void ieee80211_return_txq(struct ieee80211_hw *hw, lockdep_assert_held(&local->active_txq_lock[txq->ac]); if (!RB_EMPTY_NODE(&txqi->schedule_order) && - (skb_queue_empty(&txqi->frags) && !txqi->tin.backlog_packets)) + !txq_has_queue(&txqi->txq)) __ieee80211_unschedule_txq(hw, txq); } EXPORT_SYMBOL(ieee80211_return_txq); @@ -3832,6 +3842,8 @@ void ieee80211_txq_schedule_start(struct ieee80211_hw *hw, u8 ac) struct ieee80211_local *local = hw_to_local(hw); spin_lock_bh(&local->active_txq_lock[ac]); + local->schedule_round[ac]++; + } EXPORT_SYMBOL(ieee80211_txq_schedule_start);
In a loop txqs dequeue scenario, if the first txq in the rbtree gets removed from rbtree immediately in the ieee80211_return_txq(), the loop will break soon in the ieee80211_next_txq() due to schedule_pos not leading to the second txq in the rbtree. Thus update schedule_pos to previous node once the node of schedule_pos is either removed from rbtree or move to other location in rbtree due to airtime update. Signed-off-by: Yibo Zhao <yiboz@codeaurora.org> --- net/mac80211/ieee80211_i.h | 2 ++ net/mac80211/tx.c | 14 +++++++++++++- 2 files changed, 15 insertions(+), 1 deletion(-)