On Wed, 2013-12-04 at 23:47 +0100, Christian Lamparter wrote: > On Wednesday, December 04, 2013 11:23:49 PM Johannes Berg wrote: > > On Wed, 2013-12-04 at 23:16 +0100, Christian Lamparter wrote: > > > > Is there any other driver that assumes it is safe to delete a station > > > > pointer in the sta_state callback and not use synchronize_rcu()? From > > > > looking at the code, I don't see any, but I can't really be sure that > > > > everyone uses __rcu annotations correctly ... :) > > > > > No, carl9170 doesn't use sta_state but it uses sta_remove. > > > > Yeah, I actually saw this. But I think you use it for aggregation > > sessions only? And the code didn't look like it could still get to the > > station pointer after it was removed with sta_remove() callback, but I > > may be wrong. > no, you are not wrong. The code gets its sta pointers either from > callback-parameters or via ieee80211_get_sta. [But would it even be > possible to do it any other way? After all, this should crash "sooner > or later" otherwise]. Yeah that's what I thought. Both of those are obviously fine. In iwlmvm, we have something like this: struct ieee80211_sta __rcu *our_stations[16]; and then just set it to NULL (well, simplifying a bit) on sta_remove (actually sta_state.) This is safe because after sta_state/sta_remove, mac80211 still does call_rcu() to really free the station, but it's kinda pointless. By adding the callback I'd suggested before to set this to NULL before the synchronize_net() in mac80211, I think we can get rid of the whole call_rcu() stuff. Anyway, right now my patch is breaking iwlmvm, I'll try to sort that out tomorrow and then post patches. johannes -- 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
