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
>> data_lock.
>
> 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
>> (ath10k_peer
>> 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.


MichaƂ

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

Reply via email to