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@xxxxxxxxxx> > 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@xxxxxxxxxxxxxxx > Signed-off-by: Stanislaw Gruszka <sgruszka@xxxxxxxxxx> I don't care which version gets picked. In both cases: Acked-by: Gertjan van Wingerde <gwingerde@xxxxxxxxx> > --- > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
