Re: question for mac80211 driver maintainers - RCU usage in drivers

[Johannes Berg <johannes@sipsolutions.net>]

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.

> I'm curious: how you would achieve this feat? After all, mac80211's
> tx- and rx-paths currently relies on RCU protected station pointers 
> too (and all that goes along with it. e.g.: ampdu sessions, keys, ...)?

Oh, that'll stay. But right now we do *two* things:
 a) synchronize_net() to protect against mac80211
 b) rcu_barrier() stuff (see commit b22cfcfca) to protect again

It seems to me if the drivers don't assume RCU grace period after
sta_state/sta_remove, then the second one can go away.

> > PS: I'll probably have to add another callback "sta going away before
> > RCU" so you can invalidate pointers there ... otherwise I'd have to
> > synchronize_rcu() in iwlmvm which would kinda defeat the purpose.
> hm, invalidating ampdu sessions is going to be tricky isn't it?
> But ok, some time ago I played around with moving the complicated
> amdpu scheduler of carl9170 into the (single-threaded) firmware.
> So large parts of carl9170's code which uses rcu in this context would
> become "unnecessary".
> 
> I didn't merge the patch - since there was no obvious benefit - but
> I still have them and they can be revived if needed.

No, don't worry. I'm keeping the aggregation session guarantees even,
right now I'm not touching that so each aggregation session still uses
synchronize_rcu() etc.

johannes