From: Petter Mabacker <[email protected]>
The rtw8822cu driver have problem to handle high rx or tx rates compared
with high load (such as high I/O) on slower systems, such as for example
i.MX6 SoloX and similar platforms.
The problems are more frequent when having the access point close to the
device. On slower systems it's often enough to download a large file,
combined with generating I/O load to trigger:
[ 374.763424] rtw_8822cu 1-1.2:1.2: failed to get tx report from firmware
[ 377.771790] rtw_8822cu 1-1.2:1.2: failed to send h2c command
[ 407.813460] rtw_8822cu 1-1.2:1.2: firmware failed to report density after scan
[ 414.965826] rtw_8822cu 1-1.2:1.2: failed to send h2c command
[ 444.993462] rtw_8822cu 1-1.2:1.2: firmware failed to report density after scan
[ 452.144551] rtw_8822cu 1-1.2:1.2: failed to send h2c command
[ 482.183445] rtw_8822cu 1-1.2:1.2: firmware failed to report density after scan
[ 489.426263] rtw_8822cu 1-1.2:1.2: failed to send h2c command
Another way is to simply perform a wifi rescan.
Benchmarking shows that setting a high prio workqueue for tx/rx will
significally improve things. Also compared alloc_workqueue with
alloc_ordered_workqueue, but even thou the later seems to slightly
improve things it's still quite easy to reproduce the above issues. So
that leads to the decision to go for alloc_workqueue.
Thanks to Ping-Ke Shih <[email protected]> that came up with the idea
of exploring tweaking of the work queue's within a similar discussion.
Fixes: a82dfd33d1237 ("wifi: rtw88: Add common USB chip support")
Signed-off-by: Petter Mabacker <[email protected]>
---
drivers/net/wireless/realtek/rtw88/usb.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtw88/usb.c b/drivers/net/wireless/realtek/rtw88/usb.c
index 44a5fafb9905..bfe0845528ec 100644
--- a/drivers/net/wireless/realtek/rtw88/usb.c
+++ b/drivers/net/wireless/realtek/rtw88/usb.c
@@ -716,7 +716,7 @@ static int rtw_usb_init_rx(struct rtw_dev *rtwdev)
struct rtw_usb *rtwusb = rtw_get_usb_priv(rtwdev);
int i;
- rtwusb->rxwq = create_singlethread_workqueue("rtw88_usb: rx wq");
+ rtwusb->rxwq = alloc_workqueue("rtw88_usb: rx wq", WQ_UNBOUND | WQ_HIGHPRI, 0);
if (!rtwusb->rxwq) {
rtw_err(rtwdev, "failed to create RX work queue\n");
return -ENOMEM;
@@ -750,7 +750,7 @@ static int rtw_usb_init_tx(struct rtw_dev *rtwdev)
struct rtw_usb *rtwusb = rtw_get_usb_priv(rtwdev);
int i;
- rtwusb->txwq = create_singlethread_workqueue("rtw88_usb: tx wq");
+ rtwusb->txwq = alloc_workqueue("rtw88_usb: tx wq", WQ_UNBOUND | WQ_HIGHPRI, 0);
if (!rtwusb->txwq) {
rtw_err(rtwdev, "failed to create TX work queue\n");
return -ENOMEM;
--
2.39.2
[email protected] writes:
> From: Petter Mabacker <[email protected]>
>
> The rtw8822cu driver have problem to handle high rx or tx rates compared
> with high load (such as high I/O) on slower systems, such as for example
> i.MX6 SoloX and similar platforms.
>
> The problems are more frequent when having the access point close to the
> device. On slower systems it's often enough to download a large file,
> combined with generating I/O load to trigger:
>
> [ 374.763424] rtw_8822cu 1-1.2:1.2: failed to get tx report from firmware
> [ 377.771790] rtw_8822cu 1-1.2:1.2: failed to send h2c command
> [ 407.813460] rtw_8822cu 1-1.2:1.2: firmware failed to report density after scan
> [ 414.965826] rtw_8822cu 1-1.2:1.2: failed to send h2c command
> [ 444.993462] rtw_8822cu 1-1.2:1.2: firmware failed to report density after scan
> [ 452.144551] rtw_8822cu 1-1.2:1.2: failed to send h2c command
> [ 482.183445] rtw_8822cu 1-1.2:1.2: firmware failed to report density after scan
> [ 489.426263] rtw_8822cu 1-1.2:1.2: failed to send h2c command
>
> Another way is to simply perform a wifi rescan.
>
> Benchmarking shows that setting a high prio workqueue for tx/rx will
> significally improve things. Also compared alloc_workqueue with
> alloc_ordered_workqueue, but even thou the later seems to slightly
> improve things it's still quite easy to reproduce the above issues. So
> that leads to the decision to go for alloc_workqueue.
>
> Thanks to Ping-Ke Shih <[email protected]> that came up with the idea
> of exploring tweaking of the work queue's within a similar discussion.
>
> Fixes: a82dfd33d1237 ("wifi: rtw88: Add common USB chip support")
> Signed-off-by: Petter Mabacker <[email protected]>
> ---
> drivers/net/wireless/realtek/rtw88/usb.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/wireless/realtek/rtw88/usb.c b/drivers/net/wireless/realtek/rtw88/usb.c
> index 44a5fafb9905..bfe0845528ec 100644
> --- a/drivers/net/wireless/realtek/rtw88/usb.c
> +++ b/drivers/net/wireless/realtek/rtw88/usb.c
> @@ -716,7 +716,7 @@ static int rtw_usb_init_rx(struct rtw_dev *rtwdev)
> struct rtw_usb *rtwusb = rtw_get_usb_priv(rtwdev);
> int i;
>
> - rtwusb->rxwq = create_singlethread_workqueue("rtw88_usb: rx wq");
> + rtwusb->rxwq = alloc_workqueue("rtw88_usb: rx wq", WQ_UNBOUND | WQ_HIGHPRI, 0);
> if (!rtwusb->rxwq) {
> rtw_err(rtwdev, "failed to create RX work queue\n");
> return -ENOMEM;
> @@ -750,7 +750,7 @@ static int rtw_usb_init_tx(struct rtw_dev *rtwdev)
> struct rtw_usb *rtwusb = rtw_get_usb_priv(rtwdev);
> int i;
>
> - rtwusb->txwq = create_singlethread_workqueue("rtw88_usb: tx wq");
> + rtwusb->txwq = alloc_workqueue("rtw88_usb: tx wq", WQ_UNBOUND | WQ_HIGHPRI, 0);
> if (!rtwusb->txwq) {
> rtw_err(rtwdev, "failed to create TX work queue\n");
> return -ENOMEM;
Should this workqueue be ordered or not? Please check Tejun's patchset
about using ordered queues:
https://lore.kernel.org/lkml/[email protected]/
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
[email protected] writes:
>> From: Petter Mabacker <[email protected]>
>>
>> The rtw8822cu driver have problem to handle high rx or tx rates compared
>> with high load (such as high I/O) on slower systems, such as for example
>> i.MX6 SoloX and similar platforms.
>>
>> The problems are more frequent when having the access point close to the
>> device. On slower systems it's often enough to download a large file,
>> combined with generating I/O load to trigger:
>>
>> [ 374.763424] rtw_8822cu 1-1.2:1.2: failed to get tx report from firmware
>> [ 377.771790] rtw_8822cu 1-1.2:1.2: failed to send h2c command
>> [ 407.813460] rtw_8822cu 1-1.2:1.2: firmware failed to report density after scan
>> [ 414.965826] rtw_8822cu 1-1.2:1.2: failed to send h2c command
>> [ 444.993462] rtw_8822cu 1-1.2:1.2: firmware failed to report density after scan
>> [ 452.144551] rtw_8822cu 1-1.2:1.2: failed to send h2c command
>> [ 482.183445] rtw_8822cu 1-1.2:1.2: firmware failed to report density after scan
>> [ 489.426263] rtw_8822cu 1-1.2:1.2: failed to send h2c command
>>
>> Another way is to simply perform a wifi rescan.
>>
>> Benchmarking shows that setting a high prio workqueue for tx/rx will
>> significally improve things. Also compared alloc_workqueue with
>> alloc_ordered_workqueue, but even thou the later seems to slightly
>> improve things it's still quite easy to reproduce the above issues. So
>> that leads to the decision to go for alloc_workqueue.
>>
>> Thanks to Ping-Ke Shih <[email protected]> that came up with the idea
>> of exploring tweaking of the work queue's within a similar discussion.
>>
>> Fixes: a82dfd33d1237 ("wifi: rtw88: Add common USB chip support")
>> Signed-off-by: Petter Mabacker <[email protected]>
>> ---
>> drivers/net/wireless/realtek/rtw88/usb.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/wireless/realtek/rtw88/usb.c b/drivers/net/wireless/realtek/rtw88/usb.c
>> index 44a5fafb9905..bfe0845528ec 100644
>> --- a/drivers/net/wireless/realtek/rtw88/usb.c
>> +++ b/drivers/net/wireless/realtek/rtw88/usb.c
>> @@ -716,7 +716,7 @@ static int rtw_usb_init_rx(struct rtw_dev *rtwdev)
>> struct rtw_usb *rtwusb = rtw_get_usb_priv(rtwdev);
>> int i;
>>
>> - rtwusb->rxwq = create_singlethread_workqueue("rtw88_usb: rx wq");
>> + rtwusb->rxwq = alloc_workqueue("rtw88_usb: rx wq", WQ_UNBOUND | WQ_HIGHPRI, 0);
>> if (!rtwusb->rxwq) {
>> rtw_err(rtwdev, "failed to create RX work queue\n");
>> return -ENOMEM;
>> @@ -750,7 +750,7 @@ static int rtw_usb_init_tx(struct rtw_dev *rtwdev)
>> struct rtw_usb *rtwusb = rtw_get_usb_priv(rtwdev);
>> int i;
>>
>> - rtwusb->txwq = create_singlethread_workqueue("rtw88_usb: tx wq");
>> + rtwusb->txwq = alloc_workqueue("rtw88_usb: tx wq", WQ_UNBOUND | WQ_HIGHPRI, 0);
>> if (!rtwusb->txwq) {
>> rtw_err(rtwdev, "failed to create TX work queue\n");
>> return -ENOMEM;
>Should this workqueue be ordered or not? Please check Tejun's patchset
>about using ordered queues:
>https://lore.kernel.org/lkml/[email protected]/
Thanks for pointing out this interesting patchset. As described in the
commit msg, I did play around with alloc_ordered_workqueue. But at least
on the slower systems I tested it on (i.MX6 SoloX and BCM2835) it worked
a bit better, but I was still able to reproduce the above mention issue.
So I tried to instead use alloc_workqueue and set max_active=0 and that
seems to be enough to make things a lot more stable.
However after reading Tejun's patchet I'm very intersted of feedback if
you or someone else have comments about using alloc_workqueue with
max_active=0 , or if this can give some other issues? It seems to work
fine for me when running it also on a i.MX8 multicore system.
>
>--
>https://patchwork.kernel.org/project/linux-wireless/list/
>
>https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
> -----Original Message-----
> From: [email protected] <[email protected]>
> Sent: Tuesday, May 30, 2023 9:09 PM
> To: [email protected]
> Cc: [email protected]; [email protected]; [email protected]; [email protected];
> [email protected]; [email protected]; [email protected]; [email protected]; Ping-Ke
> Shih <[email protected]>; [email protected]
> Subject: Re: [PATCH] wifi: rtw88: usb: Make work queues high prio
>
> [email protected] writes:
>
> >> From: Petter Mabacker <[email protected]>
> >>
> >> The rtw8822cu driver have problem to handle high rx or tx rates compared
> >> with high load (such as high I/O) on slower systems, such as for example
> >> i.MX6 SoloX and similar platforms.
> >>
> >> The problems are more frequent when having the access point close to the
> >> device. On slower systems it's often enough to download a large file,
> >> combined with generating I/O load to trigger:
> >>
> >> [ 374.763424] rtw_8822cu 1-1.2:1.2: failed to get tx report from firmware
> >> [ 377.771790] rtw_8822cu 1-1.2:1.2: failed to send h2c command
> >> [ 407.813460] rtw_8822cu 1-1.2:1.2: firmware failed to report density after scan
> >> [ 414.965826] rtw_8822cu 1-1.2:1.2: failed to send h2c command
> >> [ 444.993462] rtw_8822cu 1-1.2:1.2: firmware failed to report density after scan
> >> [ 452.144551] rtw_8822cu 1-1.2:1.2: failed to send h2c command
> >> [ 482.183445] rtw_8822cu 1-1.2:1.2: firmware failed to report density after scan
> >> [ 489.426263] rtw_8822cu 1-1.2:1.2: failed to send h2c command
> >>
> >> Another way is to simply perform a wifi rescan.
> >>
> >> Benchmarking shows that setting a high prio workqueue for tx/rx will
> >> significally improve things. Also compared alloc_workqueue with
> >> alloc_ordered_workqueue, but even thou the later seems to slightly
> >> improve things it's still quite easy to reproduce the above issues. So
> >> that leads to the decision to go for alloc_workqueue.
> >>
> >> Thanks to Ping-Ke Shih <[email protected]> that came up with the idea
> >> of exploring tweaking of the work queue's within a similar discussion.
> >>
> >> Fixes: a82dfd33d1237 ("wifi: rtw88: Add common USB chip support")
> >> Signed-off-by: Petter Mabacker <[email protected]>
> >> ---
> >> drivers/net/wireless/realtek/rtw88/usb.c | 4 ++--
> >> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/net/wireless/realtek/rtw88/usb.c b/drivers/net/wireless/realtek/rtw88/usb.c
> >> index 44a5fafb9905..bfe0845528ec 100644
> >> --- a/drivers/net/wireless/realtek/rtw88/usb.c
> >> +++ b/drivers/net/wireless/realtek/rtw88/usb.c
> >> @@ -716,7 +716,7 @@ static int rtw_usb_init_rx(struct rtw_dev *rtwdev)
> >> struct rtw_usb *rtwusb = rtw_get_usb_priv(rtwdev);
> >> int i;
> >>
> >> - rtwusb->rxwq = create_singlethread_workqueue("rtw88_usb: rx wq");
> >> + rtwusb->rxwq = alloc_workqueue("rtw88_usb: rx wq", WQ_UNBOUND | WQ_HIGHPRI, 0);
> >> if (!rtwusb->rxwq) {
> >> rtw_err(rtwdev, "failed to create RX work queue\n");
> >> return -ENOMEM;
> >> @@ -750,7 +750,7 @@ static int rtw_usb_init_tx(struct rtw_dev *rtwdev)
> >> struct rtw_usb *rtwusb = rtw_get_usb_priv(rtwdev);
> >> int i;
> >>
> >> - rtwusb->txwq = create_singlethread_workqueue("rtw88_usb: tx wq");
> >> + rtwusb->txwq = alloc_workqueue("rtw88_usb: tx wq", WQ_UNBOUND | WQ_HIGHPRI, 0);
> >> if (!rtwusb->txwq) {
> >> rtw_err(rtwdev, "failed to create TX work queue\n");
> >> return -ENOMEM;
>
> >Should this workqueue be ordered or not? Please check Tejun's patchset
> >about using ordered queues:
>
> >https://lore.kernel.org/lkml/[email protected]/
>
> Thanks for pointing out this interesting patchset. As described in the
> commit msg, I did play around with alloc_ordered_workqueue. But at least
> on the slower systems I tested it on (i.MX6 SoloX and BCM2835) it worked
> a bit better, but I was still able to reproduce the above mention issue.
> So I tried to instead use alloc_workqueue and set max_active=0 and that
> seems to be enough to make things a lot more stable.
>
> However after reading Tejun's patchet I'm very intersted of feedback if
> you or someone else have comments about using alloc_workqueue with
> max_active=0 , or if this can give some other issues? It seems to work
> fine for me when running it also on a i.MX8 multicore system.
>
Both rtwusb->rxwq and rtwusb->txwq are only queued single one work respectively,
so I thought alloc_workqueue() and alloc_ordered_workqueue() would get the same
results, but it seems not. That is a little weird to me.
I'm not familiar with workqueue, but I think we can bisect arguments to address
what impact the results.
First we can expand macro alloc_ordered_workqueue() below
rtwusb->txwq = alloc_ordered_workqueue("rtw88_usb: tx wq", WQ_HIGHPRI);
into
rtwusb->txwq = alloc_workqueue("rtw88_usb: tx wq",
WQ_UNBOUND | __WQ_ORDERED | __WQ_ORDERED_EXPLICIT |
WQ_HIGHPRI, 1);
Secondly, compare the one you are using:
rtwusb->txwq = alloc_workqueue("rtw88_usb: tx wq", WQ_UNBOUND | WQ_HIGHPRI, 0);
Then, we can align the arguments one-by-one to know which argument dominates
the result.
Ping-Ke
>> -----Original Message-----
>> From: [email protected] <[email protected]>
>> Sent: Tuesday, May 30, 2023 9:09 PM
>> To: [email protected]
>> Cc: [email protected]; [email protected]; [email protected]; [email protected];
>> [email protected]; [email protected]; [email protected]; [email protected]; Ping-Ke
>> Shih <[email protected]>; [email protected]
>> Subject: Re: [PATCH] wifi: rtw88: usb: Make work queues high prio
>>
>> [email protected] writes:
>>
>> >> From: Petter Mabacker <[email protected]>
>> >>
>> >> The rtw8822cu driver have problem to handle high rx or tx rates compared
>> >> with high load (such as high I/O) on slower systems, such as for example
>> >> i.MX6 SoloX and similar platforms.
>> >>
>> >> The problems are more frequent when having the access point close to the
>> >> device. On slower systems it's often enough to download a large file,
>> >> combined with generating I/O load to trigger:
>> >>
>> >> [ 374.763424] rtw_8822cu 1-1.2:1.2: failed to get tx report from firmware
>> >> [ 377.771790] rtw_8822cu 1-1.2:1.2: failed to send h2c command
>> >> [ 407.813460] rtw_8822cu 1-1.2:1.2: firmware failed to report density after scan
>> >> [ 414.965826] rtw_8822cu 1-1.2:1.2: failed to send h2c command
>> >> [ 444.993462] rtw_8822cu 1-1.2:1.2: firmware failed to report density after scan
>> >> [ 452.144551] rtw_8822cu 1-1.2:1.2: failed to send h2c command
>> >> [ 482.183445] rtw_8822cu 1-1.2:1.2: firmware failed to report density after scan
>> >> [ 489.426263] rtw_8822cu 1-1.2:1.2: failed to send h2c command
>> >>
>> >> Another way is to simply perform a wifi rescan.
>> >>
>> >> Benchmarking shows that setting a high prio workqueue for tx/rx will
>> >> significally improve things. Also compared alloc_workqueue with
>> >> alloc_ordered_workqueue, but even thou the later seems to slightly
>> >> improve things it's still quite easy to reproduce the above issues. So
>> >> that leads to the decision to go for alloc_workqueue.
>> >>
>> >> Thanks to Ping-Ke Shih <[email protected]> that came up with the idea
>> >> of exploring tweaking of the work queue's within a similar discussion.
>> >>
>> >> Fixes: a82dfd33d1237 ("wifi: rtw88: Add common USB chip support")
>> >> Signed-off-by: Petter Mabacker <[email protected]>
>> >> ---
>> >> drivers/net/wireless/realtek/rtw88/usb.c | 4 ++--
>> >> 1 file changed, 2 insertions(+), 2 deletions(-)
>> >>
>> >> diff --git a/drivers/net/wireless/realtek/rtw88/usb.c b/drivers/net/wireless/realtek/rtw88/usb.c
>> >> index 44a5fafb9905..bfe0845528ec 100644
>> >> --- a/drivers/net/wireless/realtek/rtw88/usb.c
>> >> +++ b/drivers/net/wireless/realtek/rtw88/usb.c
>> >> @@ -716,7 +716,7 @@ static int rtw_usb_init_rx(struct rtw_dev *rtwdev)
>> >> struct rtw_usb *rtwusb = rtw_get_usb_priv(rtwdev);
>> >> int i;
>> >>
>> >> - rtwusb->rxwq = create_singlethread_workqueue("rtw88_usb: rx wq");
>> >> + rtwusb->rxwq = alloc_workqueue("rtw88_usb: rx wq", WQ_UNBOUND | WQ_HIGHPRI, 0);
>> >> if (!rtwusb->rxwq) {
>> >> rtw_err(rtwdev, "failed to create RX work queue\n");
>> >> return -ENOMEM;
>> >> @@ -750,7 +750,7 @@ static int rtw_usb_init_tx(struct rtw_dev *rtwdev)
>> >> struct rtw_usb *rtwusb = rtw_get_usb_priv(rtwdev);
>> >> int i;
>> >>
>> >> - rtwusb->txwq = create_singlethread_workqueue("rtw88_usb: tx wq");
>> >> + rtwusb->txwq = alloc_workqueue("rtw88_usb: tx wq", WQ_UNBOUND | WQ_HIGHPRI, 0);
>> >> if (!rtwusb->txwq) {
>> >> rtw_err(rtwdev, "failed to create TX work queue\n");
>> >> return -ENOMEM;
>>
>> >Should this workqueue be ordered or not? Please check Tejun's patchset
>> >about using ordered queues:
>>
>> >https://lore.kernel.org/lkml/[email protected]/
>>
>> Thanks for pointing out this interesting patchset. As described in the
>> commit msg, I did play around with alloc_ordered_workqueue. But at least
>> on the slower systems I tested it on (i.MX6 SoloX and BCM2835) it worked
>> a bit better, but I was still able to reproduce the above mention issue.
>> So I tried to instead use alloc_workqueue and set max_active=0 and that
>> seems to be enough to make things a lot more stable.
>>
>> However after reading Tejun's patchet I'm very intersted of feedback if
>> you or someone else have comments about using alloc_workqueue with
>> max_active=0 , or if this can give some other issues? It seems to work
>> fine for me when running it also on a i.MX8 multicore system.
>>
>Both rtwusb->rxwq and rtwusb->txwq are only queued single one work respectively,
>so I thought alloc_workqueue() and alloc_ordered_workqueue() would get the same
>results, but it seems not. That is a little weird to me.
>
>I'm not familiar with workqueue, but I think we can bisect arguments to address
>what impact the results.
>
>First we can expand macro alloc_ordered_workqueue() below
> rtwusb->txwq = alloc_ordered_workqueue("rtw88_usb: tx wq", WQ_HIGHPRI);
>into
> rtwusb->txwq = alloc_workqueue("rtw88_usb: tx wq",
> WQ_UNBOUND | __WQ_ORDERED | __WQ_ORDERED_EXPLICIT |
> WQ_HIGHPRI, 1);
>
>Secondly, compare the one you are using:
> rtwusb->txwq = alloc_workqueue("rtw88_usb: tx wq", WQ_UNBOUND | WQ_HIGHPRI, 0);
>
>Then, we can align the arguments one-by-one to know which argument dominates
>the result.
>
>Ping-Ke
>
Thanks for feedback. I have encountered some issues with HW offload
scan, that might have fooled me during my benchmarking (I used rescan as
part of my stresstest). See
https://lore.kernel.org/linux-wireless/[email protected]/T/#u
for more info. But I have temporarily disabled the HW offload scan so I
will re-try my benchmarking with above suggestions in mind. I will post
here when I have some results.
BR Petter