Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH] ath10k: Restart xmit queues below low-water mark.
To:     Dave Taht <dave.taht@gmail.com>,
        Steve deRosier <derosier@gmail.com>
References: <20200427145435.13151-1-greearb@candelatech.com>
 <CALLGbR+fY9w1q=6HuU56OZLD6BeP_0KkU2xeoAA0ZZXxns+i3g@mail.gmail.com>
 <CAA93jw5tcptB64B8nGH0hqr4xC9G2SXNjp2tPf3YK5R7t6ZjXw@mail.gmail.com>
Cc:     linux-wireless <linux-wireless@vger.kernel.org>
From:   Ben Greear <greearb@candelatech.com>
Message-ID: <ae5587d1-f910-5fbe-42ab-3c4d722b259b@candelatech.com>
Date:   Tue, 28 Apr 2020 09:35:57 -0700
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101
 Thunderbird/45.8.0
MIME-Version: 1.0
In-Reply-To: <CAA93jw5tcptB64B8nGH0hqr4xC9G2SXNjp2tPf3YK5R7t6ZjXw@mail.gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk


On 04/28/2020 09:27 AM, Dave Taht wrote:
> On Tue, Apr 28, 2020 at 7:56 AM Steve deRosier <derosier@gmail.com> wrote:
>>
>> On Mon, Apr 27, 2020 at 7:54 AM <greearb@candelatech.com> wrote:
>>>
>>> From: Ben Greear <greearb@candelatech.com>
>>>
>>> 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 note that I'd prefer a BQL-like high/low watermark approach in
> general... bytes, not packets, or better, perceived
> airtime in a revision of AQL...
>
> ... but we'll try this patch, thx!

Is there a nice diagram somewhere that shows where the various
buffer-bloat technologies sit in the stack?  I suspect such should
be above the txqueue start/stop logic in the driver that I mucked
with, and certainly the old behaviour has no obvious tie-in with
any higher-level algorithms.

I doubt my patch will change much except in pathological cases where
the system is transmitting frames fast enough to fully fill the tx buffers
and also loaded in such a way that it can process just a few tx frames
at a time to keep bouncing to full and not full over and over.

Thanks,
Ben

-- 
Ben Greear <greearb@candelatech.com>
Candela Technologies Inc  http://www.candelatech.com