Rajkumar Manoharan [off-list ref] writes:

On 2018-09-10 04:13, Toke H=C3=B8iland-J=C3=B8rgensen wrote:

quoted

Johannes Berg [off-list ref] writes:

quoted

quoted

-			       txqi->flags & (1<<IEEE80211_TXQ_STOP) ? "STOP" : "RUN",
-			       txqi->flags & (1<<IEEE80211_TXQ_AMPDU) ? " AMPDU" : "",
-			       txqi->flags & (1<<IEEE80211_TXQ_NO_AMSDU) ? " NO-AMSDU" :=20
"");
+			       txqi->flags & (1 << IEEE80211_TXQ_STOP) ? "STOP" : "RUN",
+			       txqi->flags & (1 << IEEE80211_TXQ_AMPDU) ? " AMPDU" : "",
+			       txqi->flags & (1 << IEEE80211_TXQ_NO_AMSDU) ? " NO-AMSDU"=20
: "");

=20
consider BIT() instead as a cleanup? :)
=20
(or maybe this is just a leftover from merging some other patches?)

=20
Yeah, this is a merging artifact; Rajkumar's patch added another flag,
that I removed again. Didn't notice that there was still a whitespace
change in this patch...
=20

I added the flag based on our last discussion. The driver needs to check
txq status for each tx_dequeue(). One time txq check is not sufficient
as it allows the driver to dequeue all frames from txq.

drv_func() {
       while (ieee80211_airtime_may_transmit(txq) &&
               hw_has_space() &&
              (pkt =3D ieee80211_tx_dequeue(hw, txq)))
           push_to_hw(pkt);
}

Yeah, but with airtime only being recorded on TX completion, the odds of
the value changing within that loop are quite low; so it's not going to
work, which is why I removed it.

However, after reading Kan's patches I get where you're coming from; a
check in tx_dequeue() is needed for the BQL-style queue limiting. Will
try to incorporate a version of that in the next series so you can see
what I mean when I say it should be orthogonal; and I'm still not sure
it needs a flag :)

quoted

quoted

quoted

+bool ieee80211_txq_may_transmit(struct ieee80211_hw *hw,
+				struct ieee80211_txq *txq)
+{
+	struct ieee80211_local *local =3D hw_to_local(hw);
+	struct txq_info *txqi =3D to_txq_info(txq);
+	bool may_tx =3D false;
+
+	spin_lock_bh(&local->active_txq_lock);
+
+	if (ieee80211_txq_check_deficit(local, txqi)) {
+		may_tx =3D true;
+		list_del_init(&txqi->schedule_order);

=20

To handle above case, may_transmit should remove the node only
when it is in list.

if (list_empty(&txqi->schedule_order))
        list_del_init(&txqi->schedule_order);

I assume you missed a ! in that if, right? :)

So that it can be used to determine whether txq is running negative.

But still not sure what you mean here?

-Toke