From:   Toke =?utf-8?Q?H=C3=B8iland-J=C3=B8rgensen?= <toke@redhat.com>
To:     Felix Fietkau <nbd@nbd.name>, linux-wireless@vger.kernel.org
Subject: Re: [PATCH 1/6] mt76: use mac80211 txq scheduling
In-Reply-To: <7b9a0a24-14b0-9b96-0010-ce9f96308ad0@nbd.name>
References: <20190316204242.73560-1-nbd@nbd.name> <87muluv4hd.fsf@toke.dk> <7b9a0a24-14b0-9b96-0010-ce9f96308ad0@nbd.name>
Date:   Sun, 17 Mar 2019 12:32:55 +0100
Message-ID: <875zshviqg.fsf@toke.dk>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8BIT
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

Felix Fietkau <nbd@nbd.name> writes:

> On 2019-03-16 23:28, Toke Høiland-Jørgensen wrote:
>> Felix Fietkau <nbd@nbd.name> writes:
>> 
>>> Performance improvement and preparation for adding airtime fairness
>>> support
>> 
>> Great to see this! Do you have a plan for the airtime fairness part?
>> I.e., how to get the airtime information?
> Not yet. Still need to investigate what kind of information the hardware
> can provide. On a first glance it seems rather limited, so we may have
> to approximate based on tx status rates/retry and average packet size.

OK, cool. A byte-based estimator can also be useful for preventing dumb
firmware from buffering too much. The Chromium guys did that for ath10k:

https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/588190/13/drivers/net/wireless-4.2/ath/ath10k/mac.c#3826

>> The call to ieee80211_return_txq() is really meant to be unconditional.
>> The TXQ will only actually be scheduled if it still has packets queued.
>> I know it's slightly more expensive to have the check in mac80211, but
>> this is what makes it possible to change the implementation without
>> touching the drivers (such as in the RFC patch I sent earlier that
>> switches the scheduling algorithm)...
> I think this API needs to be extended to allow the driver to specify
> that it has buffered packets for a txq. Otherwise there's a small window
> where the driver has packets for a txq but mac80211 doesn't, and
> mac80211 won't schedule the queue in that case.
> I'll send a patch for this soon.

Right, makes sense. As long as mac80211 is in control over how it will
react to that information (thus allowing to e.g., invert the logic if
needed), I have no objections to extending the API... :)

-Toke