Message ID | 20170427145142.15830-9-marco.chiappero@intel.com |
---|---|
State | Changes Requested, archived |
Delegated to: | David Miller |
Headers | show |
> -----Original Message----- > From: Chiappero, Marco > Sent: Thursday, April 27, 2017 7:52 AM > To: netdev@vger.kernel.org > Cc: David S . Miller <davem@davemloft.net>; Kirsher, Jeffrey T > <jeffrey.t.kirsher@intel.com>; Duyck, Alexander H > <alexander.h.duyck@intel.com>; Grandhi, Sainath > <sainath.grandhi@intel.com>; Mahesh Bandewar <maheshb@google.com>; > Chiappero, Marco <marco.chiappero@intel.com> > Subject: [PATCH net-next 8/9] ipvlan: improve compiler hints > > Extend inlining and branch prediction hints. > > Signed-off-by: Marco Chiappero <marco.chiappero@intel.com> > --- > drivers/net/ipvlan/ipvlan_core.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/ipvlan/ipvlan_core.c b/drivers/net/ipvlan/ipvlan_core.c > index a9fc1b5..67e342d 100644 > --- a/drivers/net/ipvlan/ipvlan_core.c > +++ b/drivers/net/ipvlan/ipvlan_core.c > @@ -88,7 +88,7 @@ void ipvlan_ht_addr_del(struct ipvl_addr *addr) > hlist_del_init_rcu(&addr->hlnode); > } > > -unsigned int ipvlan_mac_hash(const unsigned char *addr) > +inline unsigned int ipvlan_mac_hash(const unsigned char *addr) > { > u32 hash = jhash_1word(__get_unaligned_cpu32(addr + 2), > ipvlan_jhash_secret); I'm kind of surprised this isn't causing a problem with differing declarations between the declaration here and the declaration in ipvlan.h. Normally for inlining something like this you would change it to a "static inline" and move the entire declaration into the header file. > @@ -505,7 +505,7 @@ static int ipvlan_xmit_mode_l2(struct sk_buff *skb, > struct net_device *dev) > return ipvlan_rcv_int_frame(addr, &skb); > > skb = skb_share_check(skb, GFP_ATOMIC); > - if (!skb) > + if (unlikely(!skb)) > return NET_XMIT_DROP; > > /* Packet definitely does not belong to any of the @@ -596,7 > +596,7 @@ static rx_handler_result_t ipvlan_handle_mode_l2(struct sk_buff > **pskb, > * when work-queue processes this frame. This is > * achieved by returning RX_HANDLER_PASS. > */ > - if (nskb) > + if (likely(nskb)) > ipvlan_multicast_enqueue(port, nskb, false); > } > } else { > -- > 2.9.3
From: Duyck, Alexander H > Sent: 27 April 2017 16:21 ... > > -unsigned int ipvlan_mac_hash(const unsigned char *addr) > > +inline unsigned int ipvlan_mac_hash(const unsigned char *addr) > > { > > u32 hash = jhash_1word(__get_unaligned_cpu32(addr + 2), > > ipvlan_jhash_secret); > > I'm kind of surprised this isn't causing a problem with differing declarations between the declaration > here and the declaration in ipvlan.h. Normally for inlining something like this you would change it to > a "static inline" and move the entire declaration into the header file. You get a callable copy for external callers and local calls inlined. Not usually what you want. David
From: "Duyck, Alexander H" <alexander.h.duyck@intel.com> Date: Thu, 27 Apr 2017 15:21:16 +0000 >> -----Original Message----- >> From: Chiappero, Marco >> Sent: Thursday, April 27, 2017 7:52 AM >> To: netdev@vger.kernel.org >> Cc: David S . Miller <davem@davemloft.net>; Kirsher, Jeffrey T >> <jeffrey.t.kirsher@intel.com>; Duyck, Alexander H >> <alexander.h.duyck@intel.com>; Grandhi, Sainath >> <sainath.grandhi@intel.com>; Mahesh Bandewar <maheshb@google.com>; >> Chiappero, Marco <marco.chiappero@intel.com> >> Subject: [PATCH net-next 8/9] ipvlan: improve compiler hints >> >> Extend inlining and branch prediction hints. >> >> Signed-off-by: Marco Chiappero <marco.chiappero@intel.com> >> --- >> drivers/net/ipvlan/ipvlan_core.c | 6 +++--- >> 1 file changed, 3 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/net/ipvlan/ipvlan_core.c b/drivers/net/ipvlan/ipvlan_core.c >> index a9fc1b5..67e342d 100644 >> --- a/drivers/net/ipvlan/ipvlan_core.c >> +++ b/drivers/net/ipvlan/ipvlan_core.c >> @@ -88,7 +88,7 @@ void ipvlan_ht_addr_del(struct ipvl_addr *addr) >> hlist_del_init_rcu(&addr->hlnode); >> } >> >> -unsigned int ipvlan_mac_hash(const unsigned char *addr) >> +inline unsigned int ipvlan_mac_hash(const unsigned char *addr) >> { >> u32 hash = jhash_1word(__get_unaligned_cpu32(addr + 2), >> ipvlan_jhash_secret); > > I'm kind of surprised this isn't causing a problem with differing > declarations between the declaration here and the declaration in > ipvlan.h. Normally for inlining something like this you would change > it to a "static inline" and move the entire declaration into the > header file. No inlines in foo.c files please, seriously let the compiler decide it knows better than you.
diff --git a/drivers/net/ipvlan/ipvlan_core.c b/drivers/net/ipvlan/ipvlan_core.c index a9fc1b5..67e342d 100644 --- a/drivers/net/ipvlan/ipvlan_core.c +++ b/drivers/net/ipvlan/ipvlan_core.c @@ -88,7 +88,7 @@ void ipvlan_ht_addr_del(struct ipvl_addr *addr) hlist_del_init_rcu(&addr->hlnode); } -unsigned int ipvlan_mac_hash(const unsigned char *addr) +inline unsigned int ipvlan_mac_hash(const unsigned char *addr) { u32 hash = jhash_1word(__get_unaligned_cpu32(addr + 2), ipvlan_jhash_secret); @@ -505,7 +505,7 @@ static int ipvlan_xmit_mode_l2(struct sk_buff *skb, struct net_device *dev) return ipvlan_rcv_int_frame(addr, &skb); skb = skb_share_check(skb, GFP_ATOMIC); - if (!skb) + if (unlikely(!skb)) return NET_XMIT_DROP; /* Packet definitely does not belong to any of the @@ -596,7 +596,7 @@ static rx_handler_result_t ipvlan_handle_mode_l2(struct sk_buff **pskb, * when work-queue processes this frame. This is * achieved by returning RX_HANDLER_PASS. */ - if (nskb) + if (likely(nskb)) ipvlan_multicast_enqueue(port, nskb, false); } } else {
Extend inlining and branch prediction hints. Signed-off-by: Marco Chiappero <marco.chiappero@intel.com> --- drivers/net/ipvlan/ipvlan_core.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)