On Tuesday 24 November 2009 08:15:53 Eric Dumazet wrote: > Arnd Bergmann a écrit : > > > > +static inline void macvlan_count_rx(const struct macvlan_dev *vlan, int length, > > + int success, int multicast) > > success and multicast should be declared as bool ok > > +{ > > + struct macvlan_rx_stats *rx_stats; > > + > > + rx_stats = per_cpu_ptr(vlan->rx_stats, smp_processor_id()); > > + rx_stats->rx_packets += success != 0; > > + rx_stats->rx_bytes += success ? length : 0; > > + rx_stats->multicast += success && multicast; > > + rx_stats->rx_errors += !success; > > +} > > + > > I find following more readable, it probably generates more branches, > but avoid dirtying rx_errors if it is in another cache line. > > if (likely(success)) { > rx_stats->rx_packets++; > rx_stats->rx_bytes += length; > if (multicast) > rx_stats->multicast++; > } else { > rx_stats->rx_errors++; > } Given that the structure only has four members and alloc_percpu requests cache aligned data, it is rather likely to be in the same cache line. I'll have a look at what gcc generates on x86-64 for both versions and use the version you suggested unless it looks significantly more expensive. Since we're into micro-optimization territory, do you think it should be marked inline or not? > > - rx_stats = per_cpu_ptr(vlan->rx_stats, smp_processor_id()); > > skb = skb_share_check(skb, GFP_ATOMIC); > > - if (skb == NULL) { > > - rx_stats->rx_errors++; > > - return NULL; > > - } > > - > > - rx_stats->rx_bytes += skb->len + ETH_HLEN; > > - rx_stats->rx_packets++; > > + macvlan_count_rx(vlan, skb->len + ETH_HLEN, likely(skb != NULL), 0); > > its not _likely_ that skb != NULL, its a fact :) > > -> macvlan_count_rx(vlan, skb->len + ETH_HLEN, true, false); I don't understand. Note how I removed the check for NULL above and the skb pointer may be the result of a failing skb_clone(). Looking at this again, I actually introduced a bug by calling netif_rx on a possibly NULL skb, I'll fix that. Thanks! Arnd <>< _______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/virtualization