On Mon, Mar 9, 2015 at 2:52 PM, Johannes Berg <johannes@xxxxxxxxxxxxxxxx> wrote: > On Mon, 2015-03-09 at 13:53 +0200, Eliad Peller wrote: > >> Fix it by cancelling hw_roc_done on roc cancel. >> Since hw_roc_done takes local->mtx, temporarily >> release it (before re-acquiring it and starting >> the next roc if needed). > > I can't say I like this, and I'm not even sure it doesn't leave a race? > After all, the work will not take the RTNL. > the other way (cancel while hw_roc_done) is protected, since ieee80211_cancel_roc() looks for a specific roc cookie. > Also, I think the whole concept is racy because you acquire "found" > before the unlock, but use it after the unlock, so if the work actually > ran (_sync, perhaps it already started) it may have freed the "found" > pointer. > hw_roc_done work bails out in case of no started roc, so i think we're safe here. > I think the only way to really fix that is to make the driver return the > roc pointer or so and then we can either queue a work struct per roc > struct, or at least mark the roc struct as driver-destroyed and > double-check that in the work function? > that was my initial implementation. but then i thought it will be nicer to implement it in mac80211 and avoid changing all the drivers (with the need to save the cookie, etc.) do you prefer adding such cookie/pointer param? > Or perhaps we could flush the work before we even take the lock, but > then it might still race with the driver trying to cancel just after we > flushed I guess. right. Eliad. -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
