All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Toke Høiland-Jørgensen" <toke@toke.dk>
To: ath9k-devel@lists.ath9k.org
Subject: [ath9k-devel] [PATCH v4] ath9k: Switch to using mac80211 intermediate software	queues.
Date: Mon, 22 Aug 2016 17:13:21 -0000	[thread overview]
Message-ID: <871t1g4thz.fsf@toke.dk> (raw)
In-Reply-To: <87fupwvisn.fsf@kamboji.qca.qualcomm.com> (Kalle Valo's message of "Mon, 22 Aug 2016 20:02:16 +0300")

Kalle Valo <kvalo@codeaurora.org> writes:

> Toke H?iland-J?rgensen <toke@toke.dk> writes:
>
>> Kalle Valo <kvalo@codeaurora.org> writes:
>>
>>> Toke H?iland-J?rgensen <toke@toke.dk> writes:
>>>
>>>> This switches ath9k over to using the mac80211 intermediate software
>>>> queueing mechanism for data packets. It removes the queueing inside the
>>>> driver, except for the retry queue, and instead pulls from mac80211 when
>>>> a packet is needed. The retry queue is used to store a packet that was
>>>> pulled but can't be sent immediately.
>>>>
>>>> The old code path in ath_tx_start that would queue packets has been
>>>> removed completely, as has the qlen limit tunables (since there's no
>>>> longer a queue in the driver to limit).
>>>>
>>>> Based on Tim's original patch set, but reworked quite thoroughly.
>>>>
>>>> Cc: Tim Shepard <shep@alum.mit.edu>
>>>> Cc: Felix Fietkau <nbd@nbd.name>
>>>> Signed-off-by: Toke H?iland-J?rgensen <toke@toke.dk>
>>>> ---
>>>> Changes since v3 (most due to Felix; thanks!):
>>>>   - Correctly notify mac80211 when there are packets in the retry queue
>>>>     on powersave start/stop.
>>>>   - Get rid of ath_tx_aggr_resume().
>>>>   - Some readability changes and additional WARN_ON/BUG_ON in
>>>>     appropriate places.
>>>
>>> This is great work but due to the regressions I'm not sure if this
>>> will be ready for 4.9. To get more testing time I wonder if we should
>>> wait for 4.10? IMHO applying this in the end of the cycle is too risky
>>> and we should try to maximise the time linux-next by applying this
>>> just after -rc1 is released.
>>>
>>> Thoughts?
>>
>> Well, now that we understand what is causing the throughput regressions,
>> fixing them should be fairly straight forward (yeah, famous last words,
>> but still...). I already have a patch for the fast path and will go poke
>> at the slow path next. It'll probably require another workaround or two,
>> so I guess it won't be the architecturally clean ideal solution; but it
>> would make it possible to have something that works for 4.9 and then
>> iterate for a cleaner design for 4.10.
>
> But if we try to rush this to 4.9 it won't be in linux-next for long. We
> are now in -rc3 and let's say that the patches are ready to apply in two
> weeks. That would leave us only two weeks of -next time before the merge
> window, which I think is not enough for a controversial patch like this
> one. There might be other bugs lurking which haven't been found yet.

What, other hidden bugs? Unpossible! :)

Would it be possible to merge the partial solution (which is ready now,
basically) and fix the slow path in a separate patch later?

(Just spit-balling here; I'm still fairly new to this process. But I am
concerned that we'll hit a catch-22 where we can't get wider testing
before it's "ready" and we can't prove that it's "ready" until we've had
wider testing...)

-Toke

WARNING: multiple messages have this Message-ID (diff)
From: "Toke Høiland-Jørgensen" <toke@toke.dk>
To: Kalle Valo <kvalo@codeaurora.org>
Cc: linux-wireless@vger.kernel.org,
	make-wifi-fast@lists.bufferbloat.net,
	ath9k-devel@lists.ath9k.org, Tim Shepard <shep@alum.mit.edu>,
	Felix Fietkau <nbd@nbd.name>
