On 13 January 2017 at 08:24, Johannes Berg <johan...@sipsolutions.net> wrote:
>> Unless you then continue to use that sta pointer after you release
> Ouch, ok. That's rather strangely hidden though.
>> Consider this:
>> > CPU0 CPU1
>> > 1 synchronize_net()
>> > 2 drv_sta_state()
>> > 3 htt_fetch_ind(pid,tid) called
>> > 4 rcu_read_lock()
>> > 5 get(data_lock)
>> > 6 txq=peers[pid]->sta->txq[tid]
>> > 7 put(data_lock)
>> > 8 get(data_lock)
>> > 9 peer->sta=0
>> > 10 put(data_lock)
>> > 11 kfree(sta)
>> > 12 ieee80211_tx_dequeue(txq)
>> Even though there's no code like (9) per se you can think of it as
>> anything that tries to "remove" the peer--sta association
>> is removed implicitly via wmi peer delete command and waiting for htt
>> event completion).
>> Holding data_lock for the entire duration of handling fetch
>> indication isn't really good for performance so it's better to fix
>> RCU handling.
> Yeah, I see it now - also the comment where this happens. You probably
> should mark some things __rcu though and actually use RCU primitives,
> so the code is actually understandable :)
Good point. I'll do that in another patch.
ath10k mailing list