Felix Fietkau <nbd@xxxxxxxx> writes:
> On 2016-11-27 16:58, Toke Høiland-Jørgensen wrote:
>> "Valo, Kalle" <kvalo@xxxxxxxxxxxxxxxx> writes:
>>
>>> (The make-wifi-fast list is annoying as it always spams me when it's on
>>> CC, so dropped it.)
>>>
>>> Toke Høiland-Jørgensen <toke@xxxxxxx> writes:
>>>
>>>> This reworks the ath9k driver to schedule transmissions to connected
>>>> stations in a way that enforces airtime fairness between them. It
>>>> accomplishes this by measuring the time spent transmitting to or
>>>> receiving from a station at TX and RX completion, and accounting this to
>>>> a per-station, per-QoS level airtime deficit. Then, an FQ-CoDel based
>>>> deficit scheduler is employed at packet dequeue time, to control which
>>>> station gets the next transmission opportunity.
>>>>
>>>> Airtime fairness can significantly improve the efficiency of the network
>>>> when station rates vary. The following throughput values are from a
>>>> simple three-station test scenario, where two stations operate at the
>>>> highest HT20 rate, and one station at the lowest, and the scheduler is
>>>> employed at the access point:
>>>>
>>>> Before / After
>>>> Fast station 1: 19.17 / 25.09 Mbps
>>>> Fast station 2: 19.83 / 25.21 Mbps
>>>> Slow station: 2.58 / 1.77 Mbps
>>>> Total: 41.58 / 52.07 Mbps
>>>>
>>>> The benefit of airtime fairness goes up the more stations are present.
>>>> In a 30-station test with one station artificially limited to 1 Mbps,
>>>> we have seen aggregate throughput go from 2.14 to 17.76 Mbps.
>>>>
>>>> Signed-off-by: Toke Høiland-Jørgensen <toke@xxxxxxx>
>>>
>>> [...]
>>>
>>>> +void ath_acq_lock(struct ath_softc *sc, struct ath_acq *acq)
>>>> + __acquires(&acq->lock)
>>>> +{
>>>> + spin_lock_bh(&acq->lock);
>>>> +}
>>>> +
>>>> +void ath_acq_unlock(struct ath_softc *sc, struct ath_acq *acq)
>>>> + __releases(&acq->lock)
>>>> +{
>>>> + spin_unlock_bh(&acq->lock);
>>>> +}
>>>
>>> Why these? To me it looks like they just add an extra function jump and
>>> unneccessary extra layer.
>>
>> Well, there's already similar functions for the txq lock (ath_txq_lock()
>> and ath_txq_unlock() in xmit.c), so figured I'd be consistent with
>> those. And also that the __acquires and __releases macros were probably
>> useful.
>>
>> Also, won't the compiler automatically inline them?
> Not necessarily, these functions are not static. I think it would be a
> good idea to turn the ath_txq_lock/unlock functions into static inline
> functions as well.
Right, I'll re-send with these functions fixed, and send a separate
patch to fix ath_txq_lock*
> Please don't blindly repeat patterns that are already there, some of
> them might just not make any sense at all ;)
But that would imply that kernel developers are not infallible. Surely
that can't be right? ;)
-Toke
