* [PATCH 0/2] brcmsmac: fix for regulatory rules and flush callback
@ 2012-09-05 9:49 Arend van Spriel
2012-09-05 9:49 ` [PATCH 1/2] brcmsmac: fix mismatch in number of custom regulatory rules Arend van Spriel
` (2 more replies)
0 siblings, 3 replies; 18+ messages in thread
From: Arend van Spriel @ 2012-09-05 9:49 UTC (permalink / raw)
To: John W. Linville; +Cc: Linux Wireless List, Arend van Spriel, Seth Forshee
Two fixes intended for the wireless tree. The issue with the .flush() has
been giving me a lot of email from RedHat's bugzilla server. Hope this
patch will silence that ;-)
The other issue was introduced with regulatory changes made by Seth Forshee
as he remove rule for channel 14 upon my review comment, but n_rules field
was not decreased so we got a random rule with bad side-effects.
Cc: Seth Forshee <seth.forshee@canonical.com>
Arend van Spriel (2):
brcmsmac: fix mismatch in number of custom regulatory rules
brcmsmac: rework of mac80211 .flush() callback operation
drivers/net/wireless/brcm80211/brcmsmac/channel.c | 2 +-
.../net/wireless/brcm80211/brcmsmac/mac80211_if.c | 41 +++++++++++++-------
.../net/wireless/brcm80211/brcmsmac/mac80211_if.h | 3 +-
drivers/net/wireless/brcm80211/brcmsmac/main.c | 21 +++-------
drivers/net/wireless/brcm80211/brcmsmac/pub.h | 4 +-
5 files changed, 38 insertions(+), 33 deletions(-)
--
1.7.9.5
^ permalink raw reply [flat|nested] 18+ messages in thread* [PATCH 1/2] brcmsmac: fix mismatch in number of custom regulatory rules 2012-09-05 9:49 [PATCH 0/2] brcmsmac: fix for regulatory rules and flush callback Arend van Spriel @ 2012-09-05 9:49 ` Arend van Spriel 2012-09-05 12:40 ` Seth Forshee 2012-09-05 9:49 ` [PATCH 2/2] brcmsmac: rework of mac80211 .flush() callback operation Arend van Spriel 2012-09-10 13:22 ` [PATCH 0/2] brcmsmac: fix for regulatory rules and flush callback Arend van Spriel 2 siblings, 1 reply; 18+ messages in thread From: Arend van Spriel @ 2012-09-05 9:49 UTC (permalink / raw) To: John W. Linville; +Cc: Linux Wireless List, Arend van Spriel, Seth Forshee The driver provides the cfg80211 regulatory framework with a set of custom rules. However, there was a mismatch in number of rules and the actual rules provided. This resulted in setting an invalid power level: ieee80211 phy0: brcms_ops_config: change channel 13 ieee80211 phy0: brcms_ops_config: Error setting power_level (8758364) Closer look in cfg80211 regulatory blurb showed following bogus rule: cfg80211: 0 KHz - -60446948 KHz @ 875836468 KHz), (875836468 mBi, 875836468 mBm) Cc: Seth Forshee <seth.forshee@canonical.com> Reviewed-by: Piotr Haber <phaber@broadcom.com> Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com> Signed-off-by: Arend van Spriel <arend@broadcom.com> --- drivers/net/wireless/brcm80211/brcmsmac/channel.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/wireless/brcm80211/brcmsmac/channel.c b/drivers/net/wireless/brcm80211/brcmsmac/channel.c index 7ed7d75..64a48f0 100644 --- a/drivers/net/wireless/brcm80211/brcmsmac/channel.c +++ b/drivers/net/wireless/brcm80211/brcmsmac/channel.c @@ -77,7 +77,7 @@ NL80211_RRF_NO_IBSS) static const struct ieee80211_regdomain brcms_regdom_x2 = { - .n_reg_rules = 7, + .n_reg_rules = 6, .alpha2 = "X2", .reg_rules = { BRCM_2GHZ_2412_2462, -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 1/2] brcmsmac: fix mismatch in number of custom regulatory rules 2012-09-05 9:49 ` [PATCH 1/2] brcmsmac: fix mismatch in number of custom regulatory rules Arend van Spriel @ 2012-09-05 12:40 ` Seth Forshee 0 siblings, 0 replies; 18+ messages in thread From: Seth Forshee @ 2012-09-05 12:40 UTC (permalink / raw) To: Arend van Spriel; +Cc: John W. Linville, Linux Wireless List On Wed, Sep 05, 2012 at 11:49:21AM +0200, Arend van Spriel wrote: > The driver provides the cfg80211 regulatory framework with a set of > custom rules. However, there was a mismatch in number of rules > and the actual rules provided. This resulted in setting an invalid > power level: > > ieee80211 phy0: brcms_ops_config: change channel 13 > ieee80211 phy0: brcms_ops_config: Error setting power_level (8758364) > > Closer look in cfg80211 regulatory blurb showed following bogus rule: > cfg80211: 0 KHz - -60446948 KHz @ 875836468 KHz), (875836468 mBi, 875836468 mBm) > > Cc: Seth Forshee <seth.forshee@canonical.com> > Reviewed-by: Piotr Haber <phaber@broadcom.com> > Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com> > Signed-off-by: Arend van Spriel <arend@broadcom.com> Oddly enough I don't see the bogus rule on my machine. But the fix is obviously correct. Reviewed-by: Seth Forshee <seth.forshee@canonical.com> ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH 2/2] brcmsmac: rework of mac80211 .flush() callback operation 2012-09-05 9:49 [PATCH 0/2] brcmsmac: fix for regulatory rules and flush callback Arend van Spriel 2012-09-05 9:49 ` [PATCH 1/2] brcmsmac: fix mismatch in number of custom regulatory rules Arend van Spriel @ 2012-09-05 9:49 ` Arend van Spriel 2012-09-05 10:20 ` Stanislaw Gruszka 2012-09-05 16:57 ` Seth Forshee 2012-09-10 13:22 ` [PATCH 0/2] brcmsmac: fix for regulatory rules and flush callback Arend van Spriel 2 siblings, 2 replies; 18+ messages in thread From: Arend van Spriel @ 2012-09-05 9:49 UTC (permalink / raw) To: John W. Linville Cc: Linux Wireless List, Arend van Spriel, stable, Jonathan Nieder, Stanislaw Gruszka, Camaleón, Milan Bouchet-Valat This patch addresses a long standing issue of the driver with the mac80211 .flush() callback. Since implementing the .flush() callback a number of issues have been fixed, but a WARN_ON_ONCE() was still triggered because the flush takes too much time. The flush now makes use of a waitqueue and still has a timeout on which a warning is given. bugzilla: 42840 <https://bugzilla.kernel.org/show_bug.cgi?id=42840> bugzilla@redhat: <https://bugzilla.redhat.com/show_bug.cgi?id=799168> bugzilla@redhat: <https://bugzilla.redhat.com/show_bug.cgi?id=787649> Cc: stable <stable@vger.kernel.org> Cc: Jonathan Nieder <jrnieder@gmail.com> Cc: Stanislaw Gruszka <sgruszka@redhat.com> Cc: Camaleón <noelamac@gmail.com> Cc: Milan Bouchet-Valat <nalimilan@club-internet.fr> Reviewed-by: Pieter-Paul Giesberts <pieterpg@broadcom.com> Reviewed-by: Hante Meuleman <meuleman@broadcom.com> Signed-off-by: Arend van Spriel <arend@broadcom.com> --- .../net/wireless/brcm80211/brcmsmac/mac80211_if.c | 41 +++++++++++++------- .../net/wireless/brcm80211/brcmsmac/mac80211_if.h | 3 +- drivers/net/wireless/brcm80211/brcmsmac/main.c | 21 +++------- drivers/net/wireless/brcm80211/brcmsmac/pub.h | 4 +- 4 files changed, 37 insertions(+), 32 deletions(-) diff --git a/drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c b/drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c index a5edebe..56c419b 100644 --- a/drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c +++ b/drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.c @@ -35,6 +35,7 @@ #include "main.h" #define N_TX_QUEUES 4 /* #tx queues on mac80211<->driver interface */ +#define BRCMS_FLUSH_TIMEOUT 100 /* msec */ /* Flags we support */ #define MAC_FILTERS (FIF_PROMISC_IN_BSS | \ @@ -693,16 +694,35 @@ static void brcms_ops_rfkill_poll(struct ieee80211_hw *hw) wiphy_rfkill_set_hw_state(wl->pub->ieee_hw->wiphy, blocked); } +static bool brcms_tx_flush_completed(struct brcms_info *wl) +{ + bool result; + + spin_lock_bh(&wl->lock); + result = brcms_c_tx_flush_completed(wl->wlc); + spin_unlock_bh(&wl->lock); + return result; +} + static void brcms_ops_flush(struct ieee80211_hw *hw, bool drop) { struct brcms_info *wl = hw->priv; + int ret; no_printk("%s: drop = %s\n", __func__, drop ? "true" : "false"); - /* wait for packet queue and dma fifos to run empty */ - spin_lock_bh(&wl->lock); - brcms_c_wait_for_tx_completion(wl->wlc, drop); - spin_unlock_bh(&wl->lock); + ieee80211_stop_queues(hw); + if (drop) { + spin_lock_bh(&wl->lock); + brcms_c_pktq_flush(wl->wlc); + spin_unlock_bh(&wl->lock); + } + ret = wait_event_timeout(wl->tx_flush_wq, + brcms_tx_flush_completed(wl), + msecs_to_jiffies(BRCMS_FLUSH_TIMEOUT)); + + ieee80211_wake_queues(hw); + WARN_ON(!ret); } static const struct ieee80211_ops brcms_ops = { @@ -757,6 +777,7 @@ void brcms_dpc(unsigned long data) done: spin_unlock_bh(&wl->lock); + wake_up(&wl->tx_flush_wq); } /* @@ -1008,6 +1029,8 @@ static struct brcms_info *brcms_attach(struct bcma_device *pdev) atomic_set(&wl->callbacks, 0); + init_waitqueue_head(&wl->tx_flush_wq); + /* setup the bottom half handler */ tasklet_init(&wl->tasklet, brcms_dpc, (unsigned long) wl); @@ -1594,13 +1617,3 @@ bool brcms_rfkill_set_hw_state(struct brcms_info *wl) spin_lock_bh(&wl->lock); return blocked; } - -/* - * precondition: perimeter lock has been acquired - */ -void brcms_msleep(struct brcms_info *wl, uint ms) -{ - spin_unlock_bh(&wl->lock); - msleep(ms); - spin_lock_bh(&wl->lock); -} diff --git a/drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.h b/drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.h index 9358bd5..947ccac 100644 --- a/drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.h +++ b/drivers/net/wireless/brcm80211/brcmsmac/mac80211_if.h @@ -68,6 +68,8 @@ struct brcms_info { spinlock_t lock; /* per-device perimeter lock */ spinlock_t isr_lock; /* per-device ISR synchronization lock */ + /* tx flush */ + wait_queue_head_t tx_flush_wq; /* timer related fields */ atomic_t callbacks; /* # outstanding callback functions */ @@ -100,7 +102,6 @@ extern struct brcms_timer *brcms_init_timer(struct brcms_info *wl, extern void brcms_free_timer(struct brcms_timer *timer); extern void brcms_add_timer(struct brcms_timer *timer, uint ms, int periodic); extern bool brcms_del_timer(struct brcms_timer *timer); -extern void brcms_msleep(struct brcms_info *wl, uint ms); extern void brcms_dpc(unsigned long data); extern void brcms_timer(struct brcms_timer *t); extern void brcms_fatal_error(struct brcms_info *wl); diff --git a/drivers/net/wireless/brcm80211/brcmsmac/main.c b/drivers/net/wireless/brcm80211/brcmsmac/main.c index 03ca653..ee44126 100644 --- a/drivers/net/wireless/brcm80211/brcmsmac/main.c +++ b/drivers/net/wireless/brcm80211/brcmsmac/main.c @@ -7979,23 +7979,14 @@ int brcms_c_get_curband(struct brcms_c_info *wlc) return wlc->band->bandunit; } -void brcms_c_wait_for_tx_completion(struct brcms_c_info *wlc, bool drop) +void brcms_c_pktq_flush(struct brcms_c_info *wlc) { - int timeout = 20; - - /* flush packet queue when requested */ - if (drop) - brcmu_pktq_flush(&wlc->pkt_queue->q, false, NULL, NULL); - - /* wait for queue and DMA fifos to run dry */ - while (!pktq_empty(&wlc->pkt_queue->q) || brcms_txpktpendtot(wlc) > 0) { - brcms_msleep(wlc->wl, 1); - - if (--timeout == 0) - break; - } + brcmu_pktq_flush(&wlc->pkt_queue->q, false, NULL, NULL); +} - WARN_ON_ONCE(timeout == 0); +bool brcms_c_tx_flush_completed(struct brcms_c_info *wlc) +{ + return pktq_empty(&wlc->pkt_queue->q) && !brcms_txpktpendtot(wlc); } void brcms_c_set_beacon_listen_interval(struct brcms_c_info *wlc, u8 interval) diff --git a/drivers/net/wireless/brcm80211/brcmsmac/pub.h b/drivers/net/wireless/brcm80211/brcmsmac/pub.h index 5855f4f..59caa1c 100644 --- a/drivers/net/wireless/brcm80211/brcmsmac/pub.h +++ b/drivers/net/wireless/brcm80211/brcmsmac/pub.h @@ -350,8 +350,6 @@ extern void brcms_c_associate_upd(struct brcms_c_info *wlc, bool state); extern void brcms_c_scan_start(struct brcms_c_info *wlc); extern void brcms_c_scan_stop(struct brcms_c_info *wlc); extern int brcms_c_get_curband(struct brcms_c_info *wlc); -extern void brcms_c_wait_for_tx_completion(struct brcms_c_info *wlc, - bool drop); extern int brcms_c_set_channel(struct brcms_c_info *wlc, u16 channel); extern int brcms_c_set_rate_limit(struct brcms_c_info *wlc, u16 srl, u16 lrl); extern void brcms_c_get_current_rateset(struct brcms_c_info *wlc, @@ -368,5 +366,7 @@ extern int brcms_c_set_tx_power(struct brcms_c_info *wlc, int txpwr); extern int brcms_c_get_tx_power(struct brcms_c_info *wlc); extern bool brcms_c_check_radio_disabled(struct brcms_c_info *wlc); extern void brcms_c_mute(struct brcms_c_info *wlc, bool on); +extern void brcms_c_pktq_flush(struct brcms_c_info *wlc); +extern bool brcms_c_tx_flush_completed(struct brcms_c_info *wlc); #endif /* _BRCM_PUB_H_ */ -- 1.7.9.5 ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] brcmsmac: rework of mac80211 .flush() callback operation 2012-09-05 9:49 ` [PATCH 2/2] brcmsmac: rework of mac80211 .flush() callback operation Arend van Spriel @ 2012-09-05 10:20 ` Stanislaw Gruszka 2012-09-05 10:37 ` Arend van Spriel 2012-09-05 16:57 ` Seth Forshee 1 sibling, 1 reply; 18+ messages in thread From: Stanislaw Gruszka @ 2012-09-05 10:20 UTC (permalink / raw) To: Arend van Spriel Cc: John W. Linville, Linux Wireless List, stable, Jonathan Nieder, Camaleón, Milan Bouchet-Valat On Wed, Sep 05, 2012 at 11:49:22AM +0200, Arend van Spriel wrote: > + ret = wait_event_timeout(wl->tx_flush_wq, > + brcms_tx_flush_completed(wl), > + msecs_to_jiffies(BRCMS_FLUSH_TIMEOUT)); > + > + ieee80211_wake_queues(hw); > + WARN_ON(!ret); Any particular reason why this WARN_ON is after ieee80211_wake_queues() ? Stanislaw ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] brcmsmac: rework of mac80211 .flush() callback operation 2012-09-05 10:20 ` Stanislaw Gruszka @ 2012-09-05 10:37 ` Arend van Spriel 2012-09-05 11:49 ` Stanislaw Gruszka 0 siblings, 1 reply; 18+ messages in thread From: Arend van Spriel @ 2012-09-05 10:37 UTC (permalink / raw) To: Stanislaw Gruszka Cc: John W. Linville, Linux Wireless List, stable, Jonathan Nieder, Camaleón, Milan Bouchet-Valat On 09/05/2012 12:20 PM, Stanislaw Gruszka wrote: > On Wed, Sep 05, 2012 at 11:49:22AM +0200, Arend van Spriel wrote: >> + ret = wait_event_timeout(wl->tx_flush_wq, >> + brcms_tx_flush_completed(wl), >> + msecs_to_jiffies(BRCMS_FLUSH_TIMEOUT)); >> + >> + ieee80211_wake_queues(hw); >> + WARN_ON(!ret); > Any particular reason why this WARN_ON is after ieee80211_wake_queues() ? > The wait has a timeout so the warning indicates flush did not complete as in the old implementation. Maybe a WARN_ON_ONCE() would be better, but I have not observed the warning yet. Gr. AvS ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] brcmsmac: rework of mac80211 .flush() callback operation 2012-09-05 10:37 ` Arend van Spriel @ 2012-09-05 11:49 ` Stanislaw Gruszka 0 siblings, 0 replies; 18+ messages in thread From: Stanislaw Gruszka @ 2012-09-05 11:49 UTC (permalink / raw) To: Arend van Spriel Cc: John W. Linville, Linux Wireless List, stable, Jonathan Nieder, Camaleón, Milan Bouchet-Valat On Wed, Sep 05, 2012 at 12:37:40PM +0200, Arend van Spriel wrote: > On 09/05/2012 12:20 PM, Stanislaw Gruszka wrote: > >On Wed, Sep 05, 2012 at 11:49:22AM +0200, Arend van Spriel wrote: > >>+ ret = wait_event_timeout(wl->tx_flush_wq, > >>+ brcms_tx_flush_completed(wl), > >>+ msecs_to_jiffies(BRCMS_FLUSH_TIMEOUT)); > >>+ > >>+ ieee80211_wake_queues(hw); > >>+ WARN_ON(!ret); > >Any particular reason why this WARN_ON is after ieee80211_wake_queues() ? > > > > The wait has a timeout so the warning indicates flush did not > complete as in the old implementation. Maybe a WARN_ON_ONCE() would > be better, but I have not observed the warning yet. Yeah, but I rather asked why it is _after_ ieee80211_wake_queues(), not before, just after wait_event_timeout(). Not big deal thought, just if something wrong will happen in ieee80211_wake_queues() order of error prints will be confusing. Stanislaw ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] brcmsmac: rework of mac80211 .flush() callback operation 2012-09-05 9:49 ` [PATCH 2/2] brcmsmac: rework of mac80211 .flush() callback operation Arend van Spriel 2012-09-05 10:20 ` Stanislaw Gruszka @ 2012-09-05 16:57 ` Seth Forshee 2012-09-05 17:33 ` Brad Figg 1 sibling, 1 reply; 18+ messages in thread From: Seth Forshee @ 2012-09-05 16:57 UTC (permalink / raw) To: Arend van Spriel Cc: John W. Linville, Linux Wireless List, stable, Jonathan Nieder, Stanislaw Gruszka, Camaleón, Milan Bouchet-Valat, Brad Figg On Wed, Sep 05, 2012 at 11:49:22AM +0200, Arend van Spriel wrote: > This patch addresses a long standing issue of the driver with the > mac80211 .flush() callback. Since implementing the .flush() callback > a number of issues have been fixed, but a WARN_ON_ONCE() was still > triggered because the flush takes too much time. The flush now > makes use of a waitqueue and still has a timeout on which a warning > is given. Brad Figg and I have been testing this patch with a 3.5 kernel. With the patch we're both still seeing the warning from brcms_ops_flush(), and Brad has also seen the "No where to go" message in brcms_c_prec_enq_head(). But the connection continues to work when this happens, whereas previously we had to reconnect, so this is definitely an improvement. Seth ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] brcmsmac: rework of mac80211 .flush() callback operation 2012-09-05 16:57 ` Seth Forshee @ 2012-09-05 17:33 ` Brad Figg 2012-09-05 19:21 ` Arend van Spriel 0 siblings, 1 reply; 18+ messages in thread From: Brad Figg @ 2012-09-05 17:33 UTC (permalink / raw) To: Seth Forshee Cc: Arend van Spriel, John W. Linville, Linux Wireless List, stable, Jonathan Nieder, Stanislaw Gruszka, Camaleón, Milan Bouchet-Valat On 09/05/2012 09:57 AM, Seth Forshee wrote: > On Wed, Sep 05, 2012 at 11:49:22AM +0200, Arend van Spriel wrote: >> This patch addresses a long standing issue of the driver with the >> mac80211 .flush() callback. Since implementing the .flush() callback >> a number of issues have been fixed, but a WARN_ON_ONCE() was still >> triggered because the flush takes too much time. The flush now >> makes use of a waitqueue and still has a timeout on which a warning >> is given. > > Brad Figg and I have been testing this patch with a 3.5 kernel. With the > patch we're both still seeing the warning from brcms_ops_flush(), and > Brad has also seen the "No where to go" message in > brcms_c_prec_enq_head(). But the connection continues to work when this > happens, whereas previously we had to reconnect, so this is definitely > an improvement. > > Seth > Arend, Though the driver is definitely better, it is still struggling quite a bit as can be seen in the iperf output found at: http://pastebin.ubuntu.com/1187578/ -- Brad Figg brad.figg@canonical.com http://www.canonical.com ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] brcmsmac: rework of mac80211 .flush() callback operation 2012-09-05 17:33 ` Brad Figg @ 2012-09-05 19:21 ` Arend van Spriel 2012-09-05 19:33 ` Seth Forshee 2012-09-05 19:46 ` Brad Figg 0 siblings, 2 replies; 18+ messages in thread From: Arend van Spriel @ 2012-09-05 19:21 UTC (permalink / raw) To: Brad Figg Cc: Seth Forshee, John W. Linville, Linux Wireless List, stable, Jonathan Nieder, Stanislaw Gruszka, Camaleón, Milan Bouchet-Valat On 09/05/2012 07:33 PM, Brad Figg wrote: > On 09/05/2012 09:57 AM, Seth Forshee wrote: >> On Wed, Sep 05, 2012 at 11:49:22AM +0200, Arend van Spriel wrote: >>> This patch addresses a long standing issue of the driver with the >>> mac80211 .flush() callback. Since implementing the .flush() callback >>> a number of issues have been fixed, but a WARN_ON_ONCE() was still >>> triggered because the flush takes too much time. The flush now >>> makes use of a waitqueue and still has a timeout on which a warning >>> is given. >> >> Brad Figg and I have been testing this patch with a 3.5 kernel. With the >> patch we're both still seeing the warning from brcms_ops_flush(), and >> Brad has also seen the "No where to go" message in >> brcms_c_prec_enq_head(). But the connection continues to work when this >> happens, whereas previously we had to reconnect, so this is definitely >> an improvement. >> >> Seth >> > > Arend, > > Though the driver is definitely better, it is still struggling quite a > bit as can be seen in the iperf output found at: > http://pastebin.ubuntu.com/1187578/ > Thanks, Brad I was focusing on the warning in the flush callback. I actually could not reproduce the "nowhere to go" message under my ubuntu test laptop. The root cause of your problem seems the "nowhere to go". Basically, all packet queues in the driver have reached their limit. In that case packets are simply dropped. Could you provide following info: * cpu info * wireless device id * scan data of your AP * kernel configuration If I can download the kernel debian package of your 3.5 kernel from somewhere that would be great. Gr. AvS ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] brcmsmac: rework of mac80211 .flush() callback operation 2012-09-05 19:21 ` Arend van Spriel @ 2012-09-05 19:33 ` Seth Forshee 2012-09-05 20:00 ` Arend van Spriel 2012-09-05 19:46 ` Brad Figg 1 sibling, 1 reply; 18+ messages in thread From: Seth Forshee @ 2012-09-05 19:33 UTC (permalink / raw) To: Arend van Spriel Cc: Brad Figg, John W. Linville, Linux Wireless List, stable, Jonathan Nieder, Stanislaw Gruszka, Camaleón, Milan Bouchet-Valat On Wed, Sep 05, 2012 at 09:21:08PM +0200, Arend van Spriel wrote: > If I can download the kernel debian package of your 3.5 kernel from > somewhere that would be great. You can get the debs for the kernel from http://people.canonical.com/~sforshee/linux-3.5.0-13.14~lp000000v201209051242/. Seth ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] brcmsmac: rework of mac80211 .flush() callback operation 2012-09-05 19:33 ` Seth Forshee @ 2012-09-05 20:00 ` Arend van Spriel 2012-09-05 20:07 ` Seth Forshee 0 siblings, 1 reply; 18+ messages in thread From: Arend van Spriel @ 2012-09-05 20:00 UTC (permalink / raw) To: Seth Forshee Cc: Brad Figg, John W. Linville, Linux Wireless List, stable, Jonathan Nieder, Stanislaw Gruszka, Camaleón, Milan Bouchet-Valat On 09/05/2012 09:33 PM, Seth Forshee wrote: > On Wed, Sep 05, 2012 at 09:21:08PM +0200, Arend van Spriel wrote: >> If I can download the kernel debian package of your 3.5 kernel from >> somewhere that would be great. > > You can get the debs for the kernel from > http://people.canonical.com/~sforshee/linux-3.5.0-13.14~lp000000v201209051242/. > > Seth > > I see the two brcmsmac patches there. Are those applied to this kernel? Gr. AvS ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] brcmsmac: rework of mac80211 .flush() callback operation 2012-09-05 20:00 ` Arend van Spriel @ 2012-09-05 20:07 ` Seth Forshee 0 siblings, 0 replies; 18+ messages in thread From: Seth Forshee @ 2012-09-05 20:07 UTC (permalink / raw) To: Arend van Spriel Cc: Brad Figg, John W. Linville, Linux Wireless List, stable, Jonathan Nieder, Stanislaw Gruszka, Camaleón, Milan Bouchet-Valat On Wed, Sep 05, 2012 at 10:00:55PM +0200, Arend van Spriel wrote: > On 09/05/2012 09:33 PM, Seth Forshee wrote: > >On Wed, Sep 05, 2012 at 09:21:08PM +0200, Arend van Spriel wrote: > >>If I can download the kernel debian package of your 3.5 kernel from > >>somewhere that would be great. > > > >You can get the debs for the kernel from > >http://people.canonical.com/~sforshee/linux-3.5.0-13.14~lp000000v201209051242/. > > > >Seth > > > > > > I see the two brcmsmac patches there. Are those applied to this kernel? Yes, they are applied to that build. Otherwise it is our stock kernel for Ubuntu 12.10. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] brcmsmac: rework of mac80211 .flush() callback operation 2012-09-05 19:21 ` Arend van Spriel 2012-09-05 19:33 ` Seth Forshee @ 2012-09-05 19:46 ` Brad Figg 2012-09-05 19:48 ` Arend van Spriel 1 sibling, 1 reply; 18+ messages in thread From: Brad Figg @ 2012-09-05 19:46 UTC (permalink / raw) To: Arend van Spriel Cc: Seth Forshee, John W. Linville, Linux Wireless List, stable, Jonathan Nieder, Stanislaw Gruszka, Camaleón, Milan Bouchet-Valat On 09/05/2012 12:21 PM, Arend van Spriel wrote: > On 09/05/2012 07:33 PM, Brad Figg wrote: >> On 09/05/2012 09:57 AM, Seth Forshee wrote: >>> On Wed, Sep 05, 2012 at 11:49:22AM +0200, Arend van Spriel wrote: >>>> This patch addresses a long standing issue of the driver with the >>>> mac80211 .flush() callback. Since implementing the .flush() callback >>>> a number of issues have been fixed, but a WARN_ON_ONCE() was still >>>> triggered because the flush takes too much time. The flush now >>>> makes use of a waitqueue and still has a timeout on which a warning >>>> is given. >>> >>> Brad Figg and I have been testing this patch with a 3.5 kernel. With the >>> patch we're both still seeing the warning from brcms_ops_flush(), and >>> Brad has also seen the "No where to go" message in >>> brcms_c_prec_enq_head(). But the connection continues to work when this >>> happens, whereas previously we had to reconnect, so this is definitely >>> an improvement. >>> >>> Seth >>> >> >> Arend, >> >> Though the driver is definitely better, it is still struggling quite a >> bit as can be seen in the iperf output found at: >> http://pastebin.ubuntu.com/1187578/ >> > > Thanks, Brad > > I was focusing on the warning in the flush callback. I actually could not reproduce the "nowhere to go" message under my ubuntu test laptop. > > The root cause of your problem seems the "nowhere to go". Basically, all packet queues in the driver have reached their limit. In that case packets are simply dropped. > > Could you provide following info: > * cpu info > * wireless device id > * scan data of your AP > * kernel configuration > > If I can download the kernel debian package of your 3.5 kernel from somewhere that would be great. > > Gr. AvS > Arend, I just filed https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1046507 which should contain all this information. If there is _anything_ more I can provide to you or test for you do not hesitate to ask. Brad -- Brad Figg brad.figg@canonical.com http://www.canonical.com ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 2/2] brcmsmac: rework of mac80211 .flush() callback operation 2012-09-05 19:46 ` Brad Figg @ 2012-09-05 19:48 ` Arend van Spriel 0 siblings, 0 replies; 18+ messages in thread From: Arend van Spriel @ 2012-09-05 19:48 UTC (permalink / raw) To: Brad Figg Cc: Seth Forshee, John W. Linville, Linux Wireless List, stable, Jonathan Nieder, Stanislaw Gruszka, Camaleón, Milan Bouchet-Valat On 09/05/2012 09:46 PM, Brad Figg wrote: > On 09/05/2012 12:21 PM, Arend van Spriel wrote: >> On 09/05/2012 07:33 PM, Brad Figg wrote: >>> On 09/05/2012 09:57 AM, Seth Forshee wrote: >>>> On Wed, Sep 05, 2012 at 11:49:22AM +0200, Arend van Spriel wrote: >>>>> This patch addresses a long standing issue of the driver with the >>>>> mac80211 .flush() callback. Since implementing the .flush() callback >>>>> a number of issues have been fixed, but a WARN_ON_ONCE() was still >>>>> triggered because the flush takes too much time. The flush now >>>>> makes use of a waitqueue and still has a timeout on which a warning >>>>> is given. >>>> >>>> Brad Figg and I have been testing this patch with a 3.5 kernel. With the >>>> patch we're both still seeing the warning from brcms_ops_flush(), and >>>> Brad has also seen the "No where to go" message in >>>> brcms_c_prec_enq_head(). But the connection continues to work when this >>>> happens, whereas previously we had to reconnect, so this is definitely >>>> an improvement. >>>> >>>> Seth >>>> >>> >>> Arend, >>> >>> Though the driver is definitely better, it is still struggling quite a >>> bit as can be seen in the iperf output found at: >>> http://pastebin.ubuntu.com/1187578/ >>> >> >> Thanks, Brad >> >> I was focusing on the warning in the flush callback. I actually could not reproduce the "nowhere to go" message under my ubuntu test laptop. >> >> The root cause of your problem seems the "nowhere to go". Basically, all packet queues in the driver have reached their limit. In that case packets are simply dropped. >> >> Could you provide following info: >> * cpu info >> * wireless device id >> * scan data of your AP >> * kernel configuration >> >> If I can download the kernel debian package of your 3.5 kernel from somewhere that would be great. >> >> Gr. AvS >> > > Arend, > > I just filed https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1046507 > which should contain all this information. > > If there is _anything_ more I can provide to you or test for you do not > hesitate to ask. I will not (hesitate, that is). Gr. AvS ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/2] brcmsmac: fix for regulatory rules and flush callback 2012-09-05 9:49 [PATCH 0/2] brcmsmac: fix for regulatory rules and flush callback Arend van Spriel 2012-09-05 9:49 ` [PATCH 1/2] brcmsmac: fix mismatch in number of custom regulatory rules Arend van Spriel 2012-09-05 9:49 ` [PATCH 2/2] brcmsmac: rework of mac80211 .flush() callback operation Arend van Spriel @ 2012-09-10 13:22 ` Arend van Spriel 2012-09-10 18:33 ` John W. Linville 2 siblings, 1 reply; 18+ messages in thread From: Arend van Spriel @ 2012-09-10 13:22 UTC (permalink / raw) To: Arend van Spriel; +Cc: John W. Linville, Linux Wireless List, Seth Forshee On 09/05/2012 11:49 AM, Arend van Spriel wrote: > Two fixes intended for the wireless tree. The issue with the .flush() has > been giving me a lot of email from RedHat's bugzilla server. Hope this > patch will silence that ;-) Hi John, I should remove that smile above, but are you planning to take these for v3.6. At least the first patch is a clear bug fix that I would like to have applied. Gr. AvS > > Arend van Spriel (2): > brcmsmac: fix mismatch in number of custom regulatory rules > brcmsmac: rework of mac80211 .flush() callback operation > > drivers/net/wireless/brcm80211/brcmsmac/channel.c | 2 +- > .../net/wireless/brcm80211/brcmsmac/mac80211_if.c | 41 +++++++++++++------- > .../net/wireless/brcm80211/brcmsmac/mac80211_if.h | 3 +- > drivers/net/wireless/brcm80211/brcmsmac/main.c | 21 +++------- > drivers/net/wireless/brcm80211/brcmsmac/pub.h | 4 +- > 5 files changed, 38 insertions(+), 33 deletions(-) > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/2] brcmsmac: fix for regulatory rules and flush callback 2012-09-10 13:22 ` [PATCH 0/2] brcmsmac: fix for regulatory rules and flush callback Arend van Spriel @ 2012-09-10 18:33 ` John W. Linville 2012-09-10 20:41 ` Arend van Spriel 0 siblings, 1 reply; 18+ messages in thread From: John W. Linville @ 2012-09-10 18:33 UTC (permalink / raw) To: Arend van Spriel; +Cc: Linux Wireless List, Seth Forshee On Mon, Sep 10, 2012 at 03:22:30PM +0200, Arend van Spriel wrote: > On 09/05/2012 11:49 AM, Arend van Spriel wrote: > >Two fixes intended for the wireless tree. The issue with the .flush() has > >been giving me a lot of email from RedHat's bugzilla server. Hope this > >patch will silence that ;-) > > Hi John, > > I should remove that smile above, but are you planning to take these > for v3.6. At least the first patch is a clear bug fix that I would > like > to have applied. > > Gr. AvS Arend, I held-off on these because of the continuing discussion... I definitely 'feel your pain' re: Red Hat bugzilla and the second patch. But I'm not sure it meets either the 'small' or 'obvious' tests, so I would prefer to leave it out for now. I'll take the first one now. Thanks, John -- John W. Linville Someday the world will need a hero, and you linville@tuxdriver.com might be all we have. Be ready. ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH 0/2] brcmsmac: fix for regulatory rules and flush callback 2012-09-10 18:33 ` John W. Linville @ 2012-09-10 20:41 ` Arend van Spriel 0 siblings, 0 replies; 18+ messages in thread From: Arend van Spriel @ 2012-09-10 20:41 UTC (permalink / raw) To: John W. Linville; +Cc: Linux Wireless List, Seth Forshee On 09/10/2012 08:33 PM, John W. Linville wrote: > On Mon, Sep 10, 2012 at 03:22:30PM +0200, Arend van Spriel wrote: >> On 09/05/2012 11:49 AM, Arend van Spriel wrote: >>> Two fixes intended for the wireless tree. The issue with the .flush() has >>> been giving me a lot of email from RedHat's bugzilla server. Hope this >>> patch will silence that ;-) >> >> Hi John, >> >> I should remove that smile above, but are you planning to take these >> for v3.6. At least the first patch is a clear bug fix that I would >> like >> to have applied. >> >> Gr. AvS > > Arend, > > I held-off on these because of the continuing discussion... Understood. > I definitely 'feel your pain' re: Red Hat bugzilla and the second > patch. But I'm not sure it meets either the 'small' or 'obvious' > tests, so I would prefer to leave it out for now. I'll take the > first one now. Thanks. I wanted to make sure the first patch got in. While the second improves the runtime behavior, it does not address the root cause (whatever that is) so I agree to keep it out until I hammer it down. > Thanks, > > John > Gr. AvS ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2012-09-10 20:41 UTC | newest] Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-09-05 9:49 [PATCH 0/2] brcmsmac: fix for regulatory rules and flush callback Arend van Spriel 2012-09-05 9:49 ` [PATCH 1/2] brcmsmac: fix mismatch in number of custom regulatory rules Arend van Spriel 2012-09-05 12:40 ` Seth Forshee 2012-09-05 9:49 ` [PATCH 2/2] brcmsmac: rework of mac80211 .flush() callback operation Arend van Spriel 2012-09-05 10:20 ` Stanislaw Gruszka 2012-09-05 10:37 ` Arend van Spriel 2012-09-05 11:49 ` Stanislaw Gruszka 2012-09-05 16:57 ` Seth Forshee 2012-09-05 17:33 ` Brad Figg 2012-09-05 19:21 ` Arend van Spriel 2012-09-05 19:33 ` Seth Forshee 2012-09-05 20:00 ` Arend van Spriel 2012-09-05 20:07 ` Seth Forshee 2012-09-05 19:46 ` Brad Figg 2012-09-05 19:48 ` Arend van Spriel 2012-09-10 13:22 ` [PATCH 0/2] brcmsmac: fix for regulatory rules and flush callback Arend van Spriel 2012-09-10 18:33 ` John W. Linville 2012-09-10 20:41 ` Arend van Spriel
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.