* [PATCH 3/3] mac80211: stop only the queues assigned to the vif during channel switch
@ 2014-06-05 16:26 Emmanuel Grumbach
2014-06-06 5:51 ` Michal Kazior
0 siblings, 1 reply; 9+ messages in thread
From: Emmanuel Grumbach @ 2014-06-05 16:26 UTC (permalink / raw)
To: johannes; +Cc: linux-wireless, Luciano Coelho, Emmanuel Grumbach
From: Luciano Coelho <luciano.coelho@intel.com>
Instead of stopping all the hardware queues during channel switch,
which is especially bad when we have large CSA counts, stop only the
queues that are assigned to the vif that is performing the channel
switch.
Signed-off-by: Luciano Coelho <luciano.coelho@intel.com>
Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
---
net/mac80211/cfg.c | 10 ++++------
net/mac80211/iface.c | 5 ++---
net/mac80211/mlme.c | 20 ++++++++------------
3 files changed, 14 insertions(+), 21 deletions(-)
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index c6fe358..5215abb 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -1142,9 +1142,8 @@ static int ieee80211_stop_ap(struct wiphy *wiphy, struct net_device *dev)
mutex_lock(&local->mtx);
sdata->vif.csa_active = false;
if (!ieee80211_csa_needs_block_tx(local))
- ieee80211_wake_queues_by_reason(&local->hw,
- IEEE80211_MAX_QUEUE_MAP,
- IEEE80211_QUEUE_STOP_REASON_CSA);
+ ieee80211_wake_vif_queues(local, sdata,
+ IEEE80211_QUEUE_STOP_REASON_CSA);
mutex_unlock(&local->mtx);
kfree(sdata->u.ap.next_beacon);
@@ -3146,9 +3145,8 @@ static int __ieee80211_csa_finalize(struct ieee80211_sub_if_data *sdata)
cfg80211_ch_switch_notify(sdata->dev, &sdata->csa_chandef);
if (!ieee80211_csa_needs_block_tx(local))
- ieee80211_wake_queues_by_reason(&local->hw,
- IEEE80211_MAX_QUEUE_MAP,
- IEEE80211_QUEUE_STOP_REASON_CSA);
+ ieee80211_wake_vif_queues(local, sdata,
+ IEEE80211_QUEUE_STOP_REASON_CSA);
return 0;
}
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 9b9d804..c7417db 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -842,9 +842,8 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data *sdata,
mutex_lock(&local->mtx);
sdata->vif.csa_active = false;
if (!ieee80211_csa_needs_block_tx(local))
- ieee80211_wake_queues_by_reason(&local->hw,
- IEEE80211_MAX_QUEUE_MAP,
- IEEE80211_QUEUE_STOP_REASON_CSA);
+ ieee80211_wake_vif_queues(local, sdata,
+ IEEE80211_QUEUE_STOP_REASON_CSA);
mutex_unlock(&local->mtx);
sdata_unlock(sdata);
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index 003d17e..2d3b3a5 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -993,9 +993,8 @@ static void ieee80211_chswitch_work(struct work_struct *work)
sdata->vif.csa_active = false;
/* XXX: wait for a beacon first? */
if (!ieee80211_csa_needs_block_tx(local))
- ieee80211_wake_queues_by_reason(&local->hw,
- IEEE80211_MAX_QUEUE_MAP,
- IEEE80211_QUEUE_STOP_REASON_CSA);
+ ieee80211_wake_vif_queues(local, sdata,
+ IEEE80211_QUEUE_STOP_REASON_CSA);
mutex_unlock(&local->mtx);
ieee80211_bss_info_change_notify(sdata, changed);
@@ -1109,9 +1108,8 @@ ieee80211_sta_process_chanswitch(struct ieee80211_sub_if_data *sdata,
sdata->csa_block_tx = csa_ie.mode;
if (sdata->csa_block_tx)
- ieee80211_stop_queues_by_reason(&local->hw,
- IEEE80211_MAX_QUEUE_MAP,
- IEEE80211_QUEUE_STOP_REASON_CSA);
+ ieee80211_stop_vif_queues(local, sdata,
+ IEEE80211_QUEUE_STOP_REASON_CSA);
mutex_unlock(&local->mtx);
cfg80211_ch_switch_started_notify(sdata->dev, &csa_ie.chandef,
@@ -1830,9 +1828,8 @@ static void ieee80211_set_disassoc(struct ieee80211_sub_if_data *sdata,
sdata->vif.csa_active = false;
if (!ieee80211_csa_needs_block_tx(local))
- ieee80211_wake_queues_by_reason(&local->hw,
- IEEE80211_MAX_QUEUE_MAP,
- IEEE80211_QUEUE_STOP_REASON_CSA);
+ ieee80211_wake_vif_queues(local, sdata,
+ IEEE80211_QUEUE_STOP_REASON_CSA);
mutex_unlock(&local->mtx);
sdata->encrypt_headroom = IEEE80211_ENCRYPT_HEADROOM;
@@ -2078,9 +2075,8 @@ static void __ieee80211_disconnect(struct ieee80211_sub_if_data *sdata)
mutex_lock(&local->mtx);
sdata->vif.csa_active = false;
if (!ieee80211_csa_needs_block_tx(local))
- ieee80211_wake_queues_by_reason(&local->hw,
- IEEE80211_MAX_QUEUE_MAP,
- IEEE80211_QUEUE_STOP_REASON_CSA);
+ ieee80211_wake_vif_queues(local, sdata,
+ IEEE80211_QUEUE_STOP_REASON_CSA);
mutex_unlock(&local->mtx);
cfg80211_tx_mlme_mgmt(sdata->dev, frame_buf,
--
1.9.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] mac80211: stop only the queues assigned to the vif during channel switch
2014-06-05 16:26 [PATCH 3/3] mac80211: stop only the queues assigned to the vif during channel switch Emmanuel Grumbach
@ 2014-06-06 5:51 ` Michal Kazior
2014-06-06 6:31 ` Coelho, Luciano
0 siblings, 1 reply; 9+ messages in thread
From: Michal Kazior @ 2014-06-06 5:51 UTC (permalink / raw)
To: Emmanuel Grumbach; +Cc: Johannes Berg, linux-wireless, Luciano Coelho
On 5 June 2014 18:26, Emmanuel Grumbach <emmanuel.grumbach@intel.com> wrote:
> From: Luciano Coelho <luciano.coelho@intel.com>
>
> Instead of stopping all the hardware queues during channel switch,
> which is especially bad when we have large CSA counts, stop only the
> queues that are assigned to the vif that is performing the channel
> switch.
>
> Signed-off-by: Luciano Coelho <luciano.coelho@intel.com>
> Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
> ---
> net/mac80211/cfg.c | 10 ++++------
> net/mac80211/iface.c | 5 ++---
> net/mac80211/mlme.c | 20 ++++++++------------
> 3 files changed, 14 insertions(+), 21 deletions(-)
>
> diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
> index c6fe358..5215abb 100644
> --- a/net/mac80211/cfg.c
> +++ b/net/mac80211/cfg.c
> @@ -1142,9 +1142,8 @@ static int ieee80211_stop_ap(struct wiphy *wiphy, struct net_device *dev)
> mutex_lock(&local->mtx);
> sdata->vif.csa_active = false;
> if (!ieee80211_csa_needs_block_tx(local))
> - ieee80211_wake_queues_by_reason(&local->hw,
> - IEEE80211_MAX_QUEUE_MAP,
> - IEEE80211_QUEUE_STOP_REASON_CSA);
> + ieee80211_wake_vif_queues(local, sdata,
> + IEEE80211_QUEUE_STOP_REASON_CSA);
> mutex_unlock(&local->mtx);
>
> kfree(sdata->u.ap.next_beacon);
> @@ -3146,9 +3145,8 @@ static int __ieee80211_csa_finalize(struct ieee80211_sub_if_data *sdata)
> cfg80211_ch_switch_notify(sdata->dev, &sdata->csa_chandef);
>
> if (!ieee80211_csa_needs_block_tx(local))
> - ieee80211_wake_queues_by_reason(&local->hw,
> - IEEE80211_MAX_QUEUE_MAP,
> - IEEE80211_QUEUE_STOP_REASON_CSA);
> + ieee80211_wake_vif_queues(local, sdata,
> + IEEE80211_QUEUE_STOP_REASON_CSA);
I don't think this is going to work with the upcomming multi-vif
channel switching.
The ieee80211_csa_needs_block_tx() will become false upon *last*
interface switch completion. This means preceeding interface csa
finalizations won't call ieee80211_wake_vif_queues() so you actually
end up not waking all of the queues when you finish csa.
I think this can be solved with:
if (!ieee80211_csa_needs_block_tx(local))
list_for_each(sdata, &local->interfaces, list)
ieee80211_wake_vif_queues(local, sdata, REASON_CSA);
But then I guess it's just a convoluted way of saying:
if (!ieee80211_csa_needs_block_tx(local))
ieee80211_wake_queues_by_reason(hw, REASON_CSA);
IOW waking should remain as it was while stopping can be done per-vif.
This goes for all instances calling wake_vif_queues() in the patch.
Generally I don't think having wake_vif_queues() is a good idea. It's
really tricky. Sure, you can have a stop_vif_queues() which will
behave as you might expect (i.e. first call stops queues and
overlapping vif-queue mappings are not a concern). But
wake_vif_queues() seems to have little practical use to me without any
kind of per-queue reference counting.
Michał
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 3/3] mac80211: stop only the queues assigned to the vif during channel switch
2014-06-06 5:51 ` Michal Kazior
@ 2014-06-06 6:31 ` Coelho, Luciano
2014-06-06 6:44 ` Michal Kazior
0 siblings, 1 reply; 9+ messages in thread
From: Coelho, Luciano @ 2014-06-06 6:31 UTC (permalink / raw)
To: michal.kazior@tieto.com
Cc: linux-wireless@vger.kernel.org, Grumbach, Emmanuel,
johannes@sipsolutions.net
T24gRnJpLCAyMDE0LTA2LTA2IGF0IDA3OjUxICswMjAwLCBNaWNoYWwgS2F6aW9yIHdyb3RlOg0K
PiBPbiA1IEp1bmUgMjAxNCAxODoyNiwgRW1tYW51ZWwgR3J1bWJhY2ggPGVtbWFudWVsLmdydW1i
YWNoQGludGVsLmNvbT4gd3JvdGU6DQo+ID4gRnJvbTogTHVjaWFubyBDb2VsaG8gPGx1Y2lhbm8u
Y29lbGhvQGludGVsLmNvbT4NCj4gPg0KPiA+IEluc3RlYWQgb2Ygc3RvcHBpbmcgYWxsIHRoZSBo
YXJkd2FyZSBxdWV1ZXMgZHVyaW5nIGNoYW5uZWwgc3dpdGNoLA0KPiA+IHdoaWNoIGlzIGVzcGVj
aWFsbHkgYmFkIHdoZW4gd2UgaGF2ZSBsYXJnZSBDU0EgY291bnRzLCBzdG9wIG9ubHkgdGhlDQo+
ID4gcXVldWVzIHRoYXQgYXJlIGFzc2lnbmVkIHRvIHRoZSB2aWYgdGhhdCBpcyBwZXJmb3JtaW5n
IHRoZSBjaGFubmVsDQo+ID4gc3dpdGNoLg0KPiA+DQo+ID4gU2lnbmVkLW9mZi1ieTogTHVjaWFu
byBDb2VsaG8gPGx1Y2lhbm8uY29lbGhvQGludGVsLmNvbT4NCj4gPiBTaWduZWQtb2ZmLWJ5OiBF
bW1hbnVlbCBHcnVtYmFjaCA8ZW1tYW51ZWwuZ3J1bWJhY2hAaW50ZWwuY29tPg0KPiA+IC0tLQ0K
PiA+ICBuZXQvbWFjODAyMTEvY2ZnLmMgICB8IDEwICsrKystLS0tLS0NCj4gPiAgbmV0L21hYzgw
MjExL2lmYWNlLmMgfCAgNSArKy0tLQ0KPiA+ICBuZXQvbWFjODAyMTEvbWxtZS5jICB8IDIwICsr
KysrKysrLS0tLS0tLS0tLS0tDQo+ID4gIDMgZmlsZXMgY2hhbmdlZCwgMTQgaW5zZXJ0aW9ucygr
KSwgMjEgZGVsZXRpb25zKC0pDQo+ID4NCj4gPiBkaWZmIC0tZ2l0IGEvbmV0L21hYzgwMjExL2Nm
Zy5jIGIvbmV0L21hYzgwMjExL2NmZy5jDQo+ID4gaW5kZXggYzZmZTM1OC4uNTIxNWFiYiAxMDA2
NDQNCj4gPiAtLS0gYS9uZXQvbWFjODAyMTEvY2ZnLmMNCj4gPiArKysgYi9uZXQvbWFjODAyMTEv
Y2ZnLmMNCj4gPiBAQCAtMTE0Miw5ICsxMTQyLDggQEAgc3RhdGljIGludCBpZWVlODAyMTFfc3Rv
cF9hcChzdHJ1Y3Qgd2lwaHkgKndpcGh5LCBzdHJ1Y3QgbmV0X2RldmljZSAqZGV2KQ0KPiA+ICAg
ICAgICAgbXV0ZXhfbG9jaygmbG9jYWwtPm10eCk7DQo+ID4gICAgICAgICBzZGF0YS0+dmlmLmNz
YV9hY3RpdmUgPSBmYWxzZTsNCj4gPiAgICAgICAgIGlmICghaWVlZTgwMjExX2NzYV9uZWVkc19i
bG9ja190eChsb2NhbCkpDQo+ID4gLSAgICAgICAgICAgICAgIGllZWU4MDIxMV93YWtlX3F1ZXVl
c19ieV9yZWFzb24oJmxvY2FsLT5odywNCj4gPiAtICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgSUVFRTgwMjExX01BWF9RVUVVRV9NQVAsDQo+ID4gLSAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgIElFRUU4MDIxMV9RVUVVRV9TVE9QX1JFQVNPTl9DU0Ep
Ow0KPiA+ICsgICAgICAgICAgICAgICBpZWVlODAyMTFfd2FrZV92aWZfcXVldWVzKGxvY2FsLCBz
ZGF0YSwNCj4gPiArICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBJRUVF
ODAyMTFfUVVFVUVfU1RPUF9SRUFTT05fQ1NBKTsNCj4gPiAgICAgICAgIG11dGV4X3VubG9jaygm
bG9jYWwtPm10eCk7DQo+ID4NCj4gPiAgICAgICAgIGtmcmVlKHNkYXRhLT51LmFwLm5leHRfYmVh
Y29uKTsNCj4gPiBAQCAtMzE0Niw5ICszMTQ1LDggQEAgc3RhdGljIGludCBfX2llZWU4MDIxMV9j
c2FfZmluYWxpemUoc3RydWN0IGllZWU4MDIxMV9zdWJfaWZfZGF0YSAqc2RhdGEpDQo+ID4gICAg
ICAgICBjZmc4MDIxMV9jaF9zd2l0Y2hfbm90aWZ5KHNkYXRhLT5kZXYsICZzZGF0YS0+Y3NhX2No
YW5kZWYpOw0KPiA+DQo+ID4gICAgICAgICBpZiAoIWllZWU4MDIxMV9jc2FfbmVlZHNfYmxvY2tf
dHgobG9jYWwpKQ0KPiA+IC0gICAgICAgICAgICAgICBpZWVlODAyMTFfd2FrZV9xdWV1ZXNfYnlf
cmVhc29uKCZsb2NhbC0+aHcsDQo+ID4gLSAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgIElFRUU4MDIxMV9NQVhfUVVFVUVfTUFQLA0KPiA+IC0gICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgICBJRUVFODAyMTFfUVVFVUVfU1RPUF9SRUFTT05fQ1NBKTsNCj4g
PiArICAgICAgICAgICAgICAgaWVlZTgwMjExX3dha2VfdmlmX3F1ZXVlcyhsb2NhbCwgc2RhdGEs
DQo+ID4gKyAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgSUVFRTgwMjEx
X1FVRVVFX1NUT1BfUkVBU09OX0NTQSk7DQo+IA0KPiBJIGRvbid0IHRoaW5rIHRoaXMgaXMgZ29p
bmcgdG8gd29yayB3aXRoIHRoZSB1cGNvbW1pbmcgbXVsdGktdmlmDQo+IGNoYW5uZWwgc3dpdGNo
aW5nLg0KPiANCj4gVGhlIGllZWU4MDIxMV9jc2FfbmVlZHNfYmxvY2tfdHgoKSB3aWxsIGJlY29t
ZSBmYWxzZSB1cG9uICpsYXN0Kg0KPiBpbnRlcmZhY2Ugc3dpdGNoIGNvbXBsZXRpb24uIFRoaXMg
bWVhbnMgcHJlY2VlZGluZyBpbnRlcmZhY2UgY3NhDQo+IGZpbmFsaXphdGlvbnMgd29uJ3QgY2Fs
bCBpZWVlODAyMTFfd2FrZV92aWZfcXVldWVzKCkgc28geW91IGFjdHVhbGx5DQo+IGVuZCB1cCBu
b3Qgd2FraW5nIGFsbCBvZiB0aGUgcXVldWVzIHdoZW4geW91IGZpbmlzaCBjc2EuDQoNCkkgdGhp
bmsgdGhlIHJpZ2h0IHdheSB0byBkbyBpdCB3b3VsZCBiZSB0byB0cmF2ZXJzZSB0aGUgdmlmcyB0
aGF0DQpzd2l0Y2hlZCBhbmQgd2FrZSB1cCB0aGVpciBxdWV1ZXMgd2hlbiB0aGUgc3dpdGNoIGlz
IGZpbmlzaGVkLg0KDQoNCj4gSSB0aGluayB0aGlzIGNhbiBiZSBzb2x2ZWQgd2l0aDoNCj4gDQo+
ICBpZiAoIWllZWU4MDIxMV9jc2FfbmVlZHNfYmxvY2tfdHgobG9jYWwpKQ0KPiAgICBsaXN0X2Zv
cl9lYWNoKHNkYXRhLCAmbG9jYWwtPmludGVyZmFjZXMsIGxpc3QpDQo+ICAgICAgaWVlZTgwMjEx
X3dha2VfdmlmX3F1ZXVlcyhsb2NhbCwgc2RhdGEsIFJFQVNPTl9DU0EpOw0KPiANCj4gQnV0IHRo
ZW4gSSBndWVzcyBpdCdzIGp1c3QgYSBjb252b2x1dGVkIHdheSBvZiBzYXlpbmc6DQo+IA0KPiAg
aWYgKCFpZWVlODAyMTFfY3NhX25lZWRzX2Jsb2NrX3R4KGxvY2FsKSkNCj4gICAgaWVlZTgwMjEx
X3dha2VfcXVldWVzX2J5X3JlYXNvbihodywgUkVBU09OX0NTQSk7DQo+IA0KPiBJT1cgd2FraW5n
IHNob3VsZCByZW1haW4gYXMgaXQgd2FzIHdoaWxlIHN0b3BwaW5nIGNhbiBiZSBkb25lIHBlci12
aWYuDQo+IA0KPiBUaGlzIGdvZXMgZm9yIGFsbCBpbnN0YW5jZXMgY2FsbGluZyB3YWtlX3ZpZl9x
dWV1ZXMoKSBpbiB0aGUgcGF0Y2guDQoNCkFjdHVhbGx5IEkgaGFkIGl0IGxpa2UgdGhpcyBpbiBt
eSBmaXJzdCB2ZXJzaW9uLCBJIGhhZCBvbmx5IGFkZGVkIHRoZQ0Kc3RvcCBwYXJ0LCBsZWF2aW5n
IHRoZSB3YWtlIGFzIGl0IHdhcy4gIEJ1dCB0aGVuIEkgdGhvdWdodCB0aGF0IHdlIHdvdWxkDQpi
ZSByZXN0YXJ0aW5nIHF1ZXVlcyBmcm9tIG90aGVyIHZpZnMgdGhhdCB3ZSBkb24ndCBrbm93IGFi
b3V0LiAgSWYNCmFub3RoZXIgdmlmIGlzIHN3aXRjaGluZyBzZXBhcmF0ZWx5LCB3ZSB3b3VsZCBy
ZXN0YXJ0IGl0cyBxdWV1ZXMgYXQgdGhlDQp3cm9uZyB0aW1lLiAgSSB0aGluayB3ZSBzaG91bGRu
J3QgYXNzdW1lIHRoYXQgYWxsIHZpZnMgYXJlIHBhcnQgb2YgdGhlDQpzYW1lIHN3aXRjaC4NCg0K
DQo+IEdlbmVyYWxseSBJIGRvbid0IHRoaW5rIGhhdmluZyB3YWtlX3ZpZl9xdWV1ZXMoKSBpcyBh
IGdvb2QgaWRlYS4gSXQncw0KPiByZWFsbHkgdHJpY2t5LiBTdXJlLCB5b3UgY2FuIGhhdmUgYSBz
dG9wX3ZpZl9xdWV1ZXMoKSB3aGljaCB3aWxsDQo+IGJlaGF2ZSBhcyB5b3UgbWlnaHQgZXhwZWN0
IChpLmUuIGZpcnN0IGNhbGwgc3RvcHMgcXVldWVzIGFuZA0KPiBvdmVybGFwcGluZyB2aWYtcXVl
dWUgbWFwcGluZ3MgYXJlIG5vdCBhIGNvbmNlcm4pLiBCdXQNCj4gd2FrZV92aWZfcXVldWVzKCkg
c2VlbXMgdG8gaGF2ZSBsaXR0bGUgcHJhY3RpY2FsIHVzZSB0byBtZSB3aXRob3V0IGFueQ0KPiBr
aW5kIG9mIHBlci1xdWV1ZSByZWZlcmVuY2UgY291bnRpbmcuDQoNClllYWgsIHRoaXMgY291bGQg
YmUgYSBwcm9ibGVtIGZvciB2aWZzIHRoYXQgYXJlIHNoYXJpbmcgdGhlIHNhbWUgcXVldWVzLg0K
QnV0IHRoYXQgY2FzZSB3b3VsZCBiZSByZWFsbHkgdHJpY2t5IHdpdGggY2hhbm5lbC1zd2l0Y2gg
YW55d2F5LiBiZWNhdXNlDQppZiB0aGUgY2hhbm5lbCBvZiBvbmUgdmlmIGlzIGNoYW5nZWQgYnV0
IG5vdCB0aGUgb3RoZXIsIHRoZSBxdWV1ZSB3b3VsZA0KcHJvYmFibHkgaGF2ZSB0byBiZSBzcGxp
dC4uLg0KDQotLQ0KTHVjYS4NCg==
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] mac80211: stop only the queues assigned to the vif during channel switch
2014-06-06 6:31 ` Coelho, Luciano
@ 2014-06-06 6:44 ` Michal Kazior
2014-06-06 7:37 ` Coelho, Luciano
0 siblings, 1 reply; 9+ messages in thread
From: Michal Kazior @ 2014-06-06 6:44 UTC (permalink / raw)
To: Coelho, Luciano
Cc: linux-wireless@vger.kernel.org, Grumbach, Emmanuel,
johannes@sipsolutions.net
On 6 June 2014 08:31, Coelho, Luciano <luciano.coelho@intel.com> wrote:
> On Fri, 2014-06-06 at 07:51 +0200, Michal Kazior wrote:
>> On 5 June 2014 18:26, Emmanuel Grumbach <emmanuel.grumbach@intel.com> wrote:
[...]
>> > @@ -3146,9 +3145,8 @@ static int __ieee80211_csa_finalize(struct ieee80211_sub_if_data *sdata)
>> > cfg80211_ch_switch_notify(sdata->dev, &sdata->csa_chandef);
>> >
>> > if (!ieee80211_csa_needs_block_tx(local))
>> > - ieee80211_wake_queues_by_reason(&local->hw,
>> > - IEEE80211_MAX_QUEUE_MAP,
>> > - IEEE80211_QUEUE_STOP_REASON_CSA);
>> > + ieee80211_wake_vif_queues(local, sdata,
>> > + IEEE80211_QUEUE_STOP_REASON_CSA);
>>
>> I don't think this is going to work with the upcomming multi-vif
>> channel switching.
>>
>> The ieee80211_csa_needs_block_tx() will become false upon *last*
>> interface switch completion. This means preceeding interface csa
>> finalizations won't call ieee80211_wake_vif_queues() so you actually
>> end up not waking all of the queues when you finish csa.
>
> I think the right way to do it would be to traverse the vifs that
> switched and wake up their queues when the switch is finished.
>
>
>> I think this can be solved with:
>>
>> if (!ieee80211_csa_needs_block_tx(local))
>> list_for_each(sdata, &local->interfaces, list)
>> ieee80211_wake_vif_queues(local, sdata, REASON_CSA);
>>
>> But then I guess it's just a convoluted way of saying:
>>
>> if (!ieee80211_csa_needs_block_tx(local))
>> ieee80211_wake_queues_by_reason(hw, REASON_CSA);
>>
>> IOW waking should remain as it was while stopping can be done per-vif.
>>
>> This goes for all instances calling wake_vif_queues() in the patch.
>
> Actually I had it like this in my first version, I had only added the
> stop part, leaving the wake as it was. But then I thought that we would
> be restarting queues from other vifs that we don't know about. If
> another vif is switching separately, we would restart its queues at the
> wrong time. I think we shouldn't assume that all vifs are part of the
> same switch.
Well, not really.
There is no "vif switching separately" when it comes to block_tx. The
condition checks if ANY interface still needs queues to be locked for
the *REASON_CSA*. If the condition is false (i.e. "no interface needs
block_tx") then you are safe to wake *all* queues for *REASON_CSA* on
all interfaces because channel switches that don't need block_tx don't
stop their queues for REASON_CSA in the first place.
You could argue current queue locking for CSA is not efficient and
you'd be right. It was a simplification I came up with to get things
at least work correctly not efficiently.
>> Generally I don't think having wake_vif_queues() is a good idea. It's
>> really tricky. Sure, you can have a stop_vif_queues() which will
>> behave as you might expect (i.e. first call stops queues and
>> overlapping vif-queue mappings are not a concern). But
>> wake_vif_queues() seems to have little practical use to me without any
>> kind of per-queue reference counting.
>
> Yeah, this could be a problem for vifs that are sharing the same queues.
> But that case would be really tricky with channel-switch anyway. because
> if the channel of one vif is changed but not the other, the queue would
> probably have to be split...
Hmm. That's a very good point. Can we even re-map this during the switch now?
If we can't then doesn't it imply that if there are shared vif-queue
mappings then channel switching should be limited/prevented? I.e. it
might be okay if you have 2 vifs sharing a queue to switch to the
exact same channel but it won't work if you try to switch them to
separate channels. Single-channel devices won't have to worry about it
(single-channel combination cases for that matter).
Michał
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] mac80211: stop only the queues assigned to the vif during channel switch
2014-06-06 6:44 ` Michal Kazior
@ 2014-06-06 7:37 ` Coelho, Luciano
2014-06-06 7:56 ` Michal Kazior
0 siblings, 1 reply; 9+ messages in thread
From: Coelho, Luciano @ 2014-06-06 7:37 UTC (permalink / raw)
To: michal.kazior@tieto.com
Cc: linux-wireless@vger.kernel.org, Grumbach, Emmanuel,
johannes@sipsolutions.net
T24gRnJpLCAyMDE0LTA2LTA2IGF0IDA4OjQ0ICswMjAwLCBNaWNoYWwgS2F6aW9yIHdyb3RlOg0K
PiBPbiA2IEp1bmUgMjAxNCAwODozMSwgQ29lbGhvLCBMdWNpYW5vIDxsdWNpYW5vLmNvZWxob0Bp
bnRlbC5jb20+IHdyb3RlOg0KPiA+IE9uIEZyaSwgMjAxNC0wNi0wNiBhdCAwNzo1MSArMDIwMCwg
TWljaGFsIEthemlvciB3cm90ZToNCj4gPj4gT24gNSBKdW5lIDIwMTQgMTg6MjYsIEVtbWFudWVs
IEdydW1iYWNoIDxlbW1hbnVlbC5ncnVtYmFjaEBpbnRlbC5jb20+IHdyb3RlOg0KPiBbLi4uXQ0K
PiA+PiA+IEBAIC0zMTQ2LDkgKzMxNDUsOCBAQCBzdGF0aWMgaW50IF9faWVlZTgwMjExX2NzYV9m
aW5hbGl6ZShzdHJ1Y3QgaWVlZTgwMjExX3N1Yl9pZl9kYXRhICpzZGF0YSkNCj4gPj4gPiAgICAg
ICAgIGNmZzgwMjExX2NoX3N3aXRjaF9ub3RpZnkoc2RhdGEtPmRldiwgJnNkYXRhLT5jc2FfY2hh
bmRlZik7DQo+ID4+ID4NCj4gPj4gPiAgICAgICAgIGlmICghaWVlZTgwMjExX2NzYV9uZWVkc19i
bG9ja190eChsb2NhbCkpDQo+ID4+ID4gLSAgICAgICAgICAgICAgIGllZWU4MDIxMV93YWtlX3F1
ZXVlc19ieV9yZWFzb24oJmxvY2FsLT5odywNCj4gPj4gPiAtICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgICAgSUVFRTgwMjExX01BWF9RVUVVRV9NQVAsDQo+ID4+ID4gLSAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIElFRUU4MDIxMV9RVUVVRV9TVE9QX1JF
QVNPTl9DU0EpOw0KPiA+PiA+ICsgICAgICAgICAgICAgICBpZWVlODAyMTFfd2FrZV92aWZfcXVl
dWVzKGxvY2FsLCBzZGF0YSwNCj4gPj4gPiArICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICBJRUVFODAyMTFfUVVFVUVfU1RPUF9SRUFTT05fQ1NBKTsNCj4gPj4NCj4gPj4g
SSBkb24ndCB0aGluayB0aGlzIGlzIGdvaW5nIHRvIHdvcmsgd2l0aCB0aGUgdXBjb21taW5nIG11
bHRpLXZpZg0KPiA+PiBjaGFubmVsIHN3aXRjaGluZy4NCj4gPj4NCj4gPj4gVGhlIGllZWU4MDIx
MV9jc2FfbmVlZHNfYmxvY2tfdHgoKSB3aWxsIGJlY29tZSBmYWxzZSB1cG9uICpsYXN0Kg0KPiA+
PiBpbnRlcmZhY2Ugc3dpdGNoIGNvbXBsZXRpb24uIFRoaXMgbWVhbnMgcHJlY2VlZGluZyBpbnRl
cmZhY2UgY3NhDQo+ID4+IGZpbmFsaXphdGlvbnMgd29uJ3QgY2FsbCBpZWVlODAyMTFfd2FrZV92
aWZfcXVldWVzKCkgc28geW91IGFjdHVhbGx5DQo+ID4+IGVuZCB1cCBub3Qgd2FraW5nIGFsbCBv
ZiB0aGUgcXVldWVzIHdoZW4geW91IGZpbmlzaCBjc2EuDQo+ID4NCj4gPiBJIHRoaW5rIHRoZSBy
aWdodCB3YXkgdG8gZG8gaXQgd291bGQgYmUgdG8gdHJhdmVyc2UgdGhlIHZpZnMgdGhhdA0KPiA+
IHN3aXRjaGVkIGFuZCB3YWtlIHVwIHRoZWlyIHF1ZXVlcyB3aGVuIHRoZSBzd2l0Y2ggaXMgZmlu
aXNoZWQuDQo+ID4NCj4gPg0KPiA+PiBJIHRoaW5rIHRoaXMgY2FuIGJlIHNvbHZlZCB3aXRoOg0K
PiA+Pg0KPiA+PiAgaWYgKCFpZWVlODAyMTFfY3NhX25lZWRzX2Jsb2NrX3R4KGxvY2FsKSkNCj4g
Pj4gICAgbGlzdF9mb3JfZWFjaChzZGF0YSwgJmxvY2FsLT5pbnRlcmZhY2VzLCBsaXN0KQ0KPiA+
PiAgICAgIGllZWU4MDIxMV93YWtlX3ZpZl9xdWV1ZXMobG9jYWwsIHNkYXRhLCBSRUFTT05fQ1NB
KTsNCj4gPj4NCj4gPj4gQnV0IHRoZW4gSSBndWVzcyBpdCdzIGp1c3QgYSBjb252b2x1dGVkIHdh
eSBvZiBzYXlpbmc6DQo+ID4+DQo+ID4+ICBpZiAoIWllZWU4MDIxMV9jc2FfbmVlZHNfYmxvY2tf
dHgobG9jYWwpKQ0KPiA+PiAgICBpZWVlODAyMTFfd2FrZV9xdWV1ZXNfYnlfcmVhc29uKGh3LCBS
RUFTT05fQ1NBKTsNCj4gPj4NCj4gPj4gSU9XIHdha2luZyBzaG91bGQgcmVtYWluIGFzIGl0IHdh
cyB3aGlsZSBzdG9wcGluZyBjYW4gYmUgZG9uZSBwZXItdmlmLg0KPiA+Pg0KPiA+PiBUaGlzIGdv
ZXMgZm9yIGFsbCBpbnN0YW5jZXMgY2FsbGluZyB3YWtlX3ZpZl9xdWV1ZXMoKSBpbiB0aGUgcGF0
Y2guDQo+ID4NCj4gPiBBY3R1YWxseSBJIGhhZCBpdCBsaWtlIHRoaXMgaW4gbXkgZmlyc3QgdmVy
c2lvbiwgSSBoYWQgb25seSBhZGRlZCB0aGUNCj4gPiBzdG9wIHBhcnQsIGxlYXZpbmcgdGhlIHdh
a2UgYXMgaXQgd2FzLiAgQnV0IHRoZW4gSSB0aG91Z2h0IHRoYXQgd2Ugd291bGQNCj4gPiBiZSBy
ZXN0YXJ0aW5nIHF1ZXVlcyBmcm9tIG90aGVyIHZpZnMgdGhhdCB3ZSBkb24ndCBrbm93IGFib3V0
LiAgSWYNCj4gPiBhbm90aGVyIHZpZiBpcyBzd2l0Y2hpbmcgc2VwYXJhdGVseSwgd2Ugd291bGQg
cmVzdGFydCBpdHMgcXVldWVzIGF0IHRoZQ0KPiA+IHdyb25nIHRpbWUuICBJIHRoaW5rIHdlIHNo
b3VsZG4ndCBhc3N1bWUgdGhhdCBhbGwgdmlmcyBhcmUgcGFydCBvZiB0aGUNCj4gPiBzYW1lIHN3
aXRjaC4NCj4gDQo+IFdlbGwsIG5vdCByZWFsbHkuDQo+IA0KPiBUaGVyZSBpcyBubyAidmlmIHN3
aXRjaGluZyBzZXBhcmF0ZWx5IiB3aGVuIGl0IGNvbWVzIHRvIGJsb2NrX3R4LiBUaGUNCj4gY29u
ZGl0aW9uIGNoZWNrcyBpZiBBTlkgaW50ZXJmYWNlIHN0aWxsIG5lZWRzIHF1ZXVlcyB0byBiZSBs
b2NrZWQgZm9yDQo+IHRoZSAqUkVBU09OX0NTQSouIElmIHRoZSBjb25kaXRpb24gaXMgZmFsc2Ug
KGkuZS4gIm5vIGludGVyZmFjZSBuZWVkcw0KPiBibG9ja190eCIpIHRoZW4geW91IGFyZSBzYWZl
IHRvIHdha2UgKmFsbCogcXVldWVzIGZvciAqUkVBU09OX0NTQSogb24NCj4gYWxsIGludGVyZmFj
ZXMgYmVjYXVzZSBjaGFubmVsIHN3aXRjaGVzIHRoYXQgZG9uJ3QgbmVlZCBibG9ja190eCBkb24n
dA0KPiBzdG9wIHRoZWlyIHF1ZXVlcyBmb3IgUkVBU09OX0NTQSBpbiB0aGUgZmlyc3QgcGxhY2Uu
DQoNClJpZ2h0LCBhbmQgdGhpcyB1c2VkIHRvIG1ha2Ugc2Vuc2Ugd2hlbiB3ZSB3ZXJlIHN0b3Bw
aW5nICphbGwqIGh3X3F1ZXVlcw0Kd2hlbiB3ZSBoYWQgYmxvY2tfdHguDQoNCk1heWJlIHdlIHNo
b3VsZCBtYWtlIGEgbmV3IGZ1bmN0aW9uIHRoYXQgcmV0dXJucyBhbGwgdGhlIHF1ZXVlcyB0aGF0
IG91cg0KdmlmIHdhcyBibG9ja2luZyBidXQgdGhhdCBhcmUgbm90IGJsb2NrZWQgYnkgYW55IG90
aGVyIHZpZnMuICBNYXliZQ0Kc29tZXRoaW5nIGxpa2UgdGhpczoNCg0Kc3RhdGljIHUzMiBpZWVl
ODAyMTFfdmlmX3F1ZXVlc190b193YWtlKHN0cnVjdCBpZWVlODAyMTFfbG9jYWwgKmxvY2FsLA0K
CQkJCQlzdHJ1Y3QgaWVlZTgwMjExX3N1Yl9pZl9kYXRhICpzZGF0YSwNCgkJCQkJZW51bSBxdWV1
ZV9zdG9wX3JlYXNvbiByZWFzb24pDQp7DQoJc3RydWN0IGllZWU4MDIxMV9zdWJfaWZfZGF0YSAq
c2RhdGFfaXRlcjsNCgl1bnNpZ25lZCBsb25nIHF1ZXVlcywgcXVldWVzX2l0ZXIsIHE7DQoJaW50
IGk7DQoNCglsb2NrZGVwX2Fzc2VydF9oZWxkKCZsb2NhbC0+bXR4KTsNCg0KCXF1ZXVlcyA9IGll
ZWU4MDIxMV9nZXRfdmlmX3F1ZXVlcyhsb2NhbCwgc2RhdGEpOw0KDQoJcmN1X3JlYWRfbG9jaygp
Ow0KCWxpc3RfZm9yX2VhY2hfZW50cnlfcmN1KHNkYXRhX2l0ZXIsICZsb2NhbC0+aW50ZXJmYWNl
cywgbGlzdCkgew0KCQlpZiAoIWllZWU4MDIxMV9zZGF0YV9ydW5uaW5nKHNkYXRhX2l0ZXIpKQ0K
CQkJY29udGludWU7DQoNCgkJcXVldWVzX2l0ZXIgPSBpZWVlODAyMTFfZ2V0X3ZpZl9xdWV1ZXMo
bG9jYWwsIHNkYXRhX2l0ZXIpOw0KCQlmb3JfZWFjaF9zZXRfYml0KGksICZxdWV1ZXNfaXRlciwg
bG9jYWwtPmh3LnF1ZXVlcykgew0KCQkJcSA9IHNkYXRhLT52aWYuaHdfcXVldWVbaV07DQoNCgkJ
CWlmICh0ZXN0X2JpdChyZWFzb24sICZsb2NhbC0+cXVldWVfc3RvcF9yZWFzb25zW3FdKSkNCgkJ
CQlxdWV1ZXMgJj0gfnE7DQoJCX0NCgl9DQoJcmN1X3JlYWRfdW5sb2NrKCk7DQoNCglyZXR1cm4g
cXVldWVzOw0KfQ0KDQp2b2lkIGllZWU4MDIxMV93YWtlX3ZpZl9xdWV1ZXMoc3RydWN0IGllZWU4
MDIxMV9sb2NhbCAqbG9jYWwsDQoJCQkgICAgICAgc3RydWN0IGllZWU4MDIxMV9zdWJfaWZfZGF0
YSAqc2RhdGEsDQoJCQkgICAgICAgZW51bSBxdWV1ZV9zdG9wX3JlYXNvbiByZWFzb24pDQp7DQoJ
dW5zaWduZWQgaW50IHF1ZXVlcyA9IGllZWU4MDIxMV92aWZfcXVldWVzX3RvX3dha2UobG9jYWws
IHNkYXRhLCByZWFzb24pOw0KDQoJaWVlZTgwMjExX3dha2VfcXVldWVzX2J5X3JlYXNvbigmbG9j
YWwtPmh3LCBxdWV1ZXMsIHJlYXNvbik7DQp9DQoNCg0KDQo+IFlvdSBjb3VsZCBhcmd1ZSBjdXJy
ZW50IHF1ZXVlIGxvY2tpbmcgZm9yIENTQSBpcyBub3QgZWZmaWNpZW50IGFuZA0KPiB5b3UnZCBi
ZSByaWdodC4gSXQgd2FzIGEgc2ltcGxpZmljYXRpb24gSSBjYW1lIHVwIHdpdGggdG8gZ2V0IHRo
aW5ncw0KPiBhdCBsZWFzdCB3b3JrIGNvcnJlY3RseSBub3QgZWZmaWNpZW50bHkuDQo+IA0KPiAN
Cj4gPj4gR2VuZXJhbGx5IEkgZG9uJ3QgdGhpbmsgaGF2aW5nIHdha2VfdmlmX3F1ZXVlcygpIGlz
IGEgZ29vZCBpZGVhLiBJdCdzDQo+ID4+IHJlYWxseSB0cmlja3kuIFN1cmUsIHlvdSBjYW4gaGF2
ZSBhIHN0b3BfdmlmX3F1ZXVlcygpIHdoaWNoIHdpbGwNCj4gPj4gYmVoYXZlIGFzIHlvdSBtaWdo
dCBleHBlY3QgKGkuZS4gZmlyc3QgY2FsbCBzdG9wcyBxdWV1ZXMgYW5kDQo+ID4+IG92ZXJsYXBw
aW5nIHZpZi1xdWV1ZSBtYXBwaW5ncyBhcmUgbm90IGEgY29uY2VybikuIEJ1dA0KPiA+PiB3YWtl
X3ZpZl9xdWV1ZXMoKSBzZWVtcyB0byBoYXZlIGxpdHRsZSBwcmFjdGljYWwgdXNlIHRvIG1lIHdp
dGhvdXQgYW55DQo+ID4+IGtpbmQgb2YgcGVyLXF1ZXVlIHJlZmVyZW5jZSBjb3VudGluZy4NCj4g
Pg0KPiA+IFllYWgsIHRoaXMgY291bGQgYmUgYSBwcm9ibGVtIGZvciB2aWZzIHRoYXQgYXJlIHNo
YXJpbmcgdGhlIHNhbWUgcXVldWVzLg0KPiA+IEJ1dCB0aGF0IGNhc2Ugd291bGQgYmUgcmVhbGx5
IHRyaWNreSB3aXRoIGNoYW5uZWwtc3dpdGNoIGFueXdheS4gYmVjYXVzZQ0KPiA+IGlmIHRoZSBj
aGFubmVsIG9mIG9uZSB2aWYgaXMgY2hhbmdlZCBidXQgbm90IHRoZSBvdGhlciwgdGhlIHF1ZXVl
IHdvdWxkDQo+ID4gcHJvYmFibHkgaGF2ZSB0byBiZSBzcGxpdC4uLg0KPiANCj4gSG1tLiBUaGF0
J3MgYSB2ZXJ5IGdvb2QgcG9pbnQuIENhbiB3ZSBldmVuIHJlLW1hcCB0aGlzIGR1cmluZyB0aGUg
c3dpdGNoIG5vdz8NCj4gDQo+IElmIHdlIGNhbid0IHRoZW4gZG9lc24ndCBpdCBpbXBseSB0aGF0
IGlmIHRoZXJlIGFyZSBzaGFyZWQgdmlmLXF1ZXVlDQo+IG1hcHBpbmdzIHRoZW4gY2hhbm5lbCBz
d2l0Y2hpbmcgc2hvdWxkIGJlIGxpbWl0ZWQvcHJldmVudGVkPyBJLmUuIGl0DQo+IG1pZ2h0IGJl
IG9rYXkgaWYgeW91IGhhdmUgMiB2aWZzIHNoYXJpbmcgYSBxdWV1ZSB0byBzd2l0Y2ggdG8gdGhl
DQo+IGV4YWN0IHNhbWUgY2hhbm5lbCBidXQgaXQgd29uJ3Qgd29yayBpZiB5b3UgdHJ5IHRvIHN3
aXRjaCB0aGVtIHRvDQo+IHNlcGFyYXRlIGNoYW5uZWxzLiBTaW5nbGUtY2hhbm5lbCBkZXZpY2Vz
IHdvbid0IGhhdmUgdG8gd29ycnkgYWJvdXQgaXQNCj4gKHNpbmdsZS1jaGFubmVsIGNvbWJpbmF0
aW9uIGNhc2VzIGZvciB0aGF0IG1hdHRlcikuDQoNClllYWgsIGl0IHNob3VsZCBwcm9iYWJseSBi
ZSBwcmV2ZW50ZWQgZm9yIHN1Y2ggZHJpdmVycy4gIEJ1dCB0aGVuLCBhdA0Kd2hhdCBwb2ludD8g
UHJldmVudCB0aGUgY2hhbm5lbCBzd2l0Y2ggdG8gb2NjdXIgY29tcGxldGVseSBpZiBhbm90aGVy
DQp2aWYgc2hhcmVzIHRoZSBzYW1lIHF1ZXVlPyBPciBmYWlsIHRoZSBzd2l0Y2ggaWYgdGhlIG90
aGVyIHZpZiBkb2Vzbid0DQpqb2luIHRoZSBzd2l0Y2g/DQoNCi0tDQpMdWNhLg0K
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] mac80211: stop only the queues assigned to the vif during channel switch
2014-06-06 7:37 ` Coelho, Luciano
@ 2014-06-06 7:56 ` Michal Kazior
2014-06-06 8:04 ` Coelho, Luciano
0 siblings, 1 reply; 9+ messages in thread
From: Michal Kazior @ 2014-06-06 7:56 UTC (permalink / raw)
To: Coelho, Luciano
Cc: linux-wireless@vger.kernel.org, Grumbach, Emmanuel,
johannes@sipsolutions.net
On 6 June 2014 09:37, Coelho, Luciano <luciano.coelho@intel.com> wrote:
> On Fri, 2014-06-06 at 08:44 +0200, Michal Kazior wrote:
>> On 6 June 2014 08:31, Coelho, Luciano <luciano.coelho@intel.com> wrote:
>> > On Fri, 2014-06-06 at 07:51 +0200, Michal Kazior wrote:
>> >> On 5 June 2014 18:26, Emmanuel Grumbach <emmanuel.grumbach@intel.com> wrote:
>> [...]
>> >> > @@ -3146,9 +3145,8 @@ static int __ieee80211_csa_finalize(struct ieee80211_sub_if_data *sdata)
>> >> > cfg80211_ch_switch_notify(sdata->dev, &sdata->csa_chandef);
>> >> >
>> >> > if (!ieee80211_csa_needs_block_tx(local))
>> >> > - ieee80211_wake_queues_by_reason(&local->hw,
>> >> > - IEEE80211_MAX_QUEUE_MAP,
>> >> > - IEEE80211_QUEUE_STOP_REASON_CSA);
>> >> > + ieee80211_wake_vif_queues(local, sdata,
>> >> > + IEEE80211_QUEUE_STOP_REASON_CSA);
>> >>
>> >> I don't think this is going to work with the upcomming multi-vif
>> >> channel switching.
>> >>
>> >> The ieee80211_csa_needs_block_tx() will become false upon *last*
>> >> interface switch completion. This means preceeding interface csa
>> >> finalizations won't call ieee80211_wake_vif_queues() so you actually
>> >> end up not waking all of the queues when you finish csa.
>> >
>> > I think the right way to do it would be to traverse the vifs that
>> > switched and wake up their queues when the switch is finished.
>> >
>> >
>> >> I think this can be solved with:
>> >>
>> >> if (!ieee80211_csa_needs_block_tx(local))
>> >> list_for_each(sdata, &local->interfaces, list)
>> >> ieee80211_wake_vif_queues(local, sdata, REASON_CSA);
>> >>
>> >> But then I guess it's just a convoluted way of saying:
>> >>
>> >> if (!ieee80211_csa_needs_block_tx(local))
>> >> ieee80211_wake_queues_by_reason(hw, REASON_CSA);
>> >>
>> >> IOW waking should remain as it was while stopping can be done per-vif.
>> >>
>> >> This goes for all instances calling wake_vif_queues() in the patch.
>> >
>> > Actually I had it like this in my first version, I had only added the
>> > stop part, leaving the wake as it was. But then I thought that we would
>> > be restarting queues from other vifs that we don't know about. If
>> > another vif is switching separately, we would restart its queues at the
>> > wrong time. I think we shouldn't assume that all vifs are part of the
>> > same switch.
>>
>> Well, not really.
>>
>> There is no "vif switching separately" when it comes to block_tx. The
>> condition checks if ANY interface still needs queues to be locked for
>> the *REASON_CSA*. If the condition is false (i.e. "no interface needs
>> block_tx") then you are safe to wake *all* queues for *REASON_CSA* on
>> all interfaces because channel switches that don't need block_tx don't
>> stop their queues for REASON_CSA in the first place.
>
> Right, and this used to make sense when we were stopping *all* hw_queues
> when we had block_tx.
>
> Maybe we should make a new function that returns all the queues that our
> vif was blocking but that are not blocked by any other vifs. Maybe
> something like this:
>
> static u32 ieee80211_vif_queues_to_wake(struct ieee80211_local *local,
> struct ieee80211_sub_if_data *sdata,
> enum queue_stop_reason reason)
> {
> struct ieee80211_sub_if_data *sdata_iter;
> unsigned long queues, queues_iter, q;
> int i;
>
> lockdep_assert_held(&local->mtx);
>
> queues = ieee80211_get_vif_queues(local, sdata);
>
> rcu_read_lock();
> list_for_each_entry_rcu(sdata_iter, &local->interfaces, list) {
> if (!ieee80211_sdata_running(sdata_iter))
> continue;
>
> queues_iter = ieee80211_get_vif_queues(local, sdata_iter);
> for_each_set_bit(i, &queues_iter, local->hw.queues) {
> q = sdata->vif.hw_queue[i];
>
> if (test_bit(reason, &local->queue_stop_reasons[q]))
> queues &= ~q;
> }
> }
> rcu_read_unlock();
>
> return queues;
> }
>
> void ieee80211_wake_vif_queues(struct ieee80211_local *local,
> struct ieee80211_sub_if_data *sdata,
> enum queue_stop_reason reason)
> {
> unsigned int queues = ieee80211_vif_queues_to_wake(local, sdata, reason);
>
> ieee80211_wake_queues_by_reason(&local->hw, queues, reason);
> }
Once you set local->queue_stop_reasons[] for an overlapping queue you
won't be able to wake it with ieee80211_wake_vif_queues() alone.
You'll need to call ieee80211_wake_queues() to do that. Is that what
you intended?
>> You could argue current queue locking for CSA is not efficient and
>> you'd be right. It was a simplification I came up with to get things
>> at least work correctly not efficiently.
>>
>>
>> >> Generally I don't think having wake_vif_queues() is a good idea. It's
>> >> really tricky. Sure, you can have a stop_vif_queues() which will
>> >> behave as you might expect (i.e. first call stops queues and
>> >> overlapping vif-queue mappings are not a concern). But
>> >> wake_vif_queues() seems to have little practical use to me without any
>> >> kind of per-queue reference counting.
>> >
>> > Yeah, this could be a problem for vifs that are sharing the same queues.
>> > But that case would be really tricky with channel-switch anyway. because
>> > if the channel of one vif is changed but not the other, the queue would
>> > probably have to be split...
>>
>> Hmm. That's a very good point. Can we even re-map this during the switch now?
>>
>> If we can't then doesn't it imply that if there are shared vif-queue
>> mappings then channel switching should be limited/prevented? I.e. it
>> might be okay if you have 2 vifs sharing a queue to switch to the
>> exact same channel but it won't work if you try to switch them to
>> separate channels. Single-channel devices won't have to worry about it
>> (single-channel combination cases for that matter).
>
> Yeah, it should probably be prevented for such drivers. But then, at
> what point? Prevent the channel switch to occur completely if another
> vif shares the same queue? Or fail the switch if the other vif doesn't
> join the switch?
That's a good question too. I would prefer to leave it until the last
second, e.g. give the opportunity to re-map in
drv_switch_vif_chanctx() maybe so driver can fail if it's unable to
comply?
Michał
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 3/3] mac80211: stop only the queues assigned to the vif during channel switch
2014-06-06 7:56 ` Michal Kazior
@ 2014-06-06 8:04 ` Coelho, Luciano
2014-06-06 8:12 ` Michal Kazior
0 siblings, 1 reply; 9+ messages in thread
From: Coelho, Luciano @ 2014-06-06 8:04 UTC (permalink / raw)
To: michal.kazior@tieto.com
Cc: linux-wireless@vger.kernel.org, Grumbach, Emmanuel,
johannes@sipsolutions.net
T24gRnJpLCAyMDE0LTA2LTA2IGF0IDA5OjU2ICswMjAwLCBNaWNoYWwgS2F6aW9yIHdyb3RlOg0K
PiBPbiA2IEp1bmUgMjAxNCAwOTozNywgQ29lbGhvLCBMdWNpYW5vIDxsdWNpYW5vLmNvZWxob0Bp
bnRlbC5jb20+IHdyb3RlOg0KPiA+IE9uIEZyaSwgMjAxNC0wNi0wNiBhdCAwODo0NCArMDIwMCwg
TWljaGFsIEthemlvciB3cm90ZToNCj4gPj4gT24gNiBKdW5lIDIwMTQgMDg6MzEsIENvZWxobywg
THVjaWFubyA8bHVjaWFuby5jb2VsaG9AaW50ZWwuY29tPiB3cm90ZToNCj4gPj4gPiBPbiBGcmks
IDIwMTQtMDYtMDYgYXQgMDc6NTEgKzAyMDAsIE1pY2hhbCBLYXppb3Igd3JvdGU6DQo+ID4+ID4+
IE9uIDUgSnVuZSAyMDE0IDE4OjI2LCBFbW1hbnVlbCBHcnVtYmFjaCA8ZW1tYW51ZWwuZ3J1bWJh
Y2hAaW50ZWwuY29tPiB3cm90ZToNCj4gPj4gWy4uLl0NCj4gPj4gPj4gPiBAQCAtMzE0Niw5ICsz
MTQ1LDggQEAgc3RhdGljIGludCBfX2llZWU4MDIxMV9jc2FfZmluYWxpemUoc3RydWN0IGllZWU4
MDIxMV9zdWJfaWZfZGF0YSAqc2RhdGEpDQo+ID4+ID4+ID4gICAgICAgICBjZmc4MDIxMV9jaF9z
d2l0Y2hfbm90aWZ5KHNkYXRhLT5kZXYsICZzZGF0YS0+Y3NhX2NoYW5kZWYpOw0KPiA+PiA+PiA+
DQo+ID4+ID4+ID4gICAgICAgICBpZiAoIWllZWU4MDIxMV9jc2FfbmVlZHNfYmxvY2tfdHgobG9j
YWwpKQ0KPiA+PiA+PiA+IC0gICAgICAgICAgICAgICBpZWVlODAyMTFfd2FrZV9xdWV1ZXNfYnlf
cmVhc29uKCZsb2NhbC0+aHcsDQo+ID4+ID4+ID4gLSAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgIElFRUU4MDIxMV9NQVhfUVVFVUVfTUFQLA0KPiA+PiA+PiA+IC0gICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBJRUVFODAyMTFfUVVFVUVfU1RPUF9SRUFT
T05fQ1NBKTsNCj4gPj4gPj4gPiArICAgICAgICAgICAgICAgaWVlZTgwMjExX3dha2VfdmlmX3F1
ZXVlcyhsb2NhbCwgc2RhdGEsDQo+ID4+ID4+ID4gKyAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICAgICAgICAgSUVFRTgwMjExX1FVRVVFX1NUT1BfUkVBU09OX0NTQSk7DQo+ID4+ID4+
DQo+ID4+ID4+IEkgZG9uJ3QgdGhpbmsgdGhpcyBpcyBnb2luZyB0byB3b3JrIHdpdGggdGhlIHVw
Y29tbWluZyBtdWx0aS12aWYNCj4gPj4gPj4gY2hhbm5lbCBzd2l0Y2hpbmcuDQo+ID4+ID4+DQo+
ID4+ID4+IFRoZSBpZWVlODAyMTFfY3NhX25lZWRzX2Jsb2NrX3R4KCkgd2lsbCBiZWNvbWUgZmFs
c2UgdXBvbiAqbGFzdCoNCj4gPj4gPj4gaW50ZXJmYWNlIHN3aXRjaCBjb21wbGV0aW9uLiBUaGlz
IG1lYW5zIHByZWNlZWRpbmcgaW50ZXJmYWNlIGNzYQ0KPiA+PiA+PiBmaW5hbGl6YXRpb25zIHdv
bid0IGNhbGwgaWVlZTgwMjExX3dha2VfdmlmX3F1ZXVlcygpIHNvIHlvdSBhY3R1YWxseQ0KPiA+
PiA+PiBlbmQgdXAgbm90IHdha2luZyBhbGwgb2YgdGhlIHF1ZXVlcyB3aGVuIHlvdSBmaW5pc2gg
Y3NhLg0KPiA+PiA+DQo+ID4+ID4gSSB0aGluayB0aGUgcmlnaHQgd2F5IHRvIGRvIGl0IHdvdWxk
IGJlIHRvIHRyYXZlcnNlIHRoZSB2aWZzIHRoYXQNCj4gPj4gPiBzd2l0Y2hlZCBhbmQgd2FrZSB1
cCB0aGVpciBxdWV1ZXMgd2hlbiB0aGUgc3dpdGNoIGlzIGZpbmlzaGVkLg0KPiA+PiA+DQo+ID4+
ID4NCj4gPj4gPj4gSSB0aGluayB0aGlzIGNhbiBiZSBzb2x2ZWQgd2l0aDoNCj4gPj4gPj4NCj4g
Pj4gPj4gIGlmICghaWVlZTgwMjExX2NzYV9uZWVkc19ibG9ja190eChsb2NhbCkpDQo+ID4+ID4+
ICAgIGxpc3RfZm9yX2VhY2goc2RhdGEsICZsb2NhbC0+aW50ZXJmYWNlcywgbGlzdCkNCj4gPj4g
Pj4gICAgICBpZWVlODAyMTFfd2FrZV92aWZfcXVldWVzKGxvY2FsLCBzZGF0YSwgUkVBU09OX0NT
QSk7DQo+ID4+ID4+DQo+ID4+ID4+IEJ1dCB0aGVuIEkgZ3Vlc3MgaXQncyBqdXN0IGEgY29udm9s
dXRlZCB3YXkgb2Ygc2F5aW5nOg0KPiA+PiA+Pg0KPiA+PiA+PiAgaWYgKCFpZWVlODAyMTFfY3Nh
X25lZWRzX2Jsb2NrX3R4KGxvY2FsKSkNCj4gPj4gPj4gICAgaWVlZTgwMjExX3dha2VfcXVldWVz
X2J5X3JlYXNvbihodywgUkVBU09OX0NTQSk7DQo+ID4+ID4+DQo+ID4+ID4+IElPVyB3YWtpbmcg
c2hvdWxkIHJlbWFpbiBhcyBpdCB3YXMgd2hpbGUgc3RvcHBpbmcgY2FuIGJlIGRvbmUgcGVyLXZp
Zi4NCj4gPj4gPj4NCj4gPj4gPj4gVGhpcyBnb2VzIGZvciBhbGwgaW5zdGFuY2VzIGNhbGxpbmcg
d2FrZV92aWZfcXVldWVzKCkgaW4gdGhlIHBhdGNoLg0KPiA+PiA+DQo+ID4+ID4gQWN0dWFsbHkg
SSBoYWQgaXQgbGlrZSB0aGlzIGluIG15IGZpcnN0IHZlcnNpb24sIEkgaGFkIG9ubHkgYWRkZWQg
dGhlDQo+ID4+ID4gc3RvcCBwYXJ0LCBsZWF2aW5nIHRoZSB3YWtlIGFzIGl0IHdhcy4gIEJ1dCB0
aGVuIEkgdGhvdWdodCB0aGF0IHdlIHdvdWxkDQo+ID4+ID4gYmUgcmVzdGFydGluZyBxdWV1ZXMg
ZnJvbSBvdGhlciB2aWZzIHRoYXQgd2UgZG9uJ3Qga25vdyBhYm91dC4gIElmDQo+ID4+ID4gYW5v
dGhlciB2aWYgaXMgc3dpdGNoaW5nIHNlcGFyYXRlbHksIHdlIHdvdWxkIHJlc3RhcnQgaXRzIHF1
ZXVlcyBhdCB0aGUNCj4gPj4gPiB3cm9uZyB0aW1lLiAgSSB0aGluayB3ZSBzaG91bGRuJ3QgYXNz
dW1lIHRoYXQgYWxsIHZpZnMgYXJlIHBhcnQgb2YgdGhlDQo+ID4+ID4gc2FtZSBzd2l0Y2guDQo+
ID4+DQo+ID4+IFdlbGwsIG5vdCByZWFsbHkuDQo+ID4+DQo+ID4+IFRoZXJlIGlzIG5vICJ2aWYg
c3dpdGNoaW5nIHNlcGFyYXRlbHkiIHdoZW4gaXQgY29tZXMgdG8gYmxvY2tfdHguIFRoZQ0KPiA+
PiBjb25kaXRpb24gY2hlY2tzIGlmIEFOWSBpbnRlcmZhY2Ugc3RpbGwgbmVlZHMgcXVldWVzIHRv
IGJlIGxvY2tlZCBmb3INCj4gPj4gdGhlICpSRUFTT05fQ1NBKi4gSWYgdGhlIGNvbmRpdGlvbiBp
cyBmYWxzZSAoaS5lLiAibm8gaW50ZXJmYWNlIG5lZWRzDQo+ID4+IGJsb2NrX3R4IikgdGhlbiB5
b3UgYXJlIHNhZmUgdG8gd2FrZSAqYWxsKiBxdWV1ZXMgZm9yICpSRUFTT05fQ1NBKiBvbg0KPiA+
PiBhbGwgaW50ZXJmYWNlcyBiZWNhdXNlIGNoYW5uZWwgc3dpdGNoZXMgdGhhdCBkb24ndCBuZWVk
IGJsb2NrX3R4IGRvbid0DQo+ID4+IHN0b3AgdGhlaXIgcXVldWVzIGZvciBSRUFTT05fQ1NBIGlu
IHRoZSBmaXJzdCBwbGFjZS4NCj4gPg0KPiA+IFJpZ2h0LCBhbmQgdGhpcyB1c2VkIHRvIG1ha2Ug
c2Vuc2Ugd2hlbiB3ZSB3ZXJlIHN0b3BwaW5nICphbGwqIGh3X3F1ZXVlcw0KPiA+IHdoZW4gd2Ug
aGFkIGJsb2NrX3R4Lg0KPiA+DQo+ID4gTWF5YmUgd2Ugc2hvdWxkIG1ha2UgYSBuZXcgZnVuY3Rp
b24gdGhhdCByZXR1cm5zIGFsbCB0aGUgcXVldWVzIHRoYXQgb3VyDQo+ID4gdmlmIHdhcyBibG9j
a2luZyBidXQgdGhhdCBhcmUgbm90IGJsb2NrZWQgYnkgYW55IG90aGVyIHZpZnMuICBNYXliZQ0K
PiA+IHNvbWV0aGluZyBsaWtlIHRoaXM6DQo+ID4NCj4gPiBzdGF0aWMgdTMyIGllZWU4MDIxMV92
aWZfcXVldWVzX3RvX3dha2Uoc3RydWN0IGllZWU4MDIxMV9sb2NhbCAqbG9jYWwsDQo+ID4gICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIHN0cnVjdCBpZWVlODAyMTFfc3Vi
X2lmX2RhdGEgKnNkYXRhLA0KPiA+ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICBlbnVtIHF1ZXVlX3N0b3BfcmVhc29uIHJlYXNvbikNCj4gPiB7DQo+ID4gICAgICAgICBz
dHJ1Y3QgaWVlZTgwMjExX3N1Yl9pZl9kYXRhICpzZGF0YV9pdGVyOw0KPiA+ICAgICAgICAgdW5z
aWduZWQgbG9uZyBxdWV1ZXMsIHF1ZXVlc19pdGVyLCBxOw0KPiA+ICAgICAgICAgaW50IGk7DQo+
ID4NCj4gPiAgICAgICAgIGxvY2tkZXBfYXNzZXJ0X2hlbGQoJmxvY2FsLT5tdHgpOw0KPiA+DQo+
ID4gICAgICAgICBxdWV1ZXMgPSBpZWVlODAyMTFfZ2V0X3ZpZl9xdWV1ZXMobG9jYWwsIHNkYXRh
KTsNCj4gPg0KPiA+ICAgICAgICAgcmN1X3JlYWRfbG9jaygpOw0KPiA+ICAgICAgICAgbGlzdF9m
b3JfZWFjaF9lbnRyeV9yY3Uoc2RhdGFfaXRlciwgJmxvY2FsLT5pbnRlcmZhY2VzLCBsaXN0KSB7
DQo+ID4gICAgICAgICAgICAgICAgIGlmICghaWVlZTgwMjExX3NkYXRhX3J1bm5pbmcoc2RhdGFf
aXRlcikpDQo+ID4gICAgICAgICAgICAgICAgICAgICAgICAgY29udGludWU7DQo+ID4NCj4gPiAg
ICAgICAgICAgICAgICAgcXVldWVzX2l0ZXIgPSBpZWVlODAyMTFfZ2V0X3ZpZl9xdWV1ZXMobG9j
YWwsIHNkYXRhX2l0ZXIpOw0KPiA+ICAgICAgICAgICAgICAgICBmb3JfZWFjaF9zZXRfYml0KGks
ICZxdWV1ZXNfaXRlciwgbG9jYWwtPmh3LnF1ZXVlcykgew0KPiA+ICAgICAgICAgICAgICAgICAg
ICAgICAgIHEgPSBzZGF0YS0+dmlmLmh3X3F1ZXVlW2ldOw0KPiA+DQo+ID4gICAgICAgICAgICAg
ICAgICAgICAgICAgaWYgKHRlc3RfYml0KHJlYXNvbiwgJmxvY2FsLT5xdWV1ZV9zdG9wX3JlYXNv
bnNbcV0pKQ0KPiA+ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgcXVldWVzICY9IH5x
Ow0KPiA+ICAgICAgICAgICAgICAgICB9DQo+ID4gICAgICAgICB9DQo+ID4gICAgICAgICByY3Vf
cmVhZF91bmxvY2soKTsNCj4gPg0KPiA+ICAgICAgICAgcmV0dXJuIHF1ZXVlczsNCj4gPiB9DQo+
ID4NCj4gPiB2b2lkIGllZWU4MDIxMV93YWtlX3ZpZl9xdWV1ZXMoc3RydWN0IGllZWU4MDIxMV9s
b2NhbCAqbG9jYWwsDQo+ID4gICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgIHN0cnVjdCBp
ZWVlODAyMTFfc3ViX2lmX2RhdGEgKnNkYXRhLA0KPiA+ICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgICBlbnVtIHF1ZXVlX3N0b3BfcmVhc29uIHJlYXNvbikNCj4gPiB7DQo+ID4gICAgICAg
ICB1bnNpZ25lZCBpbnQgcXVldWVzID0gaWVlZTgwMjExX3ZpZl9xdWV1ZXNfdG9fd2FrZShsb2Nh
bCwgc2RhdGEsIHJlYXNvbik7DQo+ID4NCj4gPiAgICAgICAgIGllZWU4MDIxMV93YWtlX3F1ZXVl
c19ieV9yZWFzb24oJmxvY2FsLT5odywgcXVldWVzLCByZWFzb24pOw0KPiA+IH0NCj4gDQo+IE9u
Y2UgeW91IHNldCBsb2NhbC0+cXVldWVfc3RvcF9yZWFzb25zW10gZm9yIGFuIG92ZXJsYXBwaW5n
IHF1ZXVlIHlvdQ0KPiB3b24ndCBiZSBhYmxlIHRvIHdha2UgaXQgd2l0aCBpZWVlODAyMTFfd2Fr
ZV92aWZfcXVldWVzKCkgYWxvbmUuDQo+IFlvdSdsbCBuZWVkIHRvIGNhbGwgaWVlZTgwMjExX3dh
a2VfcXVldWVzKCkgdG8gZG8gdGhhdC4gSXMgdGhhdCB3aGF0DQo+IHlvdSBpbnRlbmRlZD8NCg0K
TW9yZSBvciBsZXNzLiAgTXkgaWRlYSBpcyB0aGF0IG9ubHkgdGhlICpsYXN0KiB2aWYgdGhhdCB0
cmllcyB0byB3YWtlDQp0aGUgb3ZlcmxhcHBpbmcgcXVldWUsIHdpbGwgYWN0dWFsbHkgZG8gc28u
DQoNCg0KPiA+PiA+PiBHZW5lcmFsbHkgSSBkb24ndCB0aGluayBoYXZpbmcgd2FrZV92aWZfcXVl
dWVzKCkgaXMgYSBnb29kIGlkZWEuIEl0J3MNCj4gPj4gPj4gcmVhbGx5IHRyaWNreS4gU3VyZSwg
eW91IGNhbiBoYXZlIGEgc3RvcF92aWZfcXVldWVzKCkgd2hpY2ggd2lsbA0KPiA+PiA+PiBiZWhh
dmUgYXMgeW91IG1pZ2h0IGV4cGVjdCAoaS5lLiBmaXJzdCBjYWxsIHN0b3BzIHF1ZXVlcyBhbmQN
Cj4gPj4gPj4gb3ZlcmxhcHBpbmcgdmlmLXF1ZXVlIG1hcHBpbmdzIGFyZSBub3QgYSBjb25jZXJu
KS4gQnV0DQo+ID4+ID4+IHdha2VfdmlmX3F1ZXVlcygpIHNlZW1zIHRvIGhhdmUgbGl0dGxlIHBy
YWN0aWNhbCB1c2UgdG8gbWUgd2l0aG91dCBhbnkNCj4gPj4gPj4ga2luZCBvZiBwZXItcXVldWUg
cmVmZXJlbmNlIGNvdW50aW5nLg0KPiA+PiA+DQo+ID4+ID4gWWVhaCwgdGhpcyBjb3VsZCBiZSBh
IHByb2JsZW0gZm9yIHZpZnMgdGhhdCBhcmUgc2hhcmluZyB0aGUgc2FtZSBxdWV1ZXMuDQo+ID4+
ID4gQnV0IHRoYXQgY2FzZSB3b3VsZCBiZSByZWFsbHkgdHJpY2t5IHdpdGggY2hhbm5lbC1zd2l0
Y2ggYW55d2F5LiBiZWNhdXNlDQo+ID4+ID4gaWYgdGhlIGNoYW5uZWwgb2Ygb25lIHZpZiBpcyBj
aGFuZ2VkIGJ1dCBub3QgdGhlIG90aGVyLCB0aGUgcXVldWUgd291bGQNCj4gPj4gPiBwcm9iYWJs
eSBoYXZlIHRvIGJlIHNwbGl0Li4uDQo+ID4+DQo+ID4+IEhtbS4gVGhhdCdzIGEgdmVyeSBnb29k
IHBvaW50LiBDYW4gd2UgZXZlbiByZS1tYXAgdGhpcyBkdXJpbmcgdGhlIHN3aXRjaCBub3c/DQo+
ID4+DQo+ID4+IElmIHdlIGNhbid0IHRoZW4gZG9lc24ndCBpdCBpbXBseSB0aGF0IGlmIHRoZXJl
IGFyZSBzaGFyZWQgdmlmLXF1ZXVlDQo+ID4+IG1hcHBpbmdzIHRoZW4gY2hhbm5lbCBzd2l0Y2hp
bmcgc2hvdWxkIGJlIGxpbWl0ZWQvcHJldmVudGVkPyBJLmUuIGl0DQo+ID4+IG1pZ2h0IGJlIG9r
YXkgaWYgeW91IGhhdmUgMiB2aWZzIHNoYXJpbmcgYSBxdWV1ZSB0byBzd2l0Y2ggdG8gdGhlDQo+
ID4+IGV4YWN0IHNhbWUgY2hhbm5lbCBidXQgaXQgd29uJ3Qgd29yayBpZiB5b3UgdHJ5IHRvIHN3
aXRjaCB0aGVtIHRvDQo+ID4+IHNlcGFyYXRlIGNoYW5uZWxzLiBTaW5nbGUtY2hhbm5lbCBkZXZp
Y2VzIHdvbid0IGhhdmUgdG8gd29ycnkgYWJvdXQgaXQNCj4gPj4gKHNpbmdsZS1jaGFubmVsIGNv
bWJpbmF0aW9uIGNhc2VzIGZvciB0aGF0IG1hdHRlcikuDQo+ID4NCj4gPiBZZWFoLCBpdCBzaG91
bGQgcHJvYmFibHkgYmUgcHJldmVudGVkIGZvciBzdWNoIGRyaXZlcnMuICBCdXQgdGhlbiwgYXQN
Cj4gPiB3aGF0IHBvaW50PyBQcmV2ZW50IHRoZSBjaGFubmVsIHN3aXRjaCB0byBvY2N1ciBjb21w
bGV0ZWx5IGlmIGFub3RoZXINCj4gPiB2aWYgc2hhcmVzIHRoZSBzYW1lIHF1ZXVlPyBPciBmYWls
IHRoZSBzd2l0Y2ggaWYgdGhlIG90aGVyIHZpZiBkb2Vzbid0DQo+ID4gam9pbiB0aGUgc3dpdGNo
Pw0KPiANCj4gVGhhdCdzIGEgZ29vZCBxdWVzdGlvbiB0b28uIEkgd291bGQgcHJlZmVyIHRvIGxl
YXZlIGl0IHVudGlsIHRoZSBsYXN0DQo+IHNlY29uZCwgZS5nLiBnaXZlIHRoZSBvcHBvcnR1bml0
eSB0byByZS1tYXAgaW4NCj4gZHJ2X3N3aXRjaF92aWZfY2hhbmN0eCgpIG1heWJlIHNvIGRyaXZl
ciBjYW4gZmFpbCBpZiBpdCdzIHVuYWJsZSB0bw0KPiBjb21wbHk/DQoNClllcywgSSB0ZW5kIHRv
IGFncmVlLg0KDQotLQ0KTHVjYS4NCg==
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] mac80211: stop only the queues assigned to the vif during channel switch
2014-06-06 8:04 ` Coelho, Luciano
@ 2014-06-06 8:12 ` Michal Kazior
2014-06-06 11:13 ` Coelho, Luciano
0 siblings, 1 reply; 9+ messages in thread
From: Michal Kazior @ 2014-06-06 8:12 UTC (permalink / raw)
To: Coelho, Luciano
Cc: linux-wireless@vger.kernel.org, Grumbach, Emmanuel,
johannes@sipsolutions.net
On 6 June 2014 10:04, Coelho, Luciano <luciano.coelho@intel.com> wrote:
> On Fri, 2014-06-06 at 09:56 +0200, Michal Kazior wrote:
>> On 6 June 2014 09:37, Coelho, Luciano <luciano.coelho@intel.com> wrote:
[...]
>> > static u32 ieee80211_vif_queues_to_wake(struct ieee80211_local *local,
>> > struct ieee80211_sub_if_data *sdata,
>> > enum queue_stop_reason reason)
>> > {
>> > struct ieee80211_sub_if_data *sdata_iter;
>> > unsigned long queues, queues_iter, q;
>> > int i;
>> >
>> > lockdep_assert_held(&local->mtx);
>> >
>> > queues = ieee80211_get_vif_queues(local, sdata);
>> >
>> > rcu_read_lock();
>> > list_for_each_entry_rcu(sdata_iter, &local->interfaces, list) {
>> > if (!ieee80211_sdata_running(sdata_iter))
>> > continue;
Hmm, actually, shouldn't we `if (sdata == sdata_iter) continue`?
Otherwise you'll never wake queues, will you?
>> > queues_iter = ieee80211_get_vif_queues(local, sdata_iter);
>> > for_each_set_bit(i, &queues_iter, local->hw.queues) {
>> > q = sdata->vif.hw_queue[i];
>> >
>> > if (test_bit(reason, &local->queue_stop_reasons[q]))
>> > queues &= ~q;
>> > }
>> > }
>> > rcu_read_unlock();
>> >
>> > return queues;
>> > }
>> >
>> > void ieee80211_wake_vif_queues(struct ieee80211_local *local,
>> > struct ieee80211_sub_if_data *sdata,
>> > enum queue_stop_reason reason)
>> > {
>> > unsigned int queues = ieee80211_vif_queues_to_wake(local, sdata, reason);
>> >
>> > ieee80211_wake_queues_by_reason(&local->hw, queues, reason);
>> > }
>>
>> Once you set local->queue_stop_reasons[] for an overlapping queue you
>> won't be able to wake it with ieee80211_wake_vif_queues() alone.
>> You'll need to call ieee80211_wake_queues() to do that. Is that what
>> you intended?
>
> More or less. My idea is that only the *last* vif that tries to wake
> the overlapping queue, will actually do so.
So ieee80211_wake_vif_queues() would be called regardless of the
ieee80211_csa_needs_block_tx() and ieee80211_wake_queues() is called
like it was before (i.e. when all block_tx interfaces are done).
Michał
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH 3/3] mac80211: stop only the queues assigned to the vif during channel switch
2014-06-06 8:12 ` Michal Kazior
@ 2014-06-06 11:13 ` Coelho, Luciano
0 siblings, 0 replies; 9+ messages in thread
From: Coelho, Luciano @ 2014-06-06 11:13 UTC (permalink / raw)
To: michal.kazior@tieto.com
Cc: linux-wireless@vger.kernel.org, Grumbach, Emmanuel,
johannes@sipsolutions.net
T24gRnJpLCAyMDE0LTA2LTA2IGF0IDEwOjEyICswMjAwLCBNaWNoYWwgS2F6aW9yIHdyb3RlOg0K
PiBPbiA2IEp1bmUgMjAxNCAxMDowNCwgQ29lbGhvLCBMdWNpYW5vIDxsdWNpYW5vLmNvZWxob0Bp
bnRlbC5jb20+IHdyb3RlOg0KPiA+IE9uIEZyaSwgMjAxNC0wNi0wNiBhdCAwOTo1NiArMDIwMCwg
TWljaGFsIEthemlvciB3cm90ZToNCj4gPj4gT24gNiBKdW5lIDIwMTQgMDk6MzcsIENvZWxobywg
THVjaWFubyA8bHVjaWFuby5jb2VsaG9AaW50ZWwuY29tPiB3cm90ZToNCj4gWy4uLl0NCj4gPj4g
PiBzdGF0aWMgdTMyIGllZWU4MDIxMV92aWZfcXVldWVzX3RvX3dha2Uoc3RydWN0IGllZWU4MDIx
MV9sb2NhbCAqbG9jYWwsDQo+ID4+ID4gICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg
ICAgICAgIHN0cnVjdCBpZWVlODAyMTFfc3ViX2lmX2RhdGEgKnNkYXRhLA0KPiA+PiA+ICAgICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBlbnVtIHF1ZXVlX3N0b3BfcmVhc29u
IHJlYXNvbikNCj4gPj4gPiB7DQo+ID4+ID4gICAgICAgICBzdHJ1Y3QgaWVlZTgwMjExX3N1Yl9p
Zl9kYXRhICpzZGF0YV9pdGVyOw0KPiA+PiA+ICAgICAgICAgdW5zaWduZWQgbG9uZyBxdWV1ZXMs
IHF1ZXVlc19pdGVyLCBxOw0KPiA+PiA+ICAgICAgICAgaW50IGk7DQo+ID4+ID4NCj4gPj4gPiAg
ICAgICAgIGxvY2tkZXBfYXNzZXJ0X2hlbGQoJmxvY2FsLT5tdHgpOw0KPiA+PiA+DQo+ID4+ID4g
ICAgICAgICBxdWV1ZXMgPSBpZWVlODAyMTFfZ2V0X3ZpZl9xdWV1ZXMobG9jYWwsIHNkYXRhKTsN
Cj4gPj4gPg0KPiA+PiA+ICAgICAgICAgcmN1X3JlYWRfbG9jaygpOw0KPiA+PiA+ICAgICAgICAg
bGlzdF9mb3JfZWFjaF9lbnRyeV9yY3Uoc2RhdGFfaXRlciwgJmxvY2FsLT5pbnRlcmZhY2VzLCBs
aXN0KSB7DQo+ID4+ID4gICAgICAgICAgICAgICAgIGlmICghaWVlZTgwMjExX3NkYXRhX3J1bm5p
bmcoc2RhdGFfaXRlcikpDQo+ID4+ID4gICAgICAgICAgICAgICAgICAgICAgICAgY29udGludWU7
DQo+IA0KPiBIbW0sIGFjdHVhbGx5LCBzaG91bGRuJ3Qgd2UgYGlmIChzZGF0YSA9PSBzZGF0YV9p
dGVyKSBjb250aW51ZWA/DQo+IE90aGVyd2lzZSB5b3UnbGwgbmV2ZXIgd2FrZSBxdWV1ZXMsIHdp
bGwgeW91Pw0KDQpBaG1tbS4uLiBZZXMgb2YgY291cnNlLiA6KQ0KDQoNCj4gPj4gPiAgICAgICAg
ICAgICAgICAgcXVldWVzX2l0ZXIgPSBpZWVlODAyMTFfZ2V0X3ZpZl9xdWV1ZXMobG9jYWwsIHNk
YXRhX2l0ZXIpOw0KPiA+PiA+ICAgICAgICAgICAgICAgICBmb3JfZWFjaF9zZXRfYml0KGksICZx
dWV1ZXNfaXRlciwgbG9jYWwtPmh3LnF1ZXVlcykgew0KPiA+PiA+ICAgICAgICAgICAgICAgICAg
ICAgICAgIHEgPSBzZGF0YS0+dmlmLmh3X3F1ZXVlW2ldOw0KPiA+PiA+DQo+ID4+ID4gICAgICAg
ICAgICAgICAgICAgICAgICAgaWYgKHRlc3RfYml0KHJlYXNvbiwgJmxvY2FsLT5xdWV1ZV9zdG9w
X3JlYXNvbnNbcV0pKQ0KPiA+PiA+ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgcXVl
dWVzICY9IH5xOw0KPiA+PiA+ICAgICAgICAgICAgICAgICB9DQo+ID4+ID4gICAgICAgICB9DQo+
ID4+ID4gICAgICAgICByY3VfcmVhZF91bmxvY2soKTsNCj4gPj4gPg0KPiA+PiA+ICAgICAgICAg
cmV0dXJuIHF1ZXVlczsNCj4gPj4gPiB9DQo+ID4+ID4NCj4gPj4gPiB2b2lkIGllZWU4MDIxMV93
YWtlX3ZpZl9xdWV1ZXMoc3RydWN0IGllZWU4MDIxMV9sb2NhbCAqbG9jYWwsDQo+ID4+ID4gICAg
ICAgICAgICAgICAgICAgICAgICAgICAgICAgIHN0cnVjdCBpZWVlODAyMTFfc3ViX2lmX2RhdGEg
KnNkYXRhLA0KPiA+PiA+ICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICBlbnVtIHF1ZXVl
X3N0b3BfcmVhc29uIHJlYXNvbikNCj4gPj4gPiB7DQo+ID4+ID4gICAgICAgICB1bnNpZ25lZCBp
bnQgcXVldWVzID0gaWVlZTgwMjExX3ZpZl9xdWV1ZXNfdG9fd2FrZShsb2NhbCwgc2RhdGEsIHJl
YXNvbik7DQo+ID4+ID4NCj4gPj4gPiAgICAgICAgIGllZWU4MDIxMV93YWtlX3F1ZXVlc19ieV9y
ZWFzb24oJmxvY2FsLT5odywgcXVldWVzLCByZWFzb24pOw0KPiA+PiA+IH0NCj4gPj4NCj4gPj4g
T25jZSB5b3Ugc2V0IGxvY2FsLT5xdWV1ZV9zdG9wX3JlYXNvbnNbXSBmb3IgYW4gb3ZlcmxhcHBp
bmcgcXVldWUgeW91DQo+ID4+IHdvbid0IGJlIGFibGUgdG8gd2FrZSBpdCB3aXRoIGllZWU4MDIx
MV93YWtlX3ZpZl9xdWV1ZXMoKSBhbG9uZS4NCj4gPj4gWW91J2xsIG5lZWQgdG8gY2FsbCBpZWVl
ODAyMTFfd2FrZV9xdWV1ZXMoKSB0byBkbyB0aGF0LiBJcyB0aGF0IHdoYXQNCj4gPj4geW91IGlu
dGVuZGVkPw0KPiA+DQo+ID4gTW9yZSBvciBsZXNzLiAgTXkgaWRlYSBpcyB0aGF0IG9ubHkgdGhl
ICpsYXN0KiB2aWYgdGhhdCB0cmllcyB0byB3YWtlDQo+ID4gdGhlIG92ZXJsYXBwaW5nIHF1ZXVl
LCB3aWxsIGFjdHVhbGx5IGRvIHNvLg0KPiANCj4gU28gaWVlZTgwMjExX3dha2VfdmlmX3F1ZXVl
cygpIHdvdWxkIGJlIGNhbGxlZCByZWdhcmRsZXNzIG9mIHRoZQ0KPiBpZWVlODAyMTFfY3NhX25l
ZWRzX2Jsb2NrX3R4KCkgYW5kIGllZWU4MDIxMV93YWtlX3F1ZXVlcygpIGlzIGNhbGxlZA0KPiBs
aWtlIGl0IHdhcyBiZWZvcmUgKGkuZS4gd2hlbiBhbGwgYmxvY2tfdHggaW50ZXJmYWNlcyBhcmUg
ZG9uZSkuDQoNClllcywgaWVlZTgwMjExX3dha2VfdmlmX3F1ZXVlcygpIHdvdWxkIGJlIGNhbGxl
ZCByZWdhcmRsZXNzIG9mIHRoZQ0KaWVlZTgwMjExX2NzYV9uZWVkc19ibG9ja190eCgpLCBidXQg
d2Ugc2hvdWxkIHByb2JhYmx5IHN0aWxsIGNoZWNrIGlmDQp0aGUgdmlmIGluIHF1ZXN0aW9uIGhh
cyBibG9ja190eCBzZXQ/DQoNCkFuZCBJIGRvbid0IHNlZSB3aHkgd2Ugd291bGQgaGF2ZSB0byBj
YWxsIGllZWU4MDIxMV93YWtlX3F1ZXVlcygpIGFnYWluDQphdCB0aGUgZW5kLi4uIHdvdWxkIHRo
YXQgc3RpbGwgYmUgbmVjZXNzYXJ5Pw0KDQotLQ0KTHVjYS4NCg==
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2014-06-06 11:14 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-06-05 16:26 [PATCH 3/3] mac80211: stop only the queues assigned to the vif during channel switch Emmanuel Grumbach
2014-06-06 5:51 ` Michal Kazior
2014-06-06 6:31 ` Coelho, Luciano
2014-06-06 6:44 ` Michal Kazior
2014-06-06 7:37 ` Coelho, Luciano
2014-06-06 7:56 ` Michal Kazior
2014-06-06 8:04 ` Coelho, Luciano
2014-06-06 8:12 ` Michal Kazior
2014-06-06 11:13 ` Coelho, Luciano
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.