* [PATCH net-next] net: Drop unlikely before IS_ERR(_OR_NULL) [not found] <20190605142428.84784-1-wangkefeng.wang@huawei.com> @ 2019-06-05 14:24 ` Kefeng Wang 2019-06-05 16:13 ` Jesse Brandeburg 2019-06-05 16:40 ` Neil Horman 0 siblings, 2 replies; 5+ messages in thread From: Kefeng Wang @ 2019-06-05 14:24 UTC (permalink / raw) To: linux-kernel Cc: Kefeng Wang, David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, Vlad Yasevich, Neil Horman, Marcelo Ricardo Leitner, netdev, linux-sctp IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag, so no need to do that again from its callers. Drop it. Cc: "David S. Miller" <davem@davemloft.net> Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru> Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org> Cc: Vlad Yasevich <vyasevich@gmail.com> Cc: Neil Horman <nhorman@tuxdriver.com> Cc: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> Cc: netdev@vger.kernel.org Cc: linux-sctp@vger.kernel.org Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> --- include/net/udp.h | 2 +- net/ipv4/fib_semantics.c | 2 +- net/ipv4/inet_hashtables.c | 2 +- net/ipv4/udp.c | 2 +- net/ipv4/udp_offload.c | 2 +- net/ipv6/inet6_hashtables.c | 2 +- net/ipv6/udp.c | 2 +- net/sctp/socket.c | 4 ++-- 8 files changed, 9 insertions(+), 9 deletions(-) diff --git a/include/net/udp.h b/include/net/udp.h index 79d141d2103b..bad74f780831 100644 --- a/include/net/udp.h +++ b/include/net/udp.h @@ -480,7 +480,7 @@ static inline struct sk_buff *udp_rcv_segment(struct sock *sk, * CB fragment */ segs = __skb_gso_segment(skb, features, false); - if (unlikely(IS_ERR_OR_NULL(segs))) { + if (IS_ERR_OR_NULL(segs)) { int segs_nr = skb_shinfo(skb)->gso_segs; atomic_add(segs_nr, &sk->sk_drops); diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c index b80410673915..cd35bd0a4d8a 100644 --- a/net/ipv4/fib_semantics.c +++ b/net/ipv4/fib_semantics.c @@ -1295,7 +1295,7 @@ struct fib_info *fib_create_info(struct fib_config *cfg, goto failure; fi->fib_metrics = ip_fib_metrics_init(fi->fib_net, cfg->fc_mx, cfg->fc_mx_len, extack); - if (unlikely(IS_ERR(fi->fib_metrics))) { + if (IS_ERR(fi->fib_metrics)) { err = PTR_ERR(fi->fib_metrics); kfree(fi); return ERR_PTR(err); diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c index c4503073248b..97824864e40d 100644 --- a/net/ipv4/inet_hashtables.c +++ b/net/ipv4/inet_hashtables.c @@ -316,7 +316,7 @@ struct sock *__inet_lookup_listener(struct net *net, saddr, sport, htonl(INADDR_ANY), hnum, dif, sdif); done: - if (unlikely(IS_ERR(result))) + if (IS_ERR(result)) return NULL; return result; } diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c index 189144346cd4..8983afe2fe9e 100644 --- a/net/ipv4/udp.c +++ b/net/ipv4/udp.c @@ -478,7 +478,7 @@ struct sock *__udp4_lib_lookup(struct net *net, __be32 saddr, htonl(INADDR_ANY), hnum, dif, sdif, exact_dif, hslot2, skb); } - if (unlikely(IS_ERR(result))) + if (IS_ERR(result)) return NULL; return result; } diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c index 06b3e2c1fcdc..0112f64faf69 100644 --- a/net/ipv4/udp_offload.c +++ b/net/ipv4/udp_offload.c @@ -208,7 +208,7 @@ struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb, gso_skb->destructor = NULL; segs = skb_segment(gso_skb, features); - if (unlikely(IS_ERR_OR_NULL(segs))) { + if (IS_ERR_OR_NULL(segs)) { if (copy_dtor) gso_skb->destructor = sock_wfree; return segs; diff --git a/net/ipv6/inet6_hashtables.c b/net/ipv6/inet6_hashtables.c index b2a55f300318..cf60fae9533b 100644 --- a/net/ipv6/inet6_hashtables.c +++ b/net/ipv6/inet6_hashtables.c @@ -174,7 +174,7 @@ struct sock *inet6_lookup_listener(struct net *net, saddr, sport, &in6addr_any, hnum, dif, sdif); done: - if (unlikely(IS_ERR(result))) + if (IS_ERR(result)) return NULL; return result; } diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c index b3418a7c5c74..693518350f79 100644 --- a/net/ipv6/udp.c +++ b/net/ipv6/udp.c @@ -215,7 +215,7 @@ struct sock *__udp6_lib_lookup(struct net *net, exact_dif, hslot2, skb); } - if (unlikely(IS_ERR(result))) + if (IS_ERR(result)) return NULL; return result; } diff --git a/net/sctp/socket.c b/net/sctp/socket.c index 39ea0a37af09..c7b0f51c19d5 100644 --- a/net/sctp/socket.c +++ b/net/sctp/socket.c @@ -985,7 +985,7 @@ static int sctp_setsockopt_bindx(struct sock *sk, return -EINVAL; kaddrs = memdup_user(addrs, addrs_size); - if (unlikely(IS_ERR(kaddrs))) + if (IS_ERR(kaddrs)) return PTR_ERR(kaddrs); /* Walk through the addrs buffer and count the number of addresses. */ @@ -1315,7 +1315,7 @@ static int __sctp_setsockopt_connectx(struct sock *sk, return -EINVAL; kaddrs = memdup_user(addrs, addrs_size); - if (unlikely(IS_ERR(kaddrs))) + if (IS_ERR(kaddrs)) return PTR_ERR(kaddrs); /* Allow security module to validate connectx addresses. */ -- 2.20.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] net: Drop unlikely before IS_ERR(_OR_NULL) 2019-06-05 14:24 ` [PATCH net-next] net: Drop unlikely before IS_ERR(_OR_NULL) Kefeng Wang @ 2019-06-05 16:13 ` Jesse Brandeburg 2019-06-06 1:39 ` Kefeng Wang 2019-06-05 16:40 ` Neil Horman 1 sibling, 1 reply; 5+ messages in thread From: Jesse Brandeburg @ 2019-06-05 16:13 UTC (permalink / raw) To: Kefeng Wang Cc: linux-kernel, David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, Vlad Yasevich, Neil Horman, Marcelo Ricardo Leitner, netdev, linux-sctp, jesse.brandeburg On Wed, 5 Jun 2019 22:24:26 +0800 Kefeng wrote: > IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag, > so no need to do that again from its callers. Drop it. > <snip> > segs = __skb_gso_segment(skb, features, false); > - if (unlikely(IS_ERR_OR_NULL(segs))) { > + if (IS_ERR_OR_NULL(segs)) { > int segs_nr = skb_shinfo(skb)->gso_segs; > The change itself seems reasonable, but did you check to see if the paths changed are faster/slower with your fix? Did you look at any assembly output to see if the compiler actually generated different code? Is there a set of similar changes somewhere else in the kernel we can refer to? I'm not sure in the end that the change is worth it, so would like you to prove it is, unless davem overrides me. :-) ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] net: Drop unlikely before IS_ERR(_OR_NULL) 2019-06-05 16:13 ` Jesse Brandeburg @ 2019-06-06 1:39 ` Kefeng Wang 2019-06-06 20:41 ` Enrico Weigelt, metux IT consult 0 siblings, 1 reply; 5+ messages in thread From: Kefeng Wang @ 2019-06-06 1:39 UTC (permalink / raw) To: Jesse Brandeburg Cc: linux-kernel, David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, Vlad Yasevich, Neil Horman, Marcelo Ricardo Leitner, netdev, linux-sctp, Enrico Weigelt, metux IT consult On 2019/6/6 0:13, Jesse Brandeburg wrote: > On Wed, 5 Jun 2019 22:24:26 +0800 Kefeng wrote: >> IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag, >> so no need to do that again from its callers. Drop it. >> > <snip> > >> segs = __skb_gso_segment(skb, features, false); >> - if (unlikely(IS_ERR_OR_NULL(segs))) { >> + if (IS_ERR_OR_NULL(segs)) { >> int segs_nr = skb_shinfo(skb)->gso_segs; >> > The change itself seems reasonable, but did you check to see if the > paths changed are faster/slower with your fix? Did you look at any > assembly output to see if the compiler actually generated different > code? Is there a set of similar changes somewhere else in the kernel > we can refer to? +Enrico Weigelt There is no different in assembly output (only check the x86/arm64), and the Enrico Weigelt have finished a cocci script to do this cleanup. > > I'm not sure in the end that the change is worth it, so would like you > to prove it is, unless davem overrides me. :-) > > > . > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] net: Drop unlikely before IS_ERR(_OR_NULL) 2019-06-06 1:39 ` Kefeng Wang @ 2019-06-06 20:41 ` Enrico Weigelt, metux IT consult 0 siblings, 0 replies; 5+ messages in thread From: Enrico Weigelt, metux IT consult @ 2019-06-06 20:41 UTC (permalink / raw) To: Kefeng Wang, Jesse Brandeburg Cc: linux-kernel, David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, Vlad Yasevich, Neil Horman, Marcelo Ricardo Leitner, netdev, linux-sctp On 06.06.19 03:39, Kefeng Wang wrote: Hi folks, > There is no different in assembly output (only check the x86/arm64), and > the Enrico Weigelt have finished a cocci script to do this cleanup. I haven't compared the assembly output, just logically deduced from the macro. If I understand it correctly, the likely()/unlikely() macros just add a hint to the compiler, which branch it should optimize harder. No idea what the compiler's actually doing, but I believe its things like optimized shortcut evaluation and avoiding jumps to likely branches. >> I'm not sure in the end that the change is worth it, so would like you >> to prove it is, unless davem overrides me. :-) Depends on what you count as worthy ;-) This patch just makes a source a bit more compact / easier to read. But shouldn't have any actual consequence on the generated binary. --mtx -- Enrico Weigelt, metux IT consult Free software and Linux embedded engineering info@metux.net -- +49-151-27565287 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next] net: Drop unlikely before IS_ERR(_OR_NULL) 2019-06-05 14:24 ` [PATCH net-next] net: Drop unlikely before IS_ERR(_OR_NULL) Kefeng Wang 2019-06-05 16:13 ` Jesse Brandeburg @ 2019-06-05 16:40 ` Neil Horman 1 sibling, 0 replies; 5+ messages in thread From: Neil Horman @ 2019-06-05 16:40 UTC (permalink / raw) To: Kefeng Wang Cc: linux-kernel, David S. Miller, Alexey Kuznetsov, Hideaki YOSHIFUJI, Vlad Yasevich, Marcelo Ricardo Leitner, netdev, linux-sctp On Wed, Jun 05, 2019 at 10:24:26PM +0800, Kefeng Wang wrote: > IS_ERR(_OR_NULL) already contain an 'unlikely' compiler flag, > so no need to do that again from its callers. Drop it. > > Cc: "David S. Miller" <davem@davemloft.net> > Cc: Alexey Kuznetsov <kuznet@ms2.inr.ac.ru> > Cc: Hideaki YOSHIFUJI <yoshfuji@linux-ipv6.org> > Cc: Vlad Yasevich <vyasevich@gmail.com> > Cc: Neil Horman <nhorman@tuxdriver.com> > Cc: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> > Cc: netdev@vger.kernel.org > Cc: linux-sctp@vger.kernel.org > Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com> > --- > include/net/udp.h | 2 +- > net/ipv4/fib_semantics.c | 2 +- > net/ipv4/inet_hashtables.c | 2 +- > net/ipv4/udp.c | 2 +- > net/ipv4/udp_offload.c | 2 +- > net/ipv6/inet6_hashtables.c | 2 +- > net/ipv6/udp.c | 2 +- > net/sctp/socket.c | 4 ++-- > 8 files changed, 9 insertions(+), 9 deletions(-) > > diff --git a/include/net/udp.h b/include/net/udp.h > index 79d141d2103b..bad74f780831 100644 > --- a/include/net/udp.h > +++ b/include/net/udp.h > @@ -480,7 +480,7 @@ static inline struct sk_buff *udp_rcv_segment(struct sock *sk, > * CB fragment > */ > segs = __skb_gso_segment(skb, features, false); > - if (unlikely(IS_ERR_OR_NULL(segs))) { > + if (IS_ERR_OR_NULL(segs)) { > int segs_nr = skb_shinfo(skb)->gso_segs; > > atomic_add(segs_nr, &sk->sk_drops); > diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c > index b80410673915..cd35bd0a4d8a 100644 > --- a/net/ipv4/fib_semantics.c > +++ b/net/ipv4/fib_semantics.c > @@ -1295,7 +1295,7 @@ struct fib_info *fib_create_info(struct fib_config *cfg, > goto failure; > fi->fib_metrics = ip_fib_metrics_init(fi->fib_net, cfg->fc_mx, > cfg->fc_mx_len, extack); > - if (unlikely(IS_ERR(fi->fib_metrics))) { > + if (IS_ERR(fi->fib_metrics)) { > err = PTR_ERR(fi->fib_metrics); > kfree(fi); > return ERR_PTR(err); > diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c > index c4503073248b..97824864e40d 100644 > --- a/net/ipv4/inet_hashtables.c > +++ b/net/ipv4/inet_hashtables.c > @@ -316,7 +316,7 @@ struct sock *__inet_lookup_listener(struct net *net, > saddr, sport, htonl(INADDR_ANY), hnum, > dif, sdif); > done: > - if (unlikely(IS_ERR(result))) > + if (IS_ERR(result)) > return NULL; > return result; > } > diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c > index 189144346cd4..8983afe2fe9e 100644 > --- a/net/ipv4/udp.c > +++ b/net/ipv4/udp.c > @@ -478,7 +478,7 @@ struct sock *__udp4_lib_lookup(struct net *net, __be32 saddr, > htonl(INADDR_ANY), hnum, dif, sdif, > exact_dif, hslot2, skb); > } > - if (unlikely(IS_ERR(result))) > + if (IS_ERR(result)) > return NULL; > return result; > } > diff --git a/net/ipv4/udp_offload.c b/net/ipv4/udp_offload.c > index 06b3e2c1fcdc..0112f64faf69 100644 > --- a/net/ipv4/udp_offload.c > +++ b/net/ipv4/udp_offload.c > @@ -208,7 +208,7 @@ struct sk_buff *__udp_gso_segment(struct sk_buff *gso_skb, > gso_skb->destructor = NULL; > > segs = skb_segment(gso_skb, features); > - if (unlikely(IS_ERR_OR_NULL(segs))) { > + if (IS_ERR_OR_NULL(segs)) { > if (copy_dtor) > gso_skb->destructor = sock_wfree; > return segs; > diff --git a/net/ipv6/inet6_hashtables.c b/net/ipv6/inet6_hashtables.c > index b2a55f300318..cf60fae9533b 100644 > --- a/net/ipv6/inet6_hashtables.c > +++ b/net/ipv6/inet6_hashtables.c > @@ -174,7 +174,7 @@ struct sock *inet6_lookup_listener(struct net *net, > saddr, sport, &in6addr_any, hnum, > dif, sdif); > done: > - if (unlikely(IS_ERR(result))) > + if (IS_ERR(result)) > return NULL; > return result; > } > diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c > index b3418a7c5c74..693518350f79 100644 > --- a/net/ipv6/udp.c > +++ b/net/ipv6/udp.c > @@ -215,7 +215,7 @@ struct sock *__udp6_lib_lookup(struct net *net, > exact_dif, hslot2, > skb); > } > - if (unlikely(IS_ERR(result))) > + if (IS_ERR(result)) > return NULL; > return result; > } > diff --git a/net/sctp/socket.c b/net/sctp/socket.c > index 39ea0a37af09..c7b0f51c19d5 100644 > --- a/net/sctp/socket.c > +++ b/net/sctp/socket.c > @@ -985,7 +985,7 @@ static int sctp_setsockopt_bindx(struct sock *sk, > return -EINVAL; > > kaddrs = memdup_user(addrs, addrs_size); > - if (unlikely(IS_ERR(kaddrs))) > + if (IS_ERR(kaddrs)) > return PTR_ERR(kaddrs); > > /* Walk through the addrs buffer and count the number of addresses. */ > @@ -1315,7 +1315,7 @@ static int __sctp_setsockopt_connectx(struct sock *sk, > return -EINVAL; > > kaddrs = memdup_user(addrs, addrs_size); > - if (unlikely(IS_ERR(kaddrs))) > + if (IS_ERR(kaddrs)) > return PTR_ERR(kaddrs); > > /* Allow security module to validate connectx addresses. */ > -- > 2.20.1 > > for the SCTP bits Acked-by: Neil Horman <nhorman@tuxdriver.com> ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2019-06-06 20:42 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20190605142428.84784-1-wangkefeng.wang@huawei.com>
2019-06-05 14:24 ` [PATCH net-next] net: Drop unlikely before IS_ERR(_OR_NULL) Kefeng Wang
2019-06-05 16:13 ` Jesse Brandeburg
2019-06-06 1:39 ` Kefeng Wang
2019-06-06 20:41 ` Enrico Weigelt, metux IT consult
2019-06-05 16:40 ` Neil Horman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).