Subject: Re: [PATCH v4] ath9k: Switch to using mac80211 intermediate software queues.
Date: Mon, 22 Aug 2016 19:13:12 +0200	[thread overview]
Message-ID: <871t1g4thz.fsf@toke.dk> (raw)
In-Reply-To: <87fupwvisn.fsf@kamboji.qca.qualcomm.com> (Kalle Valo's message of "Mon, 22 Aug 2016 20:02:16 +0300")

Kalle Valo <kvalo@codeaurora.org> writes:

> Toke H=C3=B8iland-J=C3=B8rgensen <toke@toke.dk> writes:
>
>> Kalle Valo <kvalo@codeaurora.org> writes:
>>
>>> Toke H=C3=B8iland-J=C3=B8rgensen <toke@toke.dk> writes:
>>>
>>>> This switches ath9k over to using the mac80211 intermediate software
>>>> queueing mechanism for data packets. It removes the queueing inside the
>>>> driver, except for the retry queue, and instead pulls from mac80211 wh=
en
>>>> a packet is needed. The retry queue is used to store a packet that was
>>>> pulled but can't be sent immediately.
>>>>
>>>> The old code path in ath_tx_start that would queue packets has been
>>>> removed completely, as has the qlen limit tunables (since there's no
>>>> longer a queue in the driver to limit).
>>>>
>>>> Based on Tim's original patch set, but reworked quite thoroughly.
>>>>
>>>> Cc: Tim Shepard <shep@alum.mit.edu>
>>>> Cc: Felix Fietkau <nbd@nbd.name>
>>>> Signed-off-by: Toke H=C3=B8iland-J=C3=B8rgensen <toke@toke.dk>
>>>> ---
>>>> Changes since v3 (most due to Felix; thanks!):
>>>>   - Correctly notify mac80211 when there are packets in the retry queue
>>>>     on powersave start/stop.
>>>>   - Get rid of ath_tx_aggr_resume().
>>>>   - Some readability changes and additional WARN_ON/BUG_ON in
>>>>     appropriate places.
>>>
>>> This is great work but due to the regressions I'm not sure if this
>>> will be ready for 4.9. To get more testing time I wonder if we should
>>> wait for 4.10? IMHO applying this in the end of the cycle is too risky
>>> and we should try to maximise the time linux-next by applying this
>>> just after -rc1 is released.
>>>
>>> Thoughts?
>>
>> Well, now that we understand what is causing the throughput regressions,
>> fixing them should be fairly straight forward (yeah, famous last words,
>> but still...). I already have a patch for the fast path and will go poke
>> at the slow path next. It'll probably require another workaround or two,
>> so I guess it won't be the architecturally clean ideal solution; but it
>> would make it possible to have something that works for 4.9 and then
>> iterate for a cleaner design for 4.10.
>
> But if we try to rush this to 4.9 it won't be in linux-next for long. We
> are now in -rc3 and let's say that the patches are ready to apply in two
> weeks. That would leave us only two weeks of -next time before the merge
> window, which I think is not enough for a controversial patch like this
> one. There might be other bugs lurking which haven't been found yet.

What, other hidden bugs? Unpossible! :)

Would it be possible to merge the partial solution (which is ready now,
basically) and fix the slow path in a separate patch later?

(Just spit-balling here; I'm still fairly new to this process. But I am
concerned that we'll hit a catch-22 where we can't get wider testing
before it's "ready" and we can't prove that it's "ready" until we've had
wider testing...)

-Toke

  reply	other threads:[~2016-08-22 17:13 UTC|newest]

