On Mon, Sep 18, 2017 at 10:11:17PM +0200, Johannes Berg wrote: > > I got the following lockdep warning about the rcu_dereference()s in > > ieee80211_tx_h_select_key(). After tracing all callers of > > ieee80211_tx_h_select_key() I discovered that > > ieee80211_get_buffered_bc() > > and ieee80211_build_data_template() had the rcu_read_lock/unlock() > > but > > three other places did not. So I just blindly added them and made the > > read side critical section extend as far as the lifetime of 'tx' > > which > > is where we seem to be stuffing the rcu protected pointers. No real > > clue whether this is correct or not. > > Heh. > > I think we should do it in ieee80211_tx_dequeue(),
Oh, I guess I didn't trace the call chains far enough. ieee80211_tx() does indeed look OK. But unless I made another mistake in my analysis ieee80211_tx_prepare_skb() is still busted. > if not even in the > driver (and document that it's required) > > johannes > > > @@ -3411,6 +3430,8 @@ struct sk_buff *ieee80211_tx_dequeue(struct > > ieee80211_hw *hw, > > ieee80211_tx_result r; > > struct ieee80211_vif *vif; > > > > + rcu_read_lock(); > > + > > spin_lock_bh(&fq->lock); > > > > if (test_bit(IEEE80211_TXQ_STOP, &txqi->flags)) > > @@ -3513,6 +3534,8 @@ struct sk_buff *ieee80211_tx_dequeue(struct > > ieee80211_hw *hw, > > out: > > spin_unlock_bh(&fq->lock); > > > > + rcu_read_unlock(); > > > > i.e. this in itself should be sufficient, though you should probably > reorder and acquire the spinlock first since that might spin, and you > want to keep the RCU section minimal (it's trivial here, after all) Good point. I'll respin with that change. -- Ville Syrjälä Intel OTC
