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(>lock);
> >
> > if (test_bit(IEEE80211_TXQ_STOP, >flags))
> > @@ -3513,6 +3534,8 @@ struct sk_buff *ieee80211_tx_dequeue(struct
> > ieee80211_hw *hw,
> > out:
> > spin_unlock_bh(>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