On Mon, 2019-01-28 at 11:00 +0200, Kalle Valo wrote:
> Luca Coelho <luca@xxxxxxxxx> writes:
>
> > From: Sara Sharon <sara.sharon@xxxxxxxxx>
> >
> > Currently whenever we get firmware notification with mac id,
> > we iterate over all the interfaces to find the ID. This is a
> > bit cumbersome. Instead, adding an array of RCU pointers, like
> > we have for station IDs. This is not expensive space wise
> > since we have only up to 4 active MACs, and not complicated
> > code wise, since we have a clear point to init and de-init it.
> >
> > Signed-off-by: Sara Sharon <sara.sharon@xxxxxxxxx>
> > Signed-off-by: Luca Coelho <luciano.coelho@xxxxxxxxx>
>
> [...]
>
> > +static inline struct ieee80211_vif *
> > +iwl_mvm_rcu_dereference_vif_id(struct iwl_mvm *mvm, u8 vif_id,
> > bool rcu)
> > +{
> > + if (WARN_ON(vif_id >= ARRAY_SIZE(mvm->vif_id_to_mac)))
> > + return NULL;
> > +
> > + if (rcu)
> > + return rcu_dereference(mvm->vif_id_to_mac[vif_id]);
> > +
> > + return rcu_dereference_protected(mvm->vif_id_to_mac[vif_id],
> > + lockdep_is_held(&mvm->mutex));
> > +}
>
> No need to change anything, but IMHO foo(bar) and foo_protected(bar)
> is a lot easier to read than foo(bar, true) and foo(bar, false).
Indeed! And Greg already warned us about the usage of booleans in our
APIs (on another, unrelated matter). ;)
I'll pay more attention to this kind of stuff during our internal
reviews.
Thanks!
--
Cheers,
Luca.
