| Message ID | 1513683236-13402-1-git-send-email-sgruszka@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 3d8f162cb8ecd8cb5f77d92ed27bdc7d8aeb2bae |
| Delegated to: | Kalle Valo |
| Headers | show |
On a WL-330N3G device, these patches seems to make the situation much better. In other words, I wasn't able to reproduce the stall on this chipset. On MT7620 a stall was never seen directly, but tests with these patches revealed no stall. So I think it's good to have these merged. Tested-by: Enrico Mioso@gmail.com On Tue, 19 Dec 2017, Stanislaw Gruszka wrote: > Date: Tue, 19 Dec 2017 12:33:55 > From: Stanislaw Gruszka <sgruszka@redhat.com> > To: linux-wireless@vger.kernel.org > Cc: Helmut Schaa <helmut.schaa@googlemail.com>, > Enrico Mioso <mrkiko.rs@gmail.com>, Daniel Golle <daniel@makrotopia.org>, > Felix Fietkau <nbd@nbd.name>, Stanislaw Gruszka <sgruszka@redhat.com> > Subject: [PATCH 1/2] rt2x00: pause almost full queue early > > Do not drop &queue->tx_lock and acquire it again to pause queue when > available entries are below the threshold. > > Patch should mitigate problem of frequently printed errors: > "Error - Dropping frame due to full tx queue" > > Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com> > --- > drivers/net/wireless/ralink/rt2x00/rt2x00mac.c | 10 ---------- > drivers/net/wireless/ralink/rt2x00/rt2x00queue.c | 8 ++++++++ > 2 files changed, 8 insertions(+), 10 deletions(-) > > diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00mac.c b/drivers/net/wireless/ralink/rt2x00/rt2x00mac.c > index ecc96312a370..c8a6f163102f 100644 > --- a/drivers/net/wireless/ralink/rt2x00/rt2x00mac.c > +++ b/drivers/net/wireless/ralink/rt2x00/rt2x00mac.c > @@ -152,16 +152,6 @@ void rt2x00mac_tx(struct ieee80211_hw *hw, > if (unlikely(rt2x00queue_write_tx_frame(queue, skb, control->sta, false))) > goto exit_fail; > > - /* > - * Pausing queue has to be serialized with rt2x00lib_txdone(). Note > - * we should not use spin_lock_bh variant as bottom halve was already > - * disabled before ieee80211_xmit() call. > - */ > - spin_lock(&queue->tx_lock); > - if (rt2x00queue_threshold(queue)) > - rt2x00queue_pause_queue(queue); > - spin_unlock(&queue->tx_lock); > - > return; > > exit_fail: > diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c b/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c > index a2c1ca5c76d1..1ad51e56bc59 100644 > --- a/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c > +++ b/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c > @@ -715,6 +715,14 @@ int rt2x00queue_write_tx_frame(struct data_queue *queue, struct sk_buff *skb, > rt2x00queue_kick_tx_queue(queue, &txdesc); > > out: > + /* > + * Pausing queue has to be serialized with rt2x00lib_txdone(), so we > + * do this under queue->tx_lock. Bottom halve was already disabled > + * before ieee80211_xmit() call. > + */ > + if (rt2x00queue_threshold(queue)) > + rt2x00queue_pause_queue(queue); > + > spin_unlock(&queue->tx_lock); > return ret; > } > -- > 1.9.3 > >
Oops, sorry, wrong tag Tested-by: Enrico Mioso <mrkiko.rs@gmail.com> On Tue, 19 Dec 2017, Stanislaw Gruszka wrote: > Date: Tue, 19 Dec 2017 12:33:55 > From: Stanislaw Gruszka <sgruszka@redhat.com> > To: linux-wireless@vger.kernel.org > Cc: Helmut Schaa <helmut.schaa@googlemail.com>, > Enrico Mioso <mrkiko.rs@gmail.com>, Daniel Golle <daniel@makrotopia.org>, > Felix Fietkau <nbd@nbd.name>, Stanislaw Gruszka <sgruszka@redhat.com> > Subject: [PATCH 1/2] rt2x00: pause almost full queue early > > Do not drop &queue->tx_lock and acquire it again to pause queue when > available entries are below the threshold. > > Patch should mitigate problem of frequently printed errors: > "Error - Dropping frame due to full tx queue" > > Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com> > --- > drivers/net/wireless/ralink/rt2x00/rt2x00mac.c | 10 ---------- > drivers/net/wireless/ralink/rt2x00/rt2x00queue.c | 8 ++++++++ > 2 files changed, 8 insertions(+), 10 deletions(-) > > diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00mac.c b/drivers/net/wireless/ralink/rt2x00/rt2x00mac.c > index ecc96312a370..c8a6f163102f 100644 > --- a/drivers/net/wireless/ralink/rt2x00/rt2x00mac.c > +++ b/drivers/net/wireless/ralink/rt2x00/rt2x00mac.c > @@ -152,16 +152,6 @@ void rt2x00mac_tx(struct ieee80211_hw *hw, > if (unlikely(rt2x00queue_write_tx_frame(queue, skb, control->sta, false))) > goto exit_fail; > > - /* > - * Pausing queue has to be serialized with rt2x00lib_txdone(). Note > - * we should not use spin_lock_bh variant as bottom halve was already > - * disabled before ieee80211_xmit() call. > - */ > - spin_lock(&queue->tx_lock); > - if (rt2x00queue_threshold(queue)) > - rt2x00queue_pause_queue(queue); > - spin_unlock(&queue->tx_lock); > - > return; > > exit_fail: > diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c b/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c > index a2c1ca5c76d1..1ad51e56bc59 100644 > --- a/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c > +++ b/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c > @@ -715,6 +715,14 @@ int rt2x00queue_write_tx_frame(struct data_queue *queue, struct sk_buff *skb, > rt2x00queue_kick_tx_queue(queue, &txdesc); > > out: > + /* > + * Pausing queue has to be serialized with rt2x00lib_txdone(), so we > + * do this under queue->tx_lock. Bottom halve was already disabled > + * before ieee80211_xmit() call. > + */ > + if (rt2x00queue_threshold(queue)) > + rt2x00queue_pause_queue(queue); > + > spin_unlock(&queue->tx_lock); > return ret; > } > -- > 1.9.3 > >
Stanislaw Gruszka <sgruszka@redhat.com> wrote: > Do not drop &queue->tx_lock and acquire it again to pause queue when > available entries are below the threshold. > > Patch should mitigate problem of frequently printed errors: > "Error - Dropping frame due to full tx queue" > > Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com> > Tested-by: Enrico Mioso@gmail.com > Tested-by: Enrico Mioso <mrkiko.rs@gmail.com> 2 patches applied to wireless-drivers-next.git, thanks. 3d8f162cb8ec rt2x00: pause almost full queue early 6dd80efd75ce rt2x00: do not pause queue unconditionally on error path
diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00mac.c b/drivers/net/wireless/ralink/rt2x00/rt2x00mac.c index ecc96312a370..c8a6f163102f 100644 --- a/drivers/net/wireless/ralink/rt2x00/rt2x00mac.c +++ b/drivers/net/wireless/ralink/rt2x00/rt2x00mac.c @@ -152,16 +152,6 @@ void rt2x00mac_tx(struct ieee80211_hw *hw, if (unlikely(rt2x00queue_write_tx_frame(queue, skb, control->sta, false))) goto exit_fail; - /* - * Pausing queue has to be serialized with rt2x00lib_txdone(). Note - * we should not use spin_lock_bh variant as bottom halve was already - * disabled before ieee80211_xmit() call. - */ - spin_lock(&queue->tx_lock); - if (rt2x00queue_threshold(queue)) - rt2x00queue_pause_queue(queue); - spin_unlock(&queue->tx_lock); - return; exit_fail: diff --git a/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c b/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c index a2c1ca5c76d1..1ad51e56bc59 100644 --- a/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c +++ b/drivers/net/wireless/ralink/rt2x00/rt2x00queue.c @@ -715,6 +715,14 @@ int rt2x00queue_write_tx_frame(struct data_queue *queue, struct sk_buff *skb, rt2x00queue_kick_tx_queue(queue, &txdesc); out: + /* + * Pausing queue has to be serialized with rt2x00lib_txdone(), so we + * do this under queue->tx_lock. Bottom halve was already disabled + * before ieee80211_xmit() call. + */ + if (rt2x00queue_threshold(queue)) + rt2x00queue_pause_queue(queue); + spin_unlock(&queue->tx_lock); return ret; }
Do not drop &queue->tx_lock and acquire it again to pause queue when available entries are below the threshold. Patch should mitigate problem of frequently printed errors: "Error - Dropping frame due to full tx queue" Signed-off-by: Stanislaw Gruszka <sgruszka@redhat.com> --- drivers/net/wireless/ralink/rt2x00/rt2x00mac.c | 10 ---------- drivers/net/wireless/ralink/rt2x00/rt2x00queue.c | 8 ++++++++ 2 files changed, 8 insertions(+), 10 deletions(-)