Ben Greear <greearb@xxxxxxxxxxxxxxx> writes:
> On 04/28/2020 07:56 AM, Steve deRosier wrote:
>> On Mon, Apr 27, 2020 at 7:54 AM <greearb@xxxxxxxxxxxxxxx> wrote:
>>>
>>> From: Ben Greear <greearb@xxxxxxxxxxxxxxx>
>>>
>>> While running tcp upload + download tests with ~200
>>> concurrent TCP streams, 1-2 processes, and 30 station
>>> vdevs, I noticed that the __ieee80211_stop_queue was taking
>>> around 20% of the CPU according to perf-top, which other locking
>>> taking an additional ~15%.
>>>
>>> I believe the issue is that the ath10k driver would unlock the
>>> txqueue when a single frame could be transmitted, instead of
>>> waiting for a low water mark.
>>>
>>> So, this patch adds a low-water mark that is 1/4 of the total
>>> tx buffers allowed.
>>>
>>> This appears to resolve the performance problem that I saw.
>>>
>>> Tested with recent wave-1 ath10k-ct firmware.
>>>
>>
>> Hey Ben,
>>
>> Did you do any testing with this patch around latency? The nature of
>> the thing that you fixed makes me wonder if it was intentional with
>> respect to making WiFi fast - ie getting rid of buffers as much as
>> possible. Obviously the CPU impact is likely to be an unintended
>> consequence. In any case, I don't know anything for sure, it was just
>> a thought that went through my head when reading this.
>
> I did not, but on average my patch should make the queues be less full,
> so I doubt it will hurt latency.
I would tend to agree with that.
-Toke
>>> Signed-off-by: Ben Greear <greearb@xxxxxxxxxxxxxxx>
>>> ---
>>> drivers/net/wireless/ath/ath10k/htt.h | 1 +
>>> drivers/net/wireless/ath/ath10k/htt_tx.c | 8 ++++++--
>>> 2 files changed, 7 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath10k/htt.h b/drivers/net/wireless/ath/ath10k/htt.h
>>> index 31c4ddbf45cb..b5634781c0dc 100644
>>> --- a/drivers/net/wireless/ath/ath10k/htt.h
>>> +++ b/drivers/net/wireless/ath/ath10k/htt.h
>>> @@ -1941,6 +1941,7 @@ struct ath10k_htt {
>>>
>>> u8 target_version_major;
>>> u8 target_version_minor;
>>> + bool needs_unlock;
>>> struct completion target_version_received;
>>> u8 max_num_amsdu;
>>> u8 max_num_ampdu;
>>> diff --git a/drivers/net/wireless/ath/ath10k/htt_tx.c b/drivers/net/wireless/ath/ath10k/htt_tx.c
>>> index 9b3c3b080e92..44795d9a7c0c 100644
>>> --- a/drivers/net/wireless/ath/ath10k/htt_tx.c
>>> +++ b/drivers/net/wireless/ath/ath10k/htt_tx.c
>>> @@ -145,8 +145,10 @@ void ath10k_htt_tx_dec_pending(struct ath10k_htt *htt)
>>> lockdep_assert_held(&htt->tx_lock);
>>>
>>> htt->num_pending_tx--;
>>> - if (htt->num_pending_tx == htt->max_num_pending_tx - 1)
>>> + if ((htt->num_pending_tx <= (htt->max_num_pending_tx / 4)) && htt->needs_unlock) {
>>> + htt->needs_unlock = false;
>>> ath10k_mac_tx_unlock(htt->ar, ATH10K_TX_PAUSE_Q_FULL);
>>> + }
>>> }
>>>
>>> int ath10k_htt_tx_inc_pending(struct ath10k_htt *htt)
>>> @@ -157,8 +159,10 @@ int ath10k_htt_tx_inc_pending(struct ath10k_htt *htt)
>>> return -EBUSY;
>>>
>>> htt->num_pending_tx++;
>>> - if (htt->num_pending_tx == htt->max_num_pending_tx)
>>> + if (htt->num_pending_tx == htt->max_num_pending_tx) {
>>> + htt->needs_unlock = true;
>>> ath10k_mac_tx_lock(htt->ar, ATH10K_TX_PAUSE_Q_FULL);
>>> + }
>>>
>>> return 0;
>>> }
>>> --
>>> 2.20.1
>>>
>>
>
> --
> Ben Greear <greearb@xxxxxxxxxxxxxxx>
> Candela Technologies Inc http://www.candelatech.com
