* [PATCH] ath10k: remove iteration in wake_tx_queue
@ 2019-03-27 16:29 ` Erik Stromdahl
0 siblings, 0 replies; 20+ messages in thread
From: Erik Stromdahl @ 2019-03-27 16:29 UTC (permalink / raw)
To: kvalo, linux-wireless, ath10k; +Cc: Erik Stromdahl, yiboz
Iterating the TX queue and thereby dequeuing all available packets in the
queue could result in performance penalties on some SMP systems.
The reason for this is most likely that the per-ac lock (active_txq_lock)
in mac80211 will be held by the CPU iterating the current queue.
This will lock up other CPUs trying to push new messages on the TX
queue.
Instead of iterating the queue we fetch just one packet at the time,
resulting in minimal starvation of the other CPUs.
Reported-by: Yibo Zhao <yiboz@codeaurora.org>
Signed-off-by: Erik Stromdahl <erik.stromdahl@gmail.com>
---
drivers/net/wireless/ath/ath10k/mac.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index b73c23d4ce86..c9e700b844f8 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -4356,7 +4356,6 @@ static void ath10k_mac_op_wake_tx_queue(struct ieee80211_hw *hw,
struct ieee80211_txq *txq)
{
struct ath10k *ar = hw->priv;
- int ret;
u8 ac;
ath10k_htt_tx_txq_update(hw, txq);
@@ -4369,11 +4368,9 @@ static void ath10k_mac_op_wake_tx_queue(struct ieee80211_hw *hw,
if (!txq)
goto out;
- while (ath10k_mac_tx_can_push(hw, txq)) {
- ret = ath10k_mac_tx_push_txq(hw, txq);
- if (ret < 0)
- break;
- }
+ if (ath10k_mac_tx_can_push(hw, txq))
+ ath10k_mac_tx_push_txq(hw, txq);
+
ieee80211_return_txq(hw, txq);
ath10k_htt_tx_txq_update(hw, txq);
out:
--
2.19.1
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply related [flat|nested] 20+ messages in thread* [PATCH] ath10k: remove iteration in wake_tx_queue
@ 2019-03-27 16:29 ` Erik Stromdahl
0 siblings, 0 replies; 20+ messages in thread
From: Erik Stromdahl @ 2019-03-27 16:29 UTC (permalink / raw)
To: kvalo, linux-wireless, ath10k; +Cc: yiboz, Erik Stromdahl
Iterating the TX queue and thereby dequeuing all available packets in the
queue could result in performance penalties on some SMP systems.
The reason for this is most likely that the per-ac lock (active_txq_lock)
in mac80211 will be held by the CPU iterating the current queue.
This will lock up other CPUs trying to push new messages on the TX
queue.
Instead of iterating the queue we fetch just one packet at the time,
resulting in minimal starvation of the other CPUs.
Reported-by: Yibo Zhao <yiboz@codeaurora.org>
Signed-off-by: Erik Stromdahl <erik.stromdahl@gmail.com>
---
drivers/net/wireless/ath/ath10k/mac.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index b73c23d4ce86..c9e700b844f8 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -4356,7 +4356,6 @@ static void ath10k_mac_op_wake_tx_queue(struct ieee80211_hw *hw,
struct ieee80211_txq *txq)
{
struct ath10k *ar = hw->priv;
- int ret;
u8 ac;
ath10k_htt_tx_txq_update(hw, txq);
@@ -4369,11 +4368,9 @@ static void ath10k_mac_op_wake_tx_queue(struct ieee80211_hw *hw,
if (!txq)
goto out;
- while (ath10k_mac_tx_can_push(hw, txq)) {
- ret = ath10k_mac_tx_push_txq(hw, txq);
- if (ret < 0)
- break;
- }
+ if (ath10k_mac_tx_can_push(hw, txq))
+ ath10k_mac_tx_push_txq(hw, txq);
+
ieee80211_return_txq(hw, txq);
ath10k_htt_tx_txq_update(hw, txq);
out:
--
2.19.1
^ permalink raw reply related [flat|nested] 20+ messages in thread* Re: [PATCH] ath10k: remove iteration in wake_tx_queue
2019-03-27 16:29 ` Erik Stromdahl
@ 2019-03-27 17:49 ` Peter Oh
-1 siblings, 0 replies; 20+ messages in thread
From: Peter Oh @ 2019-03-27 17:49 UTC (permalink / raw)
To: Erik Stromdahl, kvalo@qca.qualcomm.com,
linux-wireless@vger.kernel.org, ath10k@lists.infradead.org
Cc: yiboz@codeaurora.org
On 03/27/2019 09:29 AM, Erik Stromdahl wrote:
> Iterating the TX queue and thereby dequeuing all available packets in the
> queue could result in performance penalties on some SMP systems.
>
Please share the test results and numbers you've run to help others
thoughts.
Thanks,
Peter
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ath10k: remove iteration in wake_tx_queue
@ 2019-03-27 17:49 ` Peter Oh
0 siblings, 0 replies; 20+ messages in thread
From: Peter Oh @ 2019-03-27 17:49 UTC (permalink / raw)
To: Erik Stromdahl, kvalo@qca.qualcomm.com,
linux-wireless@vger.kernel.org, ath10k@lists.infradead.org
Cc: yiboz@codeaurora.org
On 03/27/2019 09:29 AM, Erik Stromdahl wrote:
> Iterating the TX queue and thereby dequeuing all available packets in the
> queue could result in performance penalties on some SMP systems.
>
Please share the test results and numbers you've run to help others
thoughts.
Thanks,
Peter
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ath10k: remove iteration in wake_tx_queue
2019-03-27 17:49 ` Peter Oh
@ 2019-03-29 7:47 ` Erik Stromdahl
-1 siblings, 0 replies; 20+ messages in thread
From: Erik Stromdahl @ 2019-03-29 7:47 UTC (permalink / raw)
To: Peter Oh, kvalo@qca.qualcomm.com, linux-wireless@vger.kernel.org,
ath10k@lists.infradead.org
Cc: yiboz@codeaurora.org
On 3/27/19 6:49 PM, Peter Oh wrote:
>
>
> On 03/27/2019 09:29 AM, Erik Stromdahl wrote:
>> Iterating the TX queue and thereby dequeuing all available packets in the
>> queue could result in performance penalties on some SMP systems.
>>
> Please share the test results and numbers you've run to help others
> thoughts.
>
I haven't run any tests with ath10k PCIe, but Yibo Zhao had noticed a 10%
degradation without this patch.
Yibo:
Can you provide iperf results etc. that shows the performance gain?
--
Erik
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ath10k: remove iteration in wake_tx_queue
@ 2019-03-29 7:47 ` Erik Stromdahl
0 siblings, 0 replies; 20+ messages in thread
From: Erik Stromdahl @ 2019-03-29 7:47 UTC (permalink / raw)
To: Peter Oh, kvalo@qca.qualcomm.com, linux-wireless@vger.kernel.org,
ath10k@lists.infradead.org
Cc: yiboz@codeaurora.org
On 3/27/19 6:49 PM, Peter Oh wrote:
>
>
> On 03/27/2019 09:29 AM, Erik Stromdahl wrote:
>> Iterating the TX queue and thereby dequeuing all available packets in the
>> queue could result in performance penalties on some SMP systems.
>>
> Please share the test results and numbers you've run to help others
> thoughts.
>
I haven't run any tests with ath10k PCIe, but Yibo Zhao had noticed a 10%
degradation without this patch.
Yibo:
Can you provide iperf results etc. that shows the performance gain?
--
Erik
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ath10k: remove iteration in wake_tx_queue
2019-03-29 7:47 ` Erik Stromdahl
@ 2019-04-01 12:17 ` Yibo Zhao
-1 siblings, 0 replies; 20+ messages in thread
From: Yibo Zhao @ 2019-04-01 12:17 UTC (permalink / raw)
To: Erik Stromdahl
Cc: linux-wireless-owner, kvalo, linux-wireless, ath10k, Peter Oh
On 2019-03-29 15:47, Erik Stromdahl wrote:
> On 3/27/19 6:49 PM, Peter Oh wrote:
>>
>>
>> On 03/27/2019 09:29 AM, Erik Stromdahl wrote:
>>> Iterating the TX queue and thereby dequeuing all available packets in
>>> the
>>> queue could result in performance penalties on some SMP systems.
>>>
>> Please share the test results and numbers you've run to help others
>> thoughts.
>>
>
> I haven't run any tests with ath10k PCIe, but Yibo Zhao had noticed a
> 10%
> degradation without this patch.
>
> Yibo:
> Can you provide iperf results etc. that shows the performance gain?
My tests are based on ixchariot with cabled setup(two-core AP system).
WDS mode--10% deviation:
With previous change: UDP DL-1246 Mbps
W/O previous change: UDP DL-987 Mbps
Normal mode:
With previous change: UDP DL-1380 Mbps
W/O previous change: UDP DL-1310 Mbps
Also attached the aqm status.
With previous change:
tid ac backlog-bytes backlog-packets new-flows drops marks overlimit
collisions tx-bytes tx-packets flags
0 2 0 0 5342229 0 0 0 0 3867657029 5342229 0x0(RUN)
1 3 0 0 0 0 0 0 0 0 0 0x0(RUN)
2 3 0 0 0 0 0 0 0 0 0 0x0(RUN)
3 2 0 0 0 0 0 0 0 0 0 0x0(RUN)
4 1 0 0 0 0 0 0 0 0 0 0x0(RUN)
5 1 0 0 0 0 0 0 0 0 0 0x0(RUN)
6 0 0 0 2 0 0 0 0 144 2 0x0(RUN)
7 0 0 0 2 0 0 0 0 282 2 0x0(RUN)
8 2 0 0 0 0 0 0 0 0 0 0x0(RUN)
9 3 0 0 0 0 0 0 0 0 0 0x0(RUN)
10 3 0 0 0 0 0 0 0 0 0 0x0(RUN)
11 2 0 0 0 0 0 0 0 0 0 0x0(RUN)
12 1 0 0 0 0 0 0 0 0 0 0x0(RUN)
13 1 0 0 0 0 0 0 0 0 0 0x0(RUN)
14 0 0 0 0 0 0 0 0 0 0 0x0(RUN)
15 0 0 0 0 0 0 0 0 0 0 0x0(RUN)
we see no difference between tx-packets and new-flows.
W/O previous change:
tid ac backlog-bytes backlog-packets new-flows drops marks overlimit
collisions tx-bytes tx-packets flags
0 2 0 0 2233059 3 0 9236 12 1159661783 6380867 0x0(RUN)
1 3 0 0 0 0 0 0 0 0 0 0x0(RUN)
2 3 0 0 0 0 0 0 0 0 0 0x0(RUN)
3 2 0 0 0 0 0 0 0 0 0 0x0(RUN)
4 1 0 0 0 0 0 0 0 0 0 0x0(RUN)
5 1 0 0 0 0 0 0 0 0 0 0x0(RUN)
6 0 0 0 1 0 0 0 0 144 2 0x0(RUN)
7 0 0 0 1 0 0 0 0 282 2 0x0(RUN)
8 2 0 0 0 0 0 0 0 0 0 0x0(RUN)
9 3 0 0 0 0 0 0 0 0 0 0x0(RUN)
10 3 0 0 0 0 0 0 0 0 0 0x0(RUN)
11 2 0 0 0 0 0 0 0 0 0 0x0(RUN)
12 1 0 0 0 0 0 0 0 0 0 0x0(RUN)
13 1 0 0 0 0 0 0 0 0 0 0x0(RUN)
14 0 0 0 0 0 0 0 0 0 0 0x0(RUN)
15 0 0 0 0 0 0 0 0 0 0 0x0(RUN)
new-flows are roughly one-third of the total tx-packets.
>
> --
> Erik
--
Yibo
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ath10k: remove iteration in wake_tx_queue
@ 2019-04-01 12:17 ` Yibo Zhao
0 siblings, 0 replies; 20+ messages in thread
From: Yibo Zhao @ 2019-04-01 12:17 UTC (permalink / raw)
To: Erik Stromdahl
Cc: Peter Oh, kvalo, linux-wireless, ath10k, linux-wireless-owner
On 2019-03-29 15:47, Erik Stromdahl wrote:
> On 3/27/19 6:49 PM, Peter Oh wrote:
>>
>>
>> On 03/27/2019 09:29 AM, Erik Stromdahl wrote:
>>> Iterating the TX queue and thereby dequeuing all available packets in
>>> the
>>> queue could result in performance penalties on some SMP systems.
>>>
>> Please share the test results and numbers you've run to help others
>> thoughts.
>>
>
> I haven't run any tests with ath10k PCIe, but Yibo Zhao had noticed a
> 10%
> degradation without this patch.
>
> Yibo:
> Can you provide iperf results etc. that shows the performance gain?
My tests are based on ixchariot with cabled setup(two-core AP system).
WDS mode--10% deviation:
With previous change: UDP DL-1246 Mbps
W/O previous change: UDP DL-987 Mbps
Normal mode:
With previous change: UDP DL-1380 Mbps
W/O previous change: UDP DL-1310 Mbps
Also attached the aqm status.
With previous change:
tid ac backlog-bytes backlog-packets new-flows drops marks overlimit
collisions tx-bytes tx-packets flags
0 2 0 0 5342229 0 0 0 0 3867657029 5342229 0x0(RUN)
1 3 0 0 0 0 0 0 0 0 0 0x0(RUN)
2 3 0 0 0 0 0 0 0 0 0 0x0(RUN)
3 2 0 0 0 0 0 0 0 0 0 0x0(RUN)
4 1 0 0 0 0 0 0 0 0 0 0x0(RUN)
5 1 0 0 0 0 0 0 0 0 0 0x0(RUN)
6 0 0 0 2 0 0 0 0 144 2 0x0(RUN)
7 0 0 0 2 0 0 0 0 282 2 0x0(RUN)
8 2 0 0 0 0 0 0 0 0 0 0x0(RUN)
9 3 0 0 0 0 0 0 0 0 0 0x0(RUN)
10 3 0 0 0 0 0 0 0 0 0 0x0(RUN)
11 2 0 0 0 0 0 0 0 0 0 0x0(RUN)
12 1 0 0 0 0 0 0 0 0 0 0x0(RUN)
13 1 0 0 0 0 0 0 0 0 0 0x0(RUN)
14 0 0 0 0 0 0 0 0 0 0 0x0(RUN)
15 0 0 0 0 0 0 0 0 0 0 0x0(RUN)
we see no difference between tx-packets and new-flows.
W/O previous change:
tid ac backlog-bytes backlog-packets new-flows drops marks overlimit
collisions tx-bytes tx-packets flags
0 2 0 0 2233059 3 0 9236 12 1159661783 6380867 0x0(RUN)
1 3 0 0 0 0 0 0 0 0 0 0x0(RUN)
2 3 0 0 0 0 0 0 0 0 0 0x0(RUN)
3 2 0 0 0 0 0 0 0 0 0 0x0(RUN)
4 1 0 0 0 0 0 0 0 0 0 0x0(RUN)
5 1 0 0 0 0 0 0 0 0 0 0x0(RUN)
6 0 0 0 1 0 0 0 0 144 2 0x0(RUN)
7 0 0 0 1 0 0 0 0 282 2 0x0(RUN)
8 2 0 0 0 0 0 0 0 0 0 0x0(RUN)
9 3 0 0 0 0 0 0 0 0 0 0x0(RUN)
10 3 0 0 0 0 0 0 0 0 0 0x0(RUN)
11 2 0 0 0 0 0 0 0 0 0 0x0(RUN)
12 1 0 0 0 0 0 0 0 0 0 0x0(RUN)
13 1 0 0 0 0 0 0 0 0 0 0x0(RUN)
14 0 0 0 0 0 0 0 0 0 0 0x0(RUN)
15 0 0 0 0 0 0 0 0 0 0 0x0(RUN)
new-flows are roughly one-third of the total tx-packets.
>
> --
> Erik
--
Yibo
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ath10k: remove iteration in wake_tx_queue
2019-03-27 16:29 ` Erik Stromdahl
@ 2019-04-01 11:05 ` Toke Høiland-Jørgensen
-1 siblings, 0 replies; 20+ messages in thread
From: Toke Høiland-Jørgensen @ 2019-04-01 11:05 UTC (permalink / raw)
To: Erik Stromdahl, kvalo, linux-wireless, ath10k; +Cc: yiboz
Erik Stromdahl <erik.stromdahl@gmail.com> writes:
> Iterating the TX queue and thereby dequeuing all available packets in the
> queue could result in performance penalties on some SMP systems.
>
> The reason for this is most likely that the per-ac lock (active_txq_lock)
> in mac80211 will be held by the CPU iterating the current queue.
>
> This will lock up other CPUs trying to push new messages on the TX
> queue.
>
> Instead of iterating the queue we fetch just one packet at the time,
> resulting in minimal starvation of the other CPUs.
Did you test this with Felix' patches reducing the time the lock is held
in mac80211?
-Toke
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ath10k: remove iteration in wake_tx_queue
@ 2019-04-01 11:05 ` Toke Høiland-Jørgensen
0 siblings, 0 replies; 20+ messages in thread
From: Toke Høiland-Jørgensen @ 2019-04-01 11:05 UTC (permalink / raw)
To: Erik Stromdahl, kvalo, linux-wireless, ath10k; +Cc: yiboz, Erik Stromdahl
Erik Stromdahl <erik.stromdahl@gmail.com> writes:
> Iterating the TX queue and thereby dequeuing all available packets in the
> queue could result in performance penalties on some SMP systems.
>
> The reason for this is most likely that the per-ac lock (active_txq_lock)
> in mac80211 will be held by the CPU iterating the current queue.
>
> This will lock up other CPUs trying to push new messages on the TX
> queue.
>
> Instead of iterating the queue we fetch just one packet at the time,
> resulting in minimal starvation of the other CPUs.
Did you test this with Felix' patches reducing the time the lock is held
in mac80211?
-Toke
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ath10k: remove iteration in wake_tx_queue
2019-04-01 11:05 ` Toke Høiland-Jørgensen
@ 2019-04-16 18:54 ` Erik Stromdahl
-1 siblings, 0 replies; 20+ messages in thread
From: Erik Stromdahl @ 2019-04-16 18:54 UTC (permalink / raw)
To: Toke Høiland-Jørgensen, kvalo, linux-wireless, ath10k; +Cc: yiboz
On 4/1/19 1:05 PM, Toke Høiland-Jørgensen wrote:
> Erik Stromdahl <erik.stromdahl@gmail.com> writes:
>
>> Iterating the TX queue and thereby dequeuing all available packets in the
>> queue could result in performance penalties on some SMP systems.
>>
>> The reason for this is most likely that the per-ac lock (active_txq_lock)
>> in mac80211 will be held by the CPU iterating the current queue.
>>
>> This will lock up other CPUs trying to push new messages on the TX
>> queue.
>>
>> Instead of iterating the queue we fetch just one packet at the time,
>> resulting in minimal starvation of the other CPUs.
>
> Did you test this with Felix' patches reducing the time the lock is held
> in mac80211?
>
> -Toke
>
Hi Toke,
I am not aware of these patches. Can you please point them out for me?
--
Erik
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ath10k: remove iteration in wake_tx_queue
@ 2019-04-16 18:54 ` Erik Stromdahl
0 siblings, 0 replies; 20+ messages in thread
From: Erik Stromdahl @ 2019-04-16 18:54 UTC (permalink / raw)
To: Toke Høiland-Jørgensen, kvalo, linux-wireless, ath10k; +Cc: yiboz
On 4/1/19 1:05 PM, Toke Høiland-Jørgensen wrote:
> Erik Stromdahl <erik.stromdahl@gmail.com> writes:
>
>> Iterating the TX queue and thereby dequeuing all available packets in the
>> queue could result in performance penalties on some SMP systems.
>>
>> The reason for this is most likely that the per-ac lock (active_txq_lock)
>> in mac80211 will be held by the CPU iterating the current queue.
>>
>> This will lock up other CPUs trying to push new messages on the TX
>> queue.
>>
>> Instead of iterating the queue we fetch just one packet at the time,
>> resulting in minimal starvation of the other CPUs.
>
> Did you test this with Felix' patches reducing the time the lock is held
> in mac80211?
>
> -Toke
>
Hi Toke,
I am not aware of these patches. Can you please point them out for me?
--
Erik
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ath10k: remove iteration in wake_tx_queue
2019-04-16 18:54 ` Erik Stromdahl
@ 2019-04-16 19:07 ` Toke Høiland-Jørgensen
-1 siblings, 0 replies; 20+ messages in thread
From: Toke Høiland-Jørgensen @ 2019-04-16 19:07 UTC (permalink / raw)
To: Erik Stromdahl, kvalo, linux-wireless, ath10k; +Cc: yiboz
Erik Stromdahl <erik.stromdahl@gmail.com> writes:
> On 4/1/19 1:05 PM, Toke Høiland-Jørgensen wrote:
>> Erik Stromdahl <erik.stromdahl@gmail.com> writes:
>>
>>> Iterating the TX queue and thereby dequeuing all available packets in the
>>> queue could result in performance penalties on some SMP systems.
>>>
>>> The reason for this is most likely that the per-ac lock (active_txq_lock)
>>> in mac80211 will be held by the CPU iterating the current queue.
>>>
>>> This will lock up other CPUs trying to push new messages on the TX
>>> queue.
>>>
>>> Instead of iterating the queue we fetch just one packet at the time,
>>> resulting in minimal starvation of the other CPUs.
>>
>> Did you test this with Felix' patches reducing the time the lock is held
>> in mac80211?
>>
>> -Toke
>>
> Hi Toke,
>
> I am not aware of these patches. Can you please point them out for me?
They've already been merged. Commits dcec1d9bc8a7 and 7ef769459f14 in
mac80211-next :)
-Toke
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ath10k: remove iteration in wake_tx_queue
@ 2019-04-16 19:07 ` Toke Høiland-Jørgensen
0 siblings, 0 replies; 20+ messages in thread
From: Toke Høiland-Jørgensen @ 2019-04-16 19:07 UTC (permalink / raw)
To: Erik Stromdahl, kvalo, linux-wireless, ath10k; +Cc: yiboz
Erik Stromdahl <erik.stromdahl@gmail.com> writes:
> On 4/1/19 1:05 PM, Toke Høiland-Jørgensen wrote:
>> Erik Stromdahl <erik.stromdahl@gmail.com> writes:
>>
>>> Iterating the TX queue and thereby dequeuing all available packets in the
>>> queue could result in performance penalties on some SMP systems.
>>>
>>> The reason for this is most likely that the per-ac lock (active_txq_lock)
>>> in mac80211 will be held by the CPU iterating the current queue.
>>>
>>> This will lock up other CPUs trying to push new messages on the TX
>>> queue.
>>>
>>> Instead of iterating the queue we fetch just one packet at the time,
>>> resulting in minimal starvation of the other CPUs.
>>
>> Did you test this with Felix' patches reducing the time the lock is held
>> in mac80211?
>>
>> -Toke
>>
> Hi Toke,
>
> I am not aware of these patches. Can you please point them out for me?
They've already been merged. Commits dcec1d9bc8a7 and 7ef769459f14 in
mac80211-next :)
-Toke
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ath10k: remove iteration in wake_tx_queue
2019-04-16 19:07 ` Toke Høiland-Jørgensen
@ 2019-04-17 13:29 ` Erik Stromdahl
-1 siblings, 0 replies; 20+ messages in thread
From: Erik Stromdahl @ 2019-04-17 13:29 UTC (permalink / raw)
To: Toke Høiland-Jørgensen, kvalo, linux-wireless, ath10k; +Cc: yiboz
On 4/16/19 9:07 PM, Toke Høiland-Jørgensen wrote:
> Erik Stromdahl <erik.stromdahl@gmail.com> writes:
>
>> On 4/1/19 1:05 PM, Toke Høiland-Jørgensen wrote:
>>> Erik Stromdahl <erik.stromdahl@gmail.com> writes:
>>>
>>>> Iterating the TX queue and thereby dequeuing all available packets in the
>>>> queue could result in performance penalties on some SMP systems.
>>>>
>>>> The reason for this is most likely that the per-ac lock (active_txq_lock)
>>>> in mac80211 will be held by the CPU iterating the current queue.
>>>>
>>>> This will lock up other CPUs trying to push new messages on the TX
>>>> queue.
>>>>
>>>> Instead of iterating the queue we fetch just one packet at the time,
>>>> resulting in minimal starvation of the other CPUs.
>>>
>>> Did you test this with Felix' patches reducing the time the lock is held
>>> in mac80211?
>>>
>>> -Toke
>>>
>> Hi Toke,
>>
>> I am not aware of these patches. Can you please point them out for me?
>
> They've already been merged. Commits dcec1d9bc8a7 and 7ef769459f14 in
> mac80211-next :)
>
> -Toke
>
I see. I am using the ath tree and I couldn't find them there.
I can cherry-pick them to my own tree and try them out
(or wait until Kalle updates ath.git).
--
Erik
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ath10k: remove iteration in wake_tx_queue
@ 2019-04-17 13:29 ` Erik Stromdahl
0 siblings, 0 replies; 20+ messages in thread
From: Erik Stromdahl @ 2019-04-17 13:29 UTC (permalink / raw)
To: Toke Høiland-Jørgensen, kvalo, linux-wireless, ath10k; +Cc: yiboz
On 4/16/19 9:07 PM, Toke Høiland-Jørgensen wrote:
> Erik Stromdahl <erik.stromdahl@gmail.com> writes:
>
>> On 4/1/19 1:05 PM, Toke Høiland-Jørgensen wrote:
>>> Erik Stromdahl <erik.stromdahl@gmail.com> writes:
>>>
>>>> Iterating the TX queue and thereby dequeuing all available packets in the
>>>> queue could result in performance penalties on some SMP systems.
>>>>
>>>> The reason for this is most likely that the per-ac lock (active_txq_lock)
>>>> in mac80211 will be held by the CPU iterating the current queue.
>>>>
>>>> This will lock up other CPUs trying to push new messages on the TX
>>>> queue.
>>>>
>>>> Instead of iterating the queue we fetch just one packet at the time,
>>>> resulting in minimal starvation of the other CPUs.
>>>
>>> Did you test this with Felix' patches reducing the time the lock is held
>>> in mac80211?
>>>
>>> -Toke
>>>
>> Hi Toke,
>>
>> I am not aware of these patches. Can you please point them out for me?
>
> They've already been merged. Commits dcec1d9bc8a7 and 7ef769459f14 in
> mac80211-next :)
>
> -Toke
>
I see. I am using the ath tree and I couldn't find them there.
I can cherry-pick them to my own tree and try them out
(or wait until Kalle updates ath.git).
--
Erik
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ath10k: remove iteration in wake_tx_queue
2019-04-17 13:29 ` Erik Stromdahl
@ 2019-04-26 7:07 ` Kalle Valo
-1 siblings, 0 replies; 20+ messages in thread
From: Kalle Valo @ 2019-04-26 7:07 UTC (permalink / raw)
To: Erik Stromdahl
Cc: yiboz, Toke Høiland-Jørgensen, linux-wireless, ath10k
Erik Stromdahl <erik.stromdahl@gmail.com> writes:
> On 4/16/19 9:07 PM, Toke Høiland-Jørgensen wrote:
>> Erik Stromdahl <erik.stromdahl@gmail.com> writes:
>>
>>> On 4/1/19 1:05 PM, Toke Høiland-Jørgensen wrote:
>>>> Erik Stromdahl <erik.stromdahl@gmail.com> writes:
>>>>
>>>>> Iterating the TX queue and thereby dequeuing all available packets in the
>>>>> queue could result in performance penalties on some SMP systems.
>>>>>
>>>>> The reason for this is most likely that the per-ac lock (active_txq_lock)
>>>>> in mac80211 will be held by the CPU iterating the current queue.
>>>>>
>>>>> This will lock up other CPUs trying to push new messages on the TX
>>>>> queue.
>>>>>
>>>>> Instead of iterating the queue we fetch just one packet at the time,
>>>>> resulting in minimal starvation of the other CPUs.
>>>>
>>>> Did you test this with Felix' patches reducing the time the lock is held
>>>> in mac80211?
>>>>
>>>> -Toke
>>>>
>>> Hi Toke,
>>>
>>> I am not aware of these patches. Can you please point them out for me?
>>
>> They've already been merged. Commits dcec1d9bc8a7 and 7ef769459f14 in
>> mac80211-next :)
>>
>> -Toke
>>
>
> I see. I am using the ath tree and I couldn't find them there.
> I can cherry-pick them to my own tree and try them out
> (or wait until Kalle updates ath.git).
It will take a while before these commits trickle down to ath-next
branch, most likely after v5.2-rc1 is released.
--
Kalle Valo
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ath10k: remove iteration in wake_tx_queue
@ 2019-04-26 7:07 ` Kalle Valo
0 siblings, 0 replies; 20+ messages in thread
From: Kalle Valo @ 2019-04-26 7:07 UTC (permalink / raw)
To: Erik Stromdahl
Cc: Toke Høiland-Jørgensen, linux-wireless, ath10k, yiboz
Erik Stromdahl <erik.stromdahl@gmail.com> writes:
> On 4/16/19 9:07 PM, Toke Høiland-Jørgensen wrote:
>> Erik Stromdahl <erik.stromdahl@gmail.com> writes:
>>
>>> On 4/1/19 1:05 PM, Toke Høiland-Jørgensen wrote:
>>>> Erik Stromdahl <erik.stromdahl@gmail.com> writes:
>>>>
>>>>> Iterating the TX queue and thereby dequeuing all available packets in the
>>>>> queue could result in performance penalties on some SMP systems.
>>>>>
>>>>> The reason for this is most likely that the per-ac lock (active_txq_lock)
>>>>> in mac80211 will be held by the CPU iterating the current queue.
>>>>>
>>>>> This will lock up other CPUs trying to push new messages on the TX
>>>>> queue.
>>>>>
>>>>> Instead of iterating the queue we fetch just one packet at the time,
>>>>> resulting in minimal starvation of the other CPUs.
>>>>
>>>> Did you test this with Felix' patches reducing the time the lock is held
>>>> in mac80211?
>>>>
>>>> -Toke
>>>>
>>> Hi Toke,
>>>
>>> I am not aware of these patches. Can you please point them out for me?
>>
>> They've already been merged. Commits dcec1d9bc8a7 and 7ef769459f14 in
>> mac80211-next :)
>>
>> -Toke
>>
>
> I see. I am using the ath tree and I couldn't find them there.
> I can cherry-pick them to my own tree and try them out
> (or wait until Kalle updates ath.git).
It will take a while before these commits trickle down to ath-next
branch, most likely after v5.2-rc1 is released.
--
Kalle Valo
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] ath10k: remove iteration in wake_tx_queue
2019-03-27 16:29 ` Erik Stromdahl
` (2 preceding siblings ...)
(?)
@ 2019-09-25 5:41 ` Kalle Valo
-1 siblings, 0 replies; 20+ messages in thread
From: Kalle Valo @ 2019-09-25 5:41 UTC (permalink / raw)
To: Erik Stromdahl; +Cc: kvalo, linux-wireless, yiboz, ath10k
Erik Stromdahl <erik.stromdahl@gmail.com> wrote:
> Iterating the TX queue and thereby dequeuing all available packets in the
> queue could result in performance penalties on some SMP systems.
>
> The reason for this is most likely that the per-ac lock (active_txq_lock)
> in mac80211 will be held by the CPU iterating the current queue.
>
> This will lock up other CPUs trying to push new messages on the TX
> queue.
>
> Instead of iterating the queue we fetch just one packet at the time,
> resulting in minimal starvation of the other CPUs.
>
> Reported-by: Yibo Zhao <yiboz@codeaurora.org>
> Signed-off-by: Erik Stromdahl <erik.stromdahl@gmail.com>
Like others, I'm not convinced about this either. To me it looks like a quick
workaround instead of properly investigating, and fixing, the root cause. To my
understanding the throughput dip was caused by this commit:
e3148cc5fecf ath10k: fix return value check in wake_tx_q op
So to me it feels like the issue has been there all along, just hidden, and the
fix above just exposed it.
--
https://patchwork.kernel.org/patch/10873753/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply [flat|nested] 20+ messages in thread* Re: [PATCH] ath10k: remove iteration in wake_tx_queue
2019-03-27 16:29 ` Erik Stromdahl
` (3 preceding siblings ...)
(?)
@ 2019-09-25 5:41 ` Kalle Valo
-1 siblings, 0 replies; 20+ messages in thread
From: Kalle Valo @ 2019-09-25 5:41 UTC (permalink / raw)
To: Erik Stromdahl; +Cc: kvalo, linux-wireless, ath10k, yiboz, Erik Stromdahl
Erik Stromdahl <erik.stromdahl@gmail.com> wrote:
> Iterating the TX queue and thereby dequeuing all available packets in the
> queue could result in performance penalties on some SMP systems.
>
> The reason for this is most likely that the per-ac lock (active_txq_lock)
> in mac80211 will be held by the CPU iterating the current queue.
>
> This will lock up other CPUs trying to push new messages on the TX
> queue.
>
> Instead of iterating the queue we fetch just one packet at the time,
> resulting in minimal starvation of the other CPUs.
>
> Reported-by: Yibo Zhao <yiboz@codeaurora.org>
> Signed-off-by: Erik Stromdahl <erik.stromdahl@gmail.com>
Like others, I'm not convinced about this either. To me it looks like a quick
workaround instead of properly investigating, and fixing, the root cause. To my
understanding the throughput dip was caused by this commit:
e3148cc5fecf ath10k: fix return value check in wake_tx_q op
So to me it feels like the issue has been there all along, just hidden, and the
fix above just exposed it.
--
https://patchwork.kernel.org/patch/10873753/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2019-09-25 5:41 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-03-27 16:29 [PATCH] ath10k: remove iteration in wake_tx_queue Erik Stromdahl
2019-03-27 16:29 ` Erik Stromdahl
2019-03-27 17:49 ` Peter Oh
2019-03-27 17:49 ` Peter Oh
2019-03-29 7:47 ` Erik Stromdahl
2019-03-29 7:47 ` Erik Stromdahl
2019-04-01 12:17 ` Yibo Zhao
2019-04-01 12:17 ` Yibo Zhao
2019-04-01 11:05 ` Toke Høiland-Jørgensen
2019-04-01 11:05 ` Toke Høiland-Jørgensen
2019-04-16 18:54 ` Erik Stromdahl
2019-04-16 18:54 ` Erik Stromdahl
2019-04-16 19:07 ` Toke Høiland-Jørgensen
2019-04-16 19:07 ` Toke Høiland-Jørgensen
2019-04-17 13:29 ` Erik Stromdahl
2019-04-17 13:29 ` Erik Stromdahl
2019-04-26 7:07 ` Kalle Valo
2019-04-26 7:07 ` Kalle Valo
2019-09-25 5:41 ` Kalle Valo
2019-09-25 5:41 ` Kalle Valo
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.