From: Tim Shepard <shep@alum.mit.edu>
To: "Toke Høiland-Jørgensen" <toke@toke.dk>
Cc: linux-wireless@vger.kernel.org,
make-wifi-fast@lists.bufferbloat.net,
ath9k-devel@lists.ath9k.org, Felix Fietkau <nbd@nbd.name>
Subject: Re: [PATCH] ath9k: Switch to using mac80211 intermediate software queues.
Date: Sun, 19 Jun 2016 09:39:12 -0400 [thread overview]
Message-ID: <E1bEcwS-0006jR-00@www.xplot.org> (raw)
In-Reply-To: Your message of Sun, 19 Jun 2016 10:52:16 +0200. <87k2hlsgqn.fsf@toke.dk>
>
> You're right that it doesn't check the max. However, this is less of a
> problem now that there is no intermediate queueing in the driver; and
> indeed the utility of haven the qlen_* tunables is somewhat questionable
> with the patch applied: The only thing this is going to control is the
> size of the retry queue, and possible limit the size of the retry queue.
> [....]
The driver queues things up for the hardware to DMA and transmit.
Something has to limit the amount of packets handed over to the
hardware. (We lack access to hardware documentation (grrrr!) but it
appears to me that the hardware has a hard limit on how many packets
can be handed to it.)
> > The second more mysterious bug which I'm still struggling to
> > understand is why doesn't large values in these ath9k/qlen_* (or more
> > accurately, given the first bug above, the failure to check these qlen
> > limit values at all) allow for increased hardware queue bloat (with
> > observable delay).
>
> Because there's a second limit in play (which has always been there): in
> ath_tx_sched_aggr() there is this check:
>
> if ((aggr && txq->axq_ampdu_depth >= ATH_AGGR_MIN_QDEPTH) ||
> (!aggr && txq->axq_depth >= ATH_NON_AGGR_MIN_QDEPTH)) {
> __skb_queue_tail(&tid->retry_q, bf->bf_mpdu);
> *stop = true;
> return false;
> }
>
> The two constants are 2 and 8 respectively. This means that, with
> aggregation enabled, no more than two full aggregates will be queued up.
> The size of the aggregates is dynamically computed from the current
> rate: they are limited a maximum of four milliseconds of (estimated)
> airtime (for the BE queue; the others have different limits).
>
> So in a sense there's already a dynamic limit on the hardware queues.
> Now, whether four milliseconds is the right maximum aggregate size might
> be worth discussing. It is the maximum allowed by the standard. Dave and
> I have been
Ah that may be the clue that I lacked. There's got to be a dependency
on processor speed (how quickly the system and driver can get another
packet hooked up for transmission after completions) but perhaps with
aggregates being so large in time, with full aggregates even the
slowest processors are fast enough to avoid starvation.
If there's no aggregation, a limit of some sort is needed (probably to
prevent malfunction of the hardware/driver, but in any case to limit
excess latency). And this limit will depend on processor speed (and
will need to be autotuned at runtime).
-Tim Shepard
shep@alum.mit.edu
next prev parent reply other threads:[~2016-06-19 13:40 UTC|newest]
Thread overview: 68+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-06-17 9:09 [PATCH 0/2] ath9k: Add airtime fairness scheduler Toke Høiland-Jørgensen
2016-06-17 9:09 ` [PATCH 1/2] ath9k: use mac80211 intermediate software queues Toke Høiland-Jørgensen
2016-06-17 13:28 ` Felix Fietkau
2016-06-17 13:41 ` Tim Shepard
2016-06-17 14:35 ` Felix Fietkau
2016-06-17 17:45 ` Tim Shepard
2016-06-17 19:15 ` Toke Høiland-Jørgensen
2016-06-17 13:43 ` Toke Høiland-Jørgensen
2016-06-17 13:48 ` Felix Fietkau
2016-06-17 16:33 ` Felix Fietkau
2016-06-17 14:10 ` [ath9k-devel] " Dave Taht
2016-06-18 19:05 ` [PATCH] ath9k: Switch to using " Toke Høiland-Jørgensen
2016-06-19 3:17 ` Tim Shepard
2016-06-19 8:52 ` Toke Høiland-Jørgensen
2016-06-19 13:39 ` Tim Shepard [this message]
2016-06-19 13:50 ` Toke Høiland-Jørgensen
2016-07-03 3:52 ` Tim Shepard
2016-07-04 17:46 ` Toke Høiland-Jørgensen
2016-07-06 13:23 ` Felix Fietkau
2016-07-06 14:45 ` Toke Høiland-Jørgensen
2016-07-06 16:16 ` [PATCH v2] " Toke Høiland-Jørgensen
2016-07-06 17:57 ` Sebastian Gottschall
2016-07-06 18:13 ` Felix Fietkau
2016-07-06 18:52 ` Toke Høiland-Jørgensen
2016-07-06 18:59 ` Felix Fietkau
2016-07-06 19:08 ` Toke Høiland-Jørgensen
2016-07-06 19:34 ` [PATCH v3] " Toke Høiland-Jørgensen
2016-07-08 14:26 ` [v3] " Kalle Valo
2016-07-08 15:53 ` Toke Høiland-Jørgensen
2016-07-08 16:10 ` Felix Fietkau
2016-07-08 16:28 ` Toke Høiland-Jørgensen
2016-07-08 16:31 ` Felix Fietkau
2016-07-08 16:38 ` Toke Høiland-Jørgensen
2016-07-08 18:24 ` Sebastian Gottschall
2016-07-09 12:00 ` Toke Høiland-Jørgensen
2016-07-08 16:38 ` [PATCH v3] " Tim Shepard
2016-07-09 15:44 ` Toke Høiland-Jørgensen
2016-08-05 16:03 ` [PATCH v4] " Toke Høiland-Jørgensen
2016-08-22 15:43 ` Kalle Valo
2016-08-22 16:16 ` Toke Høiland-Jørgensen
2016-08-22 17:02 ` Kalle Valo
2016-08-22 17:13 ` Toke Høiland-Jørgensen
2016-08-23 6:59 ` Kalle Valo
2016-08-23 8:52 ` Arend van Spriel
2016-10-05 14:02 ` Toke Høiland-Jørgensen
2016-10-05 15:50 ` Kalle Valo
2016-10-05 16:55 ` Toke Høiland-Jørgensen
2016-10-05 17:54 ` Kalle Valo
2016-10-05 19:56 ` Toke Høiland-Jørgensen
2016-09-02 14:00 ` [PATCH v5] " Toke Høiland-Jørgensen
2016-09-03 10:16 ` Felix Fietkau
2016-10-07 11:43 ` [v5] " Kalle Valo
2016-11-09 2:22 ` Kalle Valo
2016-11-09 2:44 ` Tim Shepard
2016-11-09 10:42 ` Toke Høiland-Jørgensen
2016-11-09 20:07 ` Valo, Kalle
2016-11-09 11:31 ` [PATCH v6] " Toke Høiland-Jørgensen
2016-11-09 22:42 ` [v6] " Kalle Valo
2016-11-09 23:10 ` Toke Høiland-Jørgensen
2016-11-15 15:00 ` Kalle Valo
2016-06-17 9:09 ` [PATCH 2/2] ath9k: Add a per-station airtime deficit scheduler Toke Høiland-Jørgensen
2016-11-24 13:54 ` [PATCH v2] ath9k: Introduce airtime fairness scheduling between stations Toke Høiland-Jørgensen
2016-11-25 15:16 ` Valo, Kalle
2016-11-27 15:58 ` Toke Høiland-Jørgensen
2016-11-28 9:34 ` Felix Fietkau
2016-11-28 10:00 ` Toke Høiland-Jørgensen
2016-11-28 10:12 ` [PATCH v3] " Toke Høiland-Jørgensen
2016-12-15 8:43 ` [v3] " Kalle Valo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=E1bEcwS-0006jR-00@www.xplot.org \
--to=shep@alum.mit.edu \
--cc=ath9k-devel@lists.ath9k.org \
--cc=linux-wireless@vger.kernel.org \
--cc=make-wifi-fast@lists.bufferbloat.net \
--cc=nbd@nbd.name \
--cc=toke@toke.dk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).