* [PATCH 3.12] rt2800usb: slow down TX status polling
@ 2013-10-17 10:04 Stanislaw Gruszka
2013-10-17 14:39 ` Larry Finger
0 siblings, 1 reply; 6+ messages in thread
From: Stanislaw Gruszka @ 2013-10-17 10:04 UTC (permalink / raw)
To: linux-wireless; +Cc: users
Polling TX statuses too frequently has two negative effects. First is
randomly peek CPU usage, causing overall system functioning delays.
Second bad effect is that device is not able to fill TX statuses in
H/W register on some workloads and we get lot of timeouts like below:
ieee80211 phy4: rt2800usb_entry_txstatus_timeout: Warning - TX status timeout for entry 7 in queue 2
ieee80211 phy4: rt2800usb_entry_txstatus_timeout: Warning - TX status timeout for entry 7 in queue 2
ieee80211 phy4: rt2800usb_txdone: Warning - Got TX status for an empty queue 2, dropping
This not only cause flood of messages in dmesg, but also bad throughput,
since rate scaling algorithm can not work optimally.
In the future, we should probably make polling interval be adjusted
automatically, but for now just increase values, this make mentioned
problems gone.
Resolve:
https://bugzilla.kernel.org/show_bug.cgi?id=62781
Cc: stable@vger.kernel.org
Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
drivers/net/wireless/rt2x00/rt2800usb.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/rt2x00/rt2800usb.c b/drivers/net/wireless/rt2x00/rt2800usb.c
index 96677ce5..e095e61 100644
--- a/drivers/net/wireless/rt2x00/rt2800usb.c
+++ b/drivers/net/wireless/rt2x00/rt2800usb.c
@@ -176,8 +176,8 @@ static bool rt2800usb_tx_sta_fifo_read_completed(struct rt2x00_dev *rt2x00dev,
queue_work(rt2x00dev->workqueue, &rt2x00dev->txdone_work);
if (rt2800usb_txstatus_pending(rt2x00dev)) {
- /* Read register after 250 us */
- hrtimer_start(&rt2x00dev->txstatus_timer, ktime_set(0, 250000),
+ /* Read register after 1 ms */
+ hrtimer_start(&rt2x00dev->txstatus_timer, ktime_set(0, 1000000),
HRTIMER_MODE_REL);
return false;
}
@@ -202,8 +202,8 @@ static void rt2800usb_async_read_tx_status(struct rt2x00_dev *rt2x00dev)
if (test_and_set_bit(TX_STATUS_READING, &rt2x00dev->flags))
return;
- /* Read TX_STA_FIFO register after 500 us */
- hrtimer_start(&rt2x00dev->txstatus_timer, ktime_set(0, 500000),
+ /* Read TX_STA_FIFO register after 2 ms */
+ hrtimer_start(&rt2x00dev->txstatus_timer, ktime_set(0, 2000000),
HRTIMER_MODE_REL);
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 3.12] rt2800usb: slow down TX status polling
2013-10-17 10:04 [PATCH 3.12] rt2800usb: slow down TX status polling Stanislaw Gruszka
@ 2013-10-17 14:39 ` Larry Finger
2013-10-18 9:42 ` Stanislaw Gruszka
0 siblings, 1 reply; 6+ messages in thread
From: Larry Finger @ 2013-10-17 14:39 UTC (permalink / raw)
To: Stanislaw Gruszka, linux-wireless; +Cc: users
On 10/17/2013 05:04 AM, Stanislaw Gruszka wrote:
> Polling TX statuses too frequently has two negative effects. First is
> randomly peek CPU usage, causing overall system functioning delays.
> Second bad effect is that device is not able to fill TX statuses in
> H/W register on some workloads and we get lot of timeouts like below:
>
> ieee80211 phy4: rt2800usb_entry_txstatus_timeout: Warning - TX status timeout for entry 7 in queue 2
> ieee80211 phy4: rt2800usb_entry_txstatus_timeout: Warning - TX status timeout for entry 7 in queue 2
> ieee80211 phy4: rt2800usb_txdone: Warning - Got TX status for an empty queue 2, dropping
>
> This not only cause flood of messages in dmesg, but also bad throughput,
> since rate scaling algorithm can not work optimally.
>
> In the future, we should probably make polling interval be adjusted
> automatically, but for now just increase values, this make mentioned
> problems gone.
>
> Resolve:
> https://bugzilla.kernel.org/show_bug.cgi?id=62781
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
> ---
> drivers/net/wireless/rt2x00/rt2800usb.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/wireless/rt2x00/rt2800usb.c b/drivers/net/wireless/rt2x00/rt2800usb.c
> index 96677ce5..e095e61 100644
> --- a/drivers/net/wireless/rt2x00/rt2800usb.c
> +++ b/drivers/net/wireless/rt2x00/rt2800usb.c
> @@ -176,8 +176,8 @@ static bool rt2800usb_tx_sta_fifo_read_completed(struct rt2x00_dev *rt2x00dev,
> queue_work(rt2x00dev->workqueue, &rt2x00dev->txdone_work);
>
> if (rt2800usb_txstatus_pending(rt2x00dev)) {
> - /* Read register after 250 us */
> - hrtimer_start(&rt2x00dev->txstatus_timer, ktime_set(0, 250000),
> + /* Read register after 1 ms */
> + hrtimer_start(&rt2x00dev->txstatus_timer, ktime_set(0, 1000000),
> HRTIMER_MODE_REL);
> return false;
> }
> @@ -202,8 +202,8 @@ static void rt2800usb_async_read_tx_status(struct rt2x00_dev *rt2x00dev)
> if (test_and_set_bit(TX_STATUS_READING, &rt2x00dev->flags))
> return;
>
> - /* Read TX_STA_FIFO register after 500 us */
> - hrtimer_start(&rt2x00dev->txstatus_timer, ktime_set(0, 500000),
> + /* Read TX_STA_FIFO register after 2 ms */
> + hrtimer_start(&rt2x00dev->txstatus_timer, ktime_set(0, 2000000),
> HRTIMER_MODE_REL);
> }
I suggest getting rid of the magic numbers as long as you are making this
change. A single define could handle the delay time for the two cases.
Larry
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 3.12] rt2800usb: slow down TX status polling
2013-10-17 14:39 ` Larry Finger
@ 2013-10-18 9:42 ` Stanislaw Gruszka
2013-10-19 8:33 ` Gertjan van Wingerde
0 siblings, 1 reply; 6+ messages in thread
From: Stanislaw Gruszka @ 2013-10-18 9:42 UTC (permalink / raw)
To: Larry Finger; +Cc: linux-wireless, users
On Thu, Oct 17, 2013 at 09:39:06AM -0500, Larry Finger wrote:
> I suggest getting rid of the magic numbers as long as you are making
> this change. A single define could handle the delay time for the two
> cases.
Thanks for sugestion Larry, though I do not see clear benefit of
introduce define since those magic numbers are just time values
expressed in nano seconds. Anyway patch with define added below.
John can pick it, if he thinks it is better.
Stanislaw
---
>From 813e0bde7340bad7d3401c6aa2a3f8635ec49597 Mon Sep 17 00:00:00 2001
From: Stanislaw Gruszka <sgruszka@redhat.com>
Date: Fri, 18 Oct 2013 11:36:54 +0200
Subject: [PATCH] rt2800usb: slow down TX status polling
Polling TX statuses too frequently has two negative effects. First is
randomly peek CPU usage, causing overall system functioning delays.
Second bad effect is that device is not able to fill TX statuses in
H/W register on some workloads and we get lot of timeouts like below:
ieee80211 phy4: rt2800usb_entry_txstatus_timeout: Warning - TX status timeout for entry 7 in queue 2
ieee80211 phy4: rt2800usb_entry_txstatus_timeout: Warning - TX status timeout for entry 7 in queue 2
ieee80211 phy4: rt2800usb_txdone: Warning - Got TX status for an empty queue 2, dropping
This not only cause flood of messages in dmesg, but also bad throughput,
since rate scaling algorithm can not work optimally.
In the future, we should probably make polling interval be adjusted
automatically, but for now just increase values, this make mentioned
problems gone.
Resolve:
https://bugzilla.kernel.org/show_bug.cgi?id=62781
Cc: stable@vger.kernel.org
Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
---
drivers/net/wireless/rt2x00/rt2800usb.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/net/wireless/rt2x00/rt2800usb.c b/drivers/net/wireless/rt2x00/rt2800usb.c
index 96677ce5..997df03 100644
--- a/drivers/net/wireless/rt2x00/rt2800usb.c
+++ b/drivers/net/wireless/rt2x00/rt2800usb.c
@@ -148,6 +148,8 @@ static bool rt2800usb_txstatus_timeout(struct rt2x00_dev *rt2x00dev)
return false;
}
+#define TXSTATUS_READ_INTERVAL 1000000
+
static bool rt2800usb_tx_sta_fifo_read_completed(struct rt2x00_dev *rt2x00dev,
int urb_status, u32 tx_status)
{
@@ -176,8 +178,9 @@ static bool rt2800usb_tx_sta_fifo_read_completed(struct rt2x00_dev *rt2x00dev,
queue_work(rt2x00dev->workqueue, &rt2x00dev->txdone_work);
if (rt2800usb_txstatus_pending(rt2x00dev)) {
- /* Read register after 250 us */
- hrtimer_start(&rt2x00dev->txstatus_timer, ktime_set(0, 250000),
+ /* Read register after 1 ms */
+ hrtimer_start(&rt2x00dev->txstatus_timer,
+ ktime_set(0, TXSTATUS_READ_INTERVAL),
HRTIMER_MODE_REL);
return false;
}
@@ -202,8 +205,9 @@ static void rt2800usb_async_read_tx_status(struct rt2x00_dev *rt2x00dev)
if (test_and_set_bit(TX_STATUS_READING, &rt2x00dev->flags))
return;
- /* Read TX_STA_FIFO register after 500 us */
- hrtimer_start(&rt2x00dev->txstatus_timer, ktime_set(0, 500000),
+ /* Read TX_STA_FIFO register after 2 ms */
+ hrtimer_start(&rt2x00dev->txstatus_timer,
+ ktime_set(0, 2*TXSTATUS_READ_INTERVAL),
HRTIMER_MODE_REL);
}
--
1.8.3.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 3.12] rt2800usb: slow down TX status polling
2013-10-18 9:42 ` Stanislaw Gruszka
@ 2013-10-19 8:33 ` Gertjan van Wingerde
2013-11-20 12:53 ` [rt2x00-users] [PATCH 3.12] rt2800usb: slow down TX statuspolling Marc Dietrich
0 siblings, 1 reply; 6+ messages in thread
From: Gertjan van Wingerde @ 2013-10-19 8:33 UTC (permalink / raw)
To: Stanislaw Gruszka, Larry Finger; +Cc: linux-wireless, users
On 10/18/13 11:42, Stanislaw Gruszka wrote:
> On Thu, Oct 17, 2013 at 09:39:06AM -0500, Larry Finger wrote:
>> I suggest getting rid of the magic numbers as long as you are making
>> this change. A single define could handle the delay time for the two
>> cases.
>
> Thanks for sugestion Larry, though I do not see clear benefit of
> introduce define since those magic numbers are just time values
> expressed in nano seconds. Anyway patch with define added below.
> John can pick it, if he thinks it is better.
>
> Stanislaw
> ---
> From 813e0bde7340bad7d3401c6aa2a3f8635ec49597 Mon Sep 17 00:00:00 2001
> From: Stanislaw Gruszka <sgruszka@redhat.com>
> Date: Fri, 18 Oct 2013 11:36:54 +0200
> Subject: [PATCH] rt2800usb: slow down TX status polling
>
> Polling TX statuses too frequently has two negative effects. First is
> randomly peek CPU usage, causing overall system functioning delays.
> Second bad effect is that device is not able to fill TX statuses in
> H/W register on some workloads and we get lot of timeouts like below:
>
> ieee80211 phy4: rt2800usb_entry_txstatus_timeout: Warning - TX status timeout for entry 7 in queue 2
> ieee80211 phy4: rt2800usb_entry_txstatus_timeout: Warning - TX status timeout for entry 7 in queue 2
> ieee80211 phy4: rt2800usb_txdone: Warning - Got TX status for an empty queue 2, dropping
>
> This not only cause flood of messages in dmesg, but also bad throughput,
> since rate scaling algorithm can not work optimally.
>
> In the future, we should probably make polling interval be adjusted
> automatically, but for now just increase values, this make mentioned
> problems gone.
>
> Resolve:
> https://bugzilla.kernel.org/show_bug.cgi?id=62781
>
> Cc: stable@vger.kernel.org
> Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
I don't care which version gets picked. In both cases:
Acked-by: Gertjan van Wingerde <gwingerde@gmail.com>
> ---
> drivers/net/wireless/rt2x00/rt2800usb.c | 12 ++++++++----
> 1 file changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/wireless/rt2x00/rt2800usb.c b/drivers/net/wireless/rt2x00/rt2800usb.c
> index 96677ce5..997df03 100644
> --- a/drivers/net/wireless/rt2x00/rt2800usb.c
> +++ b/drivers/net/wireless/rt2x00/rt2800usb.c
> @@ -148,6 +148,8 @@ static bool rt2800usb_txstatus_timeout(struct rt2x00_dev *rt2x00dev)
> return false;
> }
>
> +#define TXSTATUS_READ_INTERVAL 1000000
> +
> static bool rt2800usb_tx_sta_fifo_read_completed(struct rt2x00_dev *rt2x00dev,
> int urb_status, u32 tx_status)
> {
> @@ -176,8 +178,9 @@ static bool rt2800usb_tx_sta_fifo_read_completed(struct rt2x00_dev *rt2x00dev,
> queue_work(rt2x00dev->workqueue, &rt2x00dev->txdone_work);
>
> if (rt2800usb_txstatus_pending(rt2x00dev)) {
> - /* Read register after 250 us */
> - hrtimer_start(&rt2x00dev->txstatus_timer, ktime_set(0, 250000),
> + /* Read register after 1 ms */
> + hrtimer_start(&rt2x00dev->txstatus_timer,
> + ktime_set(0, TXSTATUS_READ_INTERVAL),
> HRTIMER_MODE_REL);
> return false;
> }
> @@ -202,8 +205,9 @@ static void rt2800usb_async_read_tx_status(struct rt2x00_dev *rt2x00dev)
> if (test_and_set_bit(TX_STATUS_READING, &rt2x00dev->flags))
> return;
>
> - /* Read TX_STA_FIFO register after 500 us */
> - hrtimer_start(&rt2x00dev->txstatus_timer, ktime_set(0, 500000),
> + /* Read TX_STA_FIFO register after 2 ms */
> + hrtimer_start(&rt2x00dev->txstatus_timer,
> + ktime_set(0, 2*TXSTATUS_READ_INTERVAL),
> HRTIMER_MODE_REL);
> }
>
>
--
---
Gertjan
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [rt2x00-users] [PATCH 3.12] rt2800usb: slow down TX statuspolling
2013-10-19 8:33 ` Gertjan van Wingerde
@ 2013-11-20 12:53 ` Marc Dietrich
2013-11-20 14:12 ` Stanislaw Gruszka
0 siblings, 1 reply; 6+ messages in thread
From: Marc Dietrich @ 2013-11-20 12:53 UTC (permalink / raw)
To: users; +Cc: Gertjan van Wingerde, Stanislaw Gruszka, Larry Finger,
linux-wireless
Am Samstag, 19. Oktober 2013, 10:33:06 schrieb Gertjan van Wingerde:
> On 10/18/13 11:42, Stanislaw Gruszka wrote:
> > On Thu, Oct 17, 2013 at 09:39:06AM -0500, Larry Finger wrote:
> >> I suggest getting rid of the magic numbers as long as you are making
> >> this change. A single define could handle the delay time for the two
> >> cases.
> >
> > Thanks for sugestion Larry, though I do not see clear benefit of
> > introduce define since those magic numbers are just time values
> > expressed in nano seconds. Anyway patch with define added below.
> > John can pick it, if he thinks it is better.
> >
> > Stanislaw
> > ---
> > From 813e0bde7340bad7d3401c6aa2a3f8635ec49597 Mon Sep 17 00:00:00 2001
> > From: Stanislaw Gruszka <sgruszka@redhat.com>
> > Date: Fri, 18 Oct 2013 11:36:54 +0200
> > Subject: [PATCH] rt2800usb: slow down TX status polling
> >
> > Polling TX statuses too frequently has two negative effects. First is
> > randomly peek CPU usage, causing overall system functioning delays.
> > Second bad effect is that device is not able to fill TX statuses in
> > H/W register on some workloads and we get lot of timeouts like below:
> >
> > ieee80211 phy4: rt2800usb_entry_txstatus_timeout: Warning - TX status
> > timeout for entry 7 in queue 2 ieee80211 phy4:
> > rt2800usb_entry_txstatus_timeout: Warning - TX status timeout for entry 7
> > in queue 2 ieee80211 phy4: rt2800usb_txdone: Warning - Got TX status for
> > an empty queue 2, dropping
> >
> > This not only cause flood of messages in dmesg, but also bad throughput,
> > since rate scaling algorithm can not work optimally.
> >
> > In the future, we should probably make polling interval be adjusted
> > automatically, but for now just increase values, this make mentioned
> > problems gone.
> >
> > Resolve:
> > https://bugzilla.kernel.org/show_bug.cgi?id=62781
> >
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com>
>
> I don't care which version gets picked. In both cases:
>
> Acked-by: Gertjan van Wingerde <gwingerde@gmail.com>
did this ever hit a stable queue?
Marc
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-11-20 13:14 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-10-17 10:04 [PATCH 3.12] rt2800usb: slow down TX status polling Stanislaw Gruszka
2013-10-17 14:39 ` Larry Finger
2013-10-18 9:42 ` Stanislaw Gruszka
2013-10-19 8:33 ` Gertjan van Wingerde
2013-11-20 12:53 ` [rt2x00-users] [PATCH 3.12] rt2800usb: slow down TX statuspolling Marc Dietrich
2013-11-20 14:12 ` Stanislaw Gruszka
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).