Thread overview: 118+ 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 ` [ath9k-devel] " 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  9:09   ` [ath9k-devel] " Toke Høiland-Jørgensen
2016-06-17 13:28   ` Felix Fietkau
2016-06-17 13:28     ` Felix Fietkau
2016-06-17 13:41     ` Tim Shepard
2016-06-17 14:08       ` [ath9k-devel] " Tim Shepard
2016-06-17 14:35       ` Felix Fietkau
2016-06-17 14:35         ` Felix Fietkau
2016-06-17 17:45         ` Tim Shepard
2016-06-17 17:45           ` [ath9k-devel] " Tim Shepard
2016-06-17 19:15           ` Toke Høiland-Jørgensen
2016-06-17 19:16             ` [ath9k-devel] " Toke Høiland-Jørgensen
2016-06-17 13:43     ` Toke Høiland-Jørgensen
2016-06-17 13:43       ` [ath9k-devel] " Toke Høiland-Jørgensen
2016-06-17 13:48       ` Felix Fietkau
2016-06-17 13:48         ` Felix Fietkau
2016-06-17 16:33         ` [ath9k-devel] " Felix Fietkau
2016-06-17 16:33           ` Felix Fietkau
2016-06-17 14:10     ` [ath9k-devel] " Dave Taht
2016-06-17 14:10       ` Dave Taht
2016-06-18 19:05   ` [PATCH] ath9k: Switch to using " Toke Høiland-Jørgensen
2016-06-18 19:06     ` [ath9k-devel] " Toke Høiland-Jørgensen
2016-06-19  3:17     ` Tim Shepard
2016-06-19  3:17       ` [ath9k-devel] " Tim Shepard
2016-06-19  8:52       ` Toke Høiland-Jørgensen
2016-06-19  8:52         ` [ath9k-devel] " Toke Høiland-Jørgensen
2016-06-19 13:39         ` Tim Shepard
2016-06-19 13:40           ` [ath9k-devel] " Tim Shepard
2016-06-19 13:50           ` Toke Høiland-Jørgensen
2016-06-19 13:50             ` [ath9k-devel] " Toke Høiland-Jørgensen
2016-07-03  3:52     ` Tim Shepard
2016-07-03  3:53       ` [ath9k-devel] " Tim Shepard
2016-07-04 17:46       ` Toke Høiland-Jørgensen
2016-07-04 17:47         ` [ath9k-devel] " Toke Høiland-Jørgensen
2016-07-06 13:23         ` Felix Fietkau
2016-07-06 13:23           ` Felix Fietkau
2016-07-06 14:45           ` Toke Høiland-Jørgensen
2016-07-06 14:46             ` [ath9k-devel] " Toke Høiland-Jørgensen
2016-07-06 16:16     ` [PATCH v2] " Toke Høiland-Jørgensen
2016-07-06 16:17       ` [ath9k-devel] " Toke Høiland-Jørgensen
2016-07-06 17:57       ` Sebastian Gottschall
2016-07-06 18:19         ` [ath9k-devel] " Sebastian Gottschall
2016-07-06 18:13       ` Felix Fietkau
2016-07-06 18:13         ` Felix Fietkau
2016-07-06 18:52         ` Toke Høiland-Jørgensen
2016-07-06 18:52           ` [ath9k-devel] " Toke Høiland-Jørgensen
2016-07-06 18:59           ` Felix Fietkau
2016-07-06 18:59             ` Felix Fietkau
2016-07-06 19:08             ` Toke Høiland-Jørgensen
2016-07-06 19:08               ` [ath9k-devel] " Toke Høiland-Jørgensen
2016-07-06 19:34       ` [PATCH v3] " Toke Høiland-Jørgensen
2016-07-06 19:38         ` [ath9k-devel] " Toke Høiland-Jørgensen
2016-07-08 14:26         ` [ath9k-devel] [v3] " Kalle Valo
2016-07-08 14:26           ` Kalle Valo
2016-07-08 15:53           ` Toke Høiland-Jørgensen
2016-07-08 15:53             ` [ath9k-devel] " Toke Høiland-Jørgensen
2016-07-08 16:10             ` Felix Fietkau
2016-07-08 16:10               ` Felix Fietkau
2016-07-08 16:28               ` Toke Høiland-Jørgensen
2016-07-08 16:28                 ` [ath9k-devel] " Toke Høiland-Jørgensen
2016-07-08 16:31                 ` Felix Fietkau
2016-07-08 16:31                   ` Felix Fietkau
2016-07-08 16:38                   ` Toke Høiland-Jørgensen
2016-07-08 16:38                     ` [ath9k-devel] " Toke Høiland-Jørgensen
2016-07-08 18:24                   ` Sebastian Gottschall
2016-07-08 18:24                     ` [ath9k-devel] " Sebastian Gottschall
2016-07-09 12:00                     ` Toke Høiland-Jørgensen
2016-07-09 12:00                       ` [ath9k-devel] " Toke Høiland-Jørgensen
2016-07-08 16:38         ` [PATCH v3] " Tim Shepard
2016-07-08 16:38           ` [ath9k-devel] " Tim Shepard
2016-07-09 15:44           ` Toke Høiland-Jørgensen
2016-07-09 15:45             ` [ath9k-devel] " Toke Høiland-Jørgensen
2016-08-05 16:03         ` [PATCH v4] " Toke Høiland-Jørgensen
2016-08-05 16:05           ` [ath9k-devel] " Toke Høiland-Jørgensen
2016-08-22 15:43           ` Kalle Valo
2016-08-22 15:43             ` Kalle Valo
2016-08-22 16:16             ` Toke Høiland-Jørgensen
2016-08-22 16:16               ` [ath9k-devel] " Toke Høiland-Jørgensen
2016-08-22 17:02               ` Kalle Valo
2016-08-22 17:02                 ` Kalle Valo
2016-08-22 17:13                 ` Toke Høiland-Jørgensen [this message]
2016-08-22 17:13                   ` [ath9k-devel] " Toke Høiland-Jørgensen
2016-08-23  6:59                   ` Kalle Valo
2016-08-23  6:59                     ` Kalle Valo
2016-08-23  8:52                     ` Arend van Spriel
2016-08-23  8:58                       ` [ath9k-devel] " Arend van Spriel
2016-10-05 14:02                     ` Toke Høiland-Jørgensen
2016-10-05 14:09                       ` [ath9k-devel] " Toke Høiland-Jørgensen
2016-10-05 15:50                       ` Kalle Valo
2016-10-05 15:50                         ` Kalle Valo
2016-10-05 16:55                         ` Toke Høiland-Jørgensen
2016-10-05 16:55                           ` [ath9k-devel] " Toke Høiland-Jørgensen
2016-10-05 17:54                           ` Kalle Valo
2016-10-05 17:54                             ` Kalle Valo
2016-10-05 19:56                             ` Toke Høiland-Jørgensen
2016-10-05 19:56                               ` [ath9k-devel] " 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-06-17  9:09   ` [ath9k-devel] " 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=871t1g4thz.fsf@toke.dk \
    --to=toke@toke.dk \
    --cc=ath9k-devel@lists.ath9k.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.