On Tue, Apr 28, 2020 at 7:56 AM Steve deRosier <derosier@xxxxxxxxx> 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 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!
>
> > 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
> >
--
Make Music, Not War
Dave Täht
CTO, TekLibre, LLC
http://www.teklibre.com
Tel: 1-831-435-0729
