* [PATCH v2 net-next] tcp/dccp: Move initialisation of refcounted into if block.
@ 2020-03-24 11:33 ` Kuniyuki Iwashima
0 siblings, 0 replies; 4+ messages in thread
From: Kuniyuki Iwashima @ 2020-03-24 11:33 UTC (permalink / raw)
To: David S . Miller, Jakub Kicinski, Eric Dumazet, Gerrit Renker,
Alexey Kuznetsov, Hideaki YOSHIFUJI
Cc: dccp, netdev, Kuniyuki Iwashima, Kuniyuki Iwashima, osa-contribution-log
The refcounted seems to be initialised at most three times, but the
complier can optimize that and the refcounted is initialised only at once.
- __inet_lookup_skb() sets it true.
- skb_steal_sock() is false and __inet_lookup() sets it true.
- __inet_lookup_established() is false and __inet_lookup() sets it false.
The code is bit confusing, so this patch makes it more readable so that no
one doubt about the complier optimization.
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.co.jp>
---
include/net/inet6_hashtables.h | 11 +++++++----
include/net/inet_hashtables.h | 11 +++++++----
2 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/include/net/inet6_hashtables.h b/include/net/inet6_hashtables.h
index fe96bf247aac..9b6c97100d54 100644
--- a/include/net/inet6_hashtables.h
+++ b/include/net/inet6_hashtables.h
@@ -70,9 +70,11 @@ static inline struct sock *__inet6_lookup(struct net *net,
struct sock *sk = __inet6_lookup_established(net, hashinfo, saddr,
sport, daddr, hnum,
dif, sdif);
- *refcounted = true;
- if (sk)
+ if (sk) {
+ *refcounted = true;
return sk;
+ }
+
*refcounted = false;
return inet6_lookup_listener(net, hashinfo, skb, doff, saddr, sport,
daddr, hnum, dif, sdif);
@@ -87,9 +89,10 @@ static inline struct sock *__inet6_lookup_skb(struct inet_hashinfo *hashinfo,
{
struct sock *sk = skb_steal_sock(skb);
- *refcounted = true;
- if (sk)
+ if (sk) {
+ *refcounted = true;
return sk;
+ }
return __inet6_lookup(dev_net(skb_dst(skb)->dev), hashinfo, skb,
doff, &ipv6_hdr(skb)->saddr, sport,
diff --git a/include/net/inet_hashtables.h b/include/net/inet_hashtables.h
index d0019d3395cf..aa859bf8607b 100644
--- a/include/net/inet_hashtables.h
+++ b/include/net/inet_hashtables.h
@@ -345,9 +345,11 @@ static inline struct sock *__inet_lookup(struct net *net,
sk = __inet_lookup_established(net, hashinfo, saddr, sport,
daddr, hnum, dif, sdif);
- *refcounted = true;
- if (sk)
+ if (sk) {
+ *refcounted = true;
return sk;
+ }
+
*refcounted = false;
return __inet_lookup_listener(net, hashinfo, skb, doff, saddr,
sport, daddr, hnum, dif, sdif);
@@ -382,9 +384,10 @@ static inline struct sock *__inet_lookup_skb(struct inet_hashinfo *hashinfo,
struct sock *sk = skb_steal_sock(skb);
const struct iphdr *iph = ip_hdr(skb);
- *refcounted = true;
- if (sk)
+ if (sk) {
+ *refcounted = true;
return sk;
+ }
return __inet_lookup(dev_net(skb_dst(skb)->dev), hashinfo, skb,
doff, iph->saddr, sport,
--
2.17.2 (Apple Git-113)
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH v2 net-next] tcp/dccp: Move initialisation of refcounted into if block.
@ 2020-03-24 11:33 ` Kuniyuki Iwashima
0 siblings, 0 replies; 4+ messages in thread
From: Kuniyuki Iwashima @ 2020-03-24 11:33 UTC (permalink / raw)
To: dccp
The refcounted seems to be initialised at most three times, but the
complier can optimize that and the refcounted is initialised only at once.
- __inet_lookup_skb() sets it true.
- skb_steal_sock() is false and __inet_lookup() sets it true.
- __inet_lookup_established() is false and __inet_lookup() sets it false.
The code is bit confusing, so this patch makes it more readable so that no
one doubt about the complier optimization.
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.co.jp>
---
include/net/inet6_hashtables.h | 11 +++++++----
include/net/inet_hashtables.h | 11 +++++++----
2 files changed, 14 insertions(+), 8 deletions(-)
diff --git a/include/net/inet6_hashtables.h b/include/net/inet6_hashtables.h
index fe96bf247aac..9b6c97100d54 100644
--- a/include/net/inet6_hashtables.h
+++ b/include/net/inet6_hashtables.h
@@ -70,9 +70,11 @@ static inline struct sock *__inet6_lookup(struct net *net,
struct sock *sk = __inet6_lookup_established(net, hashinfo, saddr,
sport, daddr, hnum,
dif, sdif);
- *refcounted = true;
- if (sk)
+ if (sk) {
+ *refcounted = true;
return sk;
+ }
+
*refcounted = false;
return inet6_lookup_listener(net, hashinfo, skb, doff, saddr, sport,
daddr, hnum, dif, sdif);
@@ -87,9 +89,10 @@ static inline struct sock *__inet6_lookup_skb(struct inet_hashinfo *hashinfo,
{
struct sock *sk = skb_steal_sock(skb);
- *refcounted = true;
- if (sk)
+ if (sk) {
+ *refcounted = true;
return sk;
+ }
return __inet6_lookup(dev_net(skb_dst(skb)->dev), hashinfo, skb,
doff, &ipv6_hdr(skb)->saddr, sport,
diff --git a/include/net/inet_hashtables.h b/include/net/inet_hashtables.h
index d0019d3395cf..aa859bf8607b 100644
--- a/include/net/inet_hashtables.h
+++ b/include/net/inet_hashtables.h
@@ -345,9 +345,11 @@ static inline struct sock *__inet_lookup(struct net *net,
sk = __inet_lookup_established(net, hashinfo, saddr, sport,
daddr, hnum, dif, sdif);
- *refcounted = true;
- if (sk)
+ if (sk) {
+ *refcounted = true;
return sk;
+ }
+
*refcounted = false;
return __inet_lookup_listener(net, hashinfo, skb, doff, saddr,
sport, daddr, hnum, dif, sdif);
@@ -382,9 +384,10 @@ static inline struct sock *__inet_lookup_skb(struct inet_hashinfo *hashinfo,
struct sock *sk = skb_steal_sock(skb);
const struct iphdr *iph = ip_hdr(skb);
- *refcounted = true;
- if (sk)
+ if (sk) {
+ *refcounted = true;
return sk;
+ }
return __inet_lookup(dev_net(skb_dst(skb)->dev), hashinfo, skb,
doff, iph->saddr, sport,
--
2.17.2 (Apple Git-113)
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2 net-next] tcp/dccp: Move initialisation of refcounted into if block.
2020-03-24 11:33 ` Kuniyuki Iwashima
@ 2020-03-24 23:23 ` David Miller
-1 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2020-03-24 23:23 UTC (permalink / raw)
To: kuniyu
Cc: kuba, edumazet, gerrit, kuznet, yoshfuji, dccp, netdev, kuni1840,
osa-contribution-log
From: Kuniyuki Iwashima <kuniyu@amazon.co.jp>
Date: Tue, 24 Mar 2020 20:33:31 +0900
> The refcounted seems to be initialised at most three times, but the
> complier can optimize that and the refcounted is initialised only at once.
>
> - __inet_lookup_skb() sets it true.
> - skb_steal_sock() is false and __inet_lookup() sets it true.
> - __inet_lookup_established() is false and __inet_lookup() sets it false.
>
> The code is bit confusing, so this patch makes it more readable so that no
> one doubt about the complier optimization.
>
> Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.co.jp>
I don't think this improves the code readability nor code generation, it's
just noise changes and will make backporting more difficult.
I'm not applying this sorry.
In the future, I am going to apply a large threshold as to whether
changes like this are reasonable and appropriate. Please keep that
in mind for your submissions.
Thank you.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v2 net-next] tcp/dccp: Move initialisation of refcounted into if block.
@ 2020-03-24 23:23 ` David Miller
0 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2020-03-24 23:23 UTC (permalink / raw)
To: dccp
From: Kuniyuki Iwashima <kuniyu@amazon.co.jp>
Date: Tue, 24 Mar 2020 20:33:31 +0900
> The refcounted seems to be initialised at most three times, but the
> complier can optimize that and the refcounted is initialised only at once.
>
> - __inet_lookup_skb() sets it true.
> - skb_steal_sock() is false and __inet_lookup() sets it true.
> - __inet_lookup_established() is false and __inet_lookup() sets it false.
>
> The code is bit confusing, so this patch makes it more readable so that no
> one doubt about the complier optimization.
>
> Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.co.jp>
I don't think this improves the code readability nor code generation, it's
just noise changes and will make backporting more difficult.
I'm not applying this sorry.
In the future, I am going to apply a large threshold as to whether
changes like this are reasonable and appropriate. Please keep that
in mind for your submissions.
Thank you.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-03-24 23:23 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-24 11:33 [PATCH v2 net-next] tcp/dccp: Move initialisation of refcounted into if block Kuniyuki Iwashima
2020-03-24 11:33 ` Kuniyuki Iwashima
2020-03-24 23:23 ` David Miller
2020-03-24 23:23 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.