On Thu, 2008-09-11 at 19:42 +0200, Johannes Berg wrote:
> On Thu, 2008-09-11 at 19:39 +0200, Johannes Berg wrote:
>
> > Well take the larger bit of code:
> >
> > struct something *an = NULL;
> >
> > ...
> >
> > rcu_read_lock();
> > sta = ieee80211_find_sta(hw, hdr->addr2);
> > if (sta)
> > an = (void *) sta->drv_priv;
> >
> > if (an) {
> > ath_rx_input(sc, an,
> > hw->conf.ht_conf.ht_supported,
> > skb, status, &st);
> > }
> > rcu_read_unlock();
> >
> > /* the "!an" here is fine even outside RCU lock */
> > if (!an || (st != ATH_RX_CONSUMED))
> > __ieee80211_rx(hw, skb, &rx_status);
> >
> >
> > So at this point it's only checking whether above it had a pointer, it's
> > not accessing it. Think of the "an" variable, after rcu_read_unlock(),
> > as a bool indicating whether or not the code that just happened had
> > access to the node or not.
>
> That said, here it's probably smarter to just initialise "st" to
> something other than _RX_CONSUMED and remove that !an condition
> entirely.
And why is that not the return value of ath_rx_input? Anyway, I'm
getting off-topic here :)
johannes
Attachment:
signature.asc
Description: This is a digitally signed message part
