From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from s3.sipsolutions.net ([5.9.151.49] helo=sipsolutions.net) by bombadil.infradead.org with esmtps (Exim 4.85_2 #1 (Red Hat Linux)) id 1cDthV-0004VM-0G for ath10k@lists.infradead.org; Mon, 05 Dec 2016 13:53:02 +0000 Message-ID: <1480945950.31788.4.camel@sipsolutions.net> Subject: Re: [PATCH 1/2] mac80211: do not iterate active interfaces when in re-configure From: Johannes Berg Date: Mon, 05 Dec 2016 14:52:30 +0100 In-Reply-To: (sfid-20161205_092619_855272_0854B225) References: <1480645800-2148-1-git-send-email-greearb@candelatech.com> (sfid-20161205_092619_855272_0854B225) Mime-Version: 1.0 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Sender: "ath10k" Errors-To: ath10k-bounces+kvalo=adurom.com@lists.infradead.org To: Michal Kazior , Ben Greear Cc: linux-wireless , "ath10k@lists.infradead.org" T24gTW9uLCAyMDE2LTEyLTA1IGF0IDA5OjEzICswMTAwLCBNaWNoYWwgS2F6aW9yIHdyb3RlOgo+ IE9uIDIgRGVjZW1iZXIgMjAxNiBhdCAwMzoyOSzCoMKgPGdyZWVhcmJAY2FuZGVsYXRlY2guY29t PiB3cm90ZToKPiA+IAo+ID4gRnJvbTogQmVuIEdyZWVhciA8Z3JlZWFyYkBjYW5kZWxhdGVjaC5j b20+Cj4gPiAKPiA+IFRoaXMgYXBwZWFycyB0byBmaXggYSBwcm9ibGVtIHdoZXJlIGF0aDEwayBm aXJtd2FyZSB3b3VsZCBjcmFzaCwKPiA+IG1hYzgwMjExIHdvdWxkIHN0YXJ0IHJlLWFkZGluZyBp bnRlcmZhY2VzIHRvIHRoZSBkcml2ZXIsIGJ1dCB0aGUKPiA+IGl0ZXJhdGUtYWN0aXZlLWludGVy ZmFjZXMgbG9naWMgd291bGQgdGhlbiB0cnkgdG8gdXNlIHRoZSBoYWxmLQo+ID4gYnVpbHQKPiA+ IGludGVyZmFjZXMuwqDCoFdpdGggYSBiaXQgb2YgZXh0cmEgZGVidWcgdG8gY2F0Y2ggdGhlIHBy b2JsZW0sIHRoZQo+ID4gYXRoMTBrIGNyYXNoIGxvb2tzIGxpa2UgdGhpczoKPiA+IAo+ID4gYXRo MTBrX3BjaSAwMDAwOjA1OjAwLjA6IEluaXRpYWxpemluZyBhcnZpZjogZmZmZjg4MDFjZTk3ZTMy MCBvbgo+ID4gdmlmOiBmZmZmODgwMWNlOTdlMWQ4Cj4gPiAKPiA+IFt0aGUgcHJpbnQgdGhhdCBo YXBwZW5zIGFmdGVyIGFydmlmLT5hciBpcyBhc3NpZ25lZCBpcyBub3Qgc2hvd24sCj4gPiBzbyBj b2RlIGRpZCBub3QgbWFrZSBpdCB0aGF0IGZhciBiZWZvcmUKPiA+IMKgdGhlIHR4LWJlYWNvbi1u b3dhaXQgbWV0aG9kIHdhcyBjYWxsZWRdCj4gPiAKPiA+IHR4LWJlYWNvbi1ub3dhaXQ6wqDCoGFy dmlmOiBmZmZmODgwMWNlOTdlMzIwwqDCoGFyOsKgwqDCoMKgwqDCoMKgwqDCoMKgwqAobnVsbCkK PiBbLi4uXQo+ID4gCj4gPiAKPiA+IFNpZ25lZC1vZmYtYnk6IEJlbiBHcmVlYXIgPGdyZWVhcmJA Y2FuZGVsYXRlY2guY29tPgo+ID4gLS0tCj4gPiDCoG5ldC9tYWM4MDIxMS91dGlsLmMgfCAyICst Cj4gPiDCoDEgZmlsZSBjaGFuZ2VkLCAxIGluc2VydGlvbigrKSwgMSBkZWxldGlvbigtKQo+ID4g Cj4gPiBkaWZmIC0tZ2l0IGEvbmV0L21hYzgwMjExL3V0aWwuYyBiL25ldC9tYWM4MDIxMS91dGls LmMKPiA+IGluZGV4IDg2M2YyYzEuLmFiZTFmNjQgMTAwNjQ0Cj4gPiAtLS0gYS9uZXQvbWFjODAy MTEvdXRpbC5jCj4gPiArKysgYi9uZXQvbWFjODAyMTEvdXRpbC5jCj4gPiBAQCAtNzA1LDcgKzcw NSw3IEBAIHN0YXRpYyB2b2lkIF9faXRlcmF0ZV9pbnRlcmZhY2VzKHN0cnVjdAo+ID4gaWVlZTgw MjExX2xvY2FsICpsb2NhbCwKPiA+IMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDC oMKgwqDCoMKgwqDCoGJyZWFrOwo+ID4gwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqB9 Cj4gPiDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoGlmICghKGl0ZXJfZmxhZ3MgJiBJ RUVFODAyMTFfSUZBQ0VfSVRFUl9SRVNVTUVfQUxMKQo+ID4gJiYKPiA+IC3CoMKgwqDCoMKgwqDC oMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoGFjdGl2ZV9vbmx5ICYmICEoc2RhdGEtPmZsYWdzICYK PiA+IElFRUU4MDIxMV9TREFUQV9JTl9EUklWRVIpKQo+ID4gK8KgwqDCoMKgwqDCoMKgwqDCoMKg wqDCoMKgwqDCoMKgwqDCoMKgKGFjdGl2ZV9vbmx5ICYmIChsb2NhbC0+aW5fcmVjb25maWcgfHwg IShzZGF0YS0KPiA+ID5mbGFncyAmIElFRUU4MDIxMV9TREFUQV9JTl9EUklWRVIpKSkpCj4gPiDC oMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqDCoMKgwqBjb250aW51ZTsK PiAKPiBEb2Vzbid0IHRoaXMgZWZmZWN0aXZlbGx5IHByZXZlbnQgeW91IGZyb20gaXRlcmF0aW5n IG92ZXIgaW50ZXJmYWNlcwo+IGNvbXBsZXRlbHkgZHVyaW5nIHJlY29uZmlnPyBBcyB5b3UgYnJp bmcgdXAgaW50ZXJmYWNlcyB5b3UgbWlnaHQKPiBuZWVkL3dhbnQgdG8gaXRlcmF0ZSBvdmVyIG90 aGVycyB0byByZS1hZGp1c3QgeW91ciBvd24gc3RhdGUuCgpBZ3JlZSwgdGhhdCBkb2Vzbid0IHJl YWxseSBtYWtlIHNlbnNlLgoKPiBJJ2QgYXJndWUgdGhlcmUgc2hvdWxkIGJlIGFub3RoZXIgZmxh ZywgSUVFRTgwMjExX1NEQVRBX1JFU1VNSU5HLAo+IHVzZWQgd2l0aCBzZGF0YS0+ZmxhZ3MgZm9y IHJlc3VtaW5nIHNvIHRoYXQgb25jZSBpdCBpcyByZS1hZGRlZCB0bwo+IHRoZSBkcml2ZXIgaXQg Y2FuIGJlIGNsZWFyZWQgKGFuZCB0aGVyZWZvcmUgcHJvcGVybHkgaXRlcmF0ZWQgb3ZlcikuCgpU aGF0IHdvdWxkIG1ha2Ugc29tZSBzZW5zZSwgb3IgcGVyaGFwcyB0aGUgc2RhdGFfaW5fZHJpdmVy IHNob3VsZCBiZQpjbGVhcmVkIChhbmQgcmVtZW1iZXJlZCBlbHNld2hlcmUpIGF0IHNvbWUgcG9p bnQgZHVyaW5nIHRoZSByZXN0YXJ0LgoKam9oYW5uZXMKCl9fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fCmF0aDEwayBtYWlsaW5nIGxpc3QKYXRoMTBrQGxpc3Rz LmluZnJhZGVhZC5vcmcKaHR0cDovL2xpc3RzLmluZnJhZGVhZC5vcmcvbWFpbG1hbi9saXN0aW5m by9hdGgxMGsK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from s3.sipsolutions.net ([5.9.151.49]:43162 "EHLO sipsolutions.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751320AbcLEOX6 (ORCPT ); Mon, 5 Dec 2016 09:23:58 -0500 Message-ID: <1480945950.31788.4.camel@sipsolutions.net> (sfid-20161205_152402_431660_1D110B32) Subject: Re: [PATCH 1/2] mac80211: do not iterate active interfaces when in re-configure From: Johannes Berg To: Michal Kazior , Ben Greear Cc: linux-wireless , "ath10k@lists.infradead.org" Date: Mon, 05 Dec 2016 14:52:30 +0100 In-Reply-To: (sfid-20161205_092619_855272_0854B225) References: <1480645800-2148-1-git-send-email-greearb@candelatech.com> (sfid-20161205_092619_855272_0854B225) Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Sender: linux-wireless-owner@vger.kernel.org List-ID: On Mon, 2016-12-05 at 09:13 +0100, Michal Kazior wrote: > On 2 December 2016 at 03:29,   wrote: > > > > From: Ben Greear > > > > This appears to fix a problem where ath10k firmware would crash, > > mac80211 would start re-adding interfaces to the driver, but the > > iterate-active-interfaces logic would then try to use the half- > > built > > interfaces.  With a bit of extra debug to catch the problem, the > > ath10k crash looks like this: > > > > ath10k_pci 0000:05:00.0: Initializing arvif: ffff8801ce97e320 on > > vif: ffff8801ce97e1d8 > > > > [the print that happens after arvif->ar is assigned is not shown, > > so code did not make it that far before > >  the tx-beacon-nowait method was called] > > > > tx-beacon-nowait:  arvif: ffff8801ce97e320  ar:           (null) > [...] > > > > > > Signed-off-by: Ben Greear > > --- > >  net/mac80211/util.c | 2 +- > >  1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/net/mac80211/util.c b/net/mac80211/util.c > > index 863f2c1..abe1f64 100644 > > --- a/net/mac80211/util.c > > +++ b/net/mac80211/util.c > > @@ -705,7 +705,7 @@ static void __iterate_interfaces(struct > > ieee80211_local *local, > >                         break; > >                 } > >                 if (!(iter_flags & IEEE80211_IFACE_ITER_RESUME_ALL) > > && > > -                   active_only && !(sdata->flags & > > IEEE80211_SDATA_IN_DRIVER)) > > +                   (active_only && (local->in_reconfig || !(sdata- > > >flags & IEEE80211_SDATA_IN_DRIVER)))) > >                         continue; > > Doesn't this effectivelly prevent you from iterating over interfaces > completely during reconfig? As you bring up interfaces you might > need/want to iterate over others to re-adjust your own state. Agree, that doesn't really make sense. > I'd argue there should be another flag, IEEE80211_SDATA_RESUMING, > used with sdata->flags for resuming so that once it is re-added to > the driver it can be cleared (and therefore properly iterated over). That would make some sense, or perhaps the sdata_in_driver should be cleared (and remembered elsewhere) at some point during the restart. johannes