| Message ID | 20110506.152623.232747437.davem@davemloft.net |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
Le vendredi 06 mai 2011 à 15:26 -0700, David Miller a écrit : > ip_setup_cork() explicitly initializes every member of > inet_cork except flags, addr, and opt. So we can simply > set those three members to zero instead of using a > memset() via an empty struct assignment. > > Signed-off-by: David S. Miller <davem@davemloft.net> > --- > net/ipv4/ip_output.c | 5 ++++- > 1 files changed, 4 insertions(+), 1 deletions(-) > > diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c > index eb0647a..5f5fe4f 100644 > --- a/net/ipv4/ip_output.c > +++ b/net/ipv4/ip_output.c > @@ -1408,7 +1408,7 @@ struct sk_buff *ip_make_skb(struct sock *sk, > struct ipcm_cookie *ipc, struct rtable **rtp, > unsigned int flags) > { > - struct inet_cork cork = {}; > + struct inet_cork cork; > struct sk_buff_head queue; > int err; > > @@ -1417,6 +1417,9 @@ struct sk_buff *ip_make_skb(struct sock *sk, > > __skb_queue_head_init(&queue); > > + cork.flags = 0; > + cork.addr = 0; > + cork.opt = 0; > err = ip_setup_cork(sk, &cork, ipc, rtp); > if (err) > return ERR_PTR(err); Very nice, thanks for finishing this stuff :) Acked-by: Eric Dumazet <eric.dumazet@gmail.com> -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Eric Dumazet <eric.dumazet@gmail.com> Date: Sat, 07 May 2011 00:35:17 +0200 > Very nice, thanks for finishing this stuff :) > > Acked-by: Eric Dumazet <eric.dumazet@gmail.com> No problem, thanks for reviewing :-) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2011-05-06 at 15:26 -0700, David Miller wrote: > ip_setup_cork() explicitly initializes every member of > inet_cork except flags, addr, and opt. So we can simply > set those three members to zero instead of using a > memset() via an empty struct assignment. > Signed-off-by: David S. Miller <davem@davemloft.net> > diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c [] > @@ -1417,6 +1417,9 @@ struct sk_buff *ip_make_skb(struct sock *sk, > > __skb_queue_head_init(&queue); > > + cork.flags = 0; > + cork.addr = 0; > + cork.opt = 0; cork.opt = NULL; > err = ip_setup_cork(sk, &cork, ipc, rtp); > if (err) > return ERR_PTR(err); Perhaps it'd be better to move the initialization of all cork fields to ip_setup_cork. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Joe Perches <joe@perches.com> Date: Fri, 06 May 2011 15:44:54 -0700 > Perhaps it'd be better to move the initialization > of all cork fields to ip_setup_cork. We can't, the flags and other fields can be setup way up high in the call chain when the inet->cork instance is used. I'll fix the NULL assignment, thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Le samedi 07 mai 2011 à 00:35 +0200, Eric Dumazet a écrit : > Le vendredi 06 mai 2011 à 15:26 -0700, David Miller a écrit : > > ip_setup_cork() explicitly initializes every member of > > inet_cork except flags, addr, and opt. So we can simply > > set those three members to zero instead of using a > > memset() via an empty struct assignment. > > > > Signed-off-by: David S. Miller <davem@davemloft.net> > > --- > > net/ipv4/ip_output.c | 5 ++++- > > 1 files changed, 4 insertions(+), 1 deletions(-) > > > > diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c > > index eb0647a..5f5fe4f 100644 > > --- a/net/ipv4/ip_output.c > > +++ b/net/ipv4/ip_output.c > > @@ -1408,7 +1408,7 @@ struct sk_buff *ip_make_skb(struct sock *sk, > > struct ipcm_cookie *ipc, struct rtable **rtp, > > unsigned int flags) > > { > > - struct inet_cork cork = {}; > > + struct inet_cork cork; > > struct sk_buff_head queue; > > int err; > > > > @@ -1417,6 +1417,9 @@ struct sk_buff *ip_make_skb(struct sock *sk, > > > > __skb_queue_head_init(&queue); > > > > + cork.flags = 0; > > + cork.addr = 0; > > + cork.opt = 0; > > err = ip_setup_cork(sk, &cork, ipc, rtp); > > if (err) > > return ERR_PTR(err); > > Very nice, thanks for finishing this stuff :) > > Acked-by: Eric Dumazet <eric.dumazet@gmail.com> > By the way, when I spotted this "struct inet_cork cork = {};" to be optimized, my idea was to add yet another case of fastpath to UDP send : For small datagrams (most UDP uses : RTP, DNS...), perform the user->kernel copy before route lookup, so that we can perform an RCU route lookup. This would tremendously speedup UDP, since the refcount handling is our last hot spot (not counting qdisc if present) PerfTop: 16142 irqs/sec kernel:97.5% exact: 0.0% [1000Hz cycles], (all, 16 CPUs) ----------------------------------------------------------------------------------------------------------- samples pcnt function DSO _______ _____ ________________________ ______________________ 16735.00 24.2% __ip_route_output_key vmlinux 9706.00 14.1% dst_release vmlinux 6754.00 9.8% __ip_make_skb vmlinux 5737.00 8.3% udp_send_skb vmlinux 5384.00 7.8% ip_finish_output vmlinux 3578.00 5.2% udp_sendmsg vmlinux 1435.00 2.1% copy_user_generic_string vmlinux 1358.00 2.0% ia32_sysenter_target vmlinux 1095.00 1.6% __ip_append_data vmlinux 832.00 1.2% kfree vmlinux 794.00 1.2% __memset vmlinux 677.00 1.0% fget_light vmlinux 641.00 0.9% sock_wfree vmlinux 637.00 0.9% dev_queue_xmit vmlinux -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Eric Dumazet <eric.dumazet@gmail.com> Date: Sat, 07 May 2011 07:11:44 +0200 > By the way, when I spotted this "struct inet_cork cork = {};" to be > optimized, my idea was to add yet another case of fastpath to UDP send : > > For small datagrams (most UDP uses : RTP, DNS...), > perform the user->kernel copy before route lookup, so that we can > perform an RCU route lookup. This would tremendously speedup UDP, since > the refcount handling is our last hot spot (not counting qdisc if > present) Interesting idea. This reminds me, remember about the input noref route lookup stuff going away with the routing cache removal? It turns out that when we do my "routes embedded in fib nexthop" for input, the noref stuff can be used. :) -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/ipv4/ip_output.c b/net/ipv4/ip_output.c index eb0647a..5f5fe4f 100644 --- a/net/ipv4/ip_output.c +++ b/net/ipv4/ip_output.c @@ -1408,7 +1408,7 @@ struct sk_buff *ip_make_skb(struct sock *sk, struct ipcm_cookie *ipc, struct rtable **rtp, unsigned int flags) { - struct inet_cork cork = {}; + struct inet_cork cork; struct sk_buff_head queue; int err; @@ -1417,6 +1417,9 @@ struct sk_buff *ip_make_skb(struct sock *sk, __skb_queue_head_init(&queue); + cork.flags = 0; + cork.addr = 0; + cork.opt = 0; err = ip_setup_cork(sk, &cork, ipc, rtp); if (err) return ERR_PTR(err);
ip_setup_cork() explicitly initializes every member of inet_cork except flags, addr, and opt. So we can simply set those three members to zero instead of using a memset() via an empty struct assignment. Signed-off-by: David S. Miller <davem@davemloft.net> --- net/ipv4/ip_output.c | 5 ++++- 1 files changed, 4 insertions(+), 1 deletions(-)