Commit cd7d8498c9a5d510c64db38d9f4f4fbc41790f09
Committed by
David S. Miller
1 parent
dc83d4d8f6
Exists in
ti-lsk-linux-4.1.y
and in
10 other branches
tcp: change tcp_skb_pcount() location
Our goal is to access no more than one cache line access per skb in a write or receive queue when doing the various walks. After recent TCP_SKB_CB() reorganizations, it is almost done. Last part is tcp_skb_pcount() which currently uses skb_shinfo(skb)->gso_segs, which is a terrible choice, because it needs 3 cache lines in current kernel (skb->head, skb->end, and shinfo->gso_segs are all in 3 different cache lines, far from skb->cb) This very simple patch reuses space currently taken by tcp_tw_isn only in input path, as tcp_skb_pcount is only needed for skb stored in write queue. This considerably speeds up tcp_ack(), granted we avoid shinfo->tx_flags to get SKBTX_ACK_TSTAMP, which seems possible. This also speeds up all sack processing in general. This speeds up tcp_sendmsg() because it no longer has to access/dirty shinfo. Signed-off-by: Eric Dumazet <edumazet@google.com> Signed-off-by: David S. Miller <davem@davemloft.net>
Showing 4 changed files with 33 additions and 11 deletions Side-by-side Diff
include/net/tcp.h
... | ... | @@ -698,7 +698,16 @@ |
698 | 698 | struct tcp_skb_cb { |
699 | 699 | __u32 seq; /* Starting sequence number */ |
700 | 700 | __u32 end_seq; /* SEQ + FIN + SYN + datalen */ |
701 | - __u32 tcp_tw_isn; /* isn chosen by tcp_timewait_state_process() */ | |
701 | + union { | |
702 | + /* Note : tcp_tw_isn is used in input path only | |
703 | + * (isn chosen by tcp_timewait_state_process()) | |
704 | + * | |
705 | + * tcp_gso_segs is used in write queue only, | |
706 | + * cf tcp_skb_pcount() | |
707 | + */ | |
708 | + __u32 tcp_tw_isn; | |
709 | + __u32 tcp_gso_segs; | |
710 | + }; | |
702 | 711 | __u8 tcp_flags; /* TCP header flags. (tcp[13]) */ |
703 | 712 | |
704 | 713 | __u8 sacked; /* State flags for SACK/FACK. */ |
... | ... | @@ -746,7 +755,17 @@ |
746 | 755 | */ |
747 | 756 | static inline int tcp_skb_pcount(const struct sk_buff *skb) |
748 | 757 | { |
749 | - return skb_shinfo(skb)->gso_segs; | |
758 | + return TCP_SKB_CB(skb)->tcp_gso_segs; | |
759 | +} | |
760 | + | |
761 | +static inline void tcp_skb_pcount_set(struct sk_buff *skb, int segs) | |
762 | +{ | |
763 | + TCP_SKB_CB(skb)->tcp_gso_segs = segs; | |
764 | +} | |
765 | + | |
766 | +static inline void tcp_skb_pcount_add(struct sk_buff *skb, int segs) | |
767 | +{ | |
768 | + TCP_SKB_CB(skb)->tcp_gso_segs += segs; | |
750 | 769 | } |
751 | 770 | |
752 | 771 | /* This is valid iff tcp_skb_pcount() > 1. */ |
net/ipv4/tcp.c
... | ... | @@ -963,7 +963,7 @@ |
963 | 963 | skb->ip_summed = CHECKSUM_PARTIAL; |
964 | 964 | tp->write_seq += copy; |
965 | 965 | TCP_SKB_CB(skb)->end_seq += copy; |
966 | - skb_shinfo(skb)->gso_segs = 0; | |
966 | + tcp_skb_pcount_set(skb, 0); | |
967 | 967 | |
968 | 968 | if (!copied) |
969 | 969 | TCP_SKB_CB(skb)->tcp_flags &= ~TCPHDR_PSH; |
... | ... | @@ -1261,7 +1261,7 @@ |
1261 | 1261 | |
1262 | 1262 | tp->write_seq += copy; |
1263 | 1263 | TCP_SKB_CB(skb)->end_seq += copy; |
1264 | - skb_shinfo(skb)->gso_segs = 0; | |
1264 | + tcp_skb_pcount_set(skb, 0); | |
1265 | 1265 | |
1266 | 1266 | from += copy; |
1267 | 1267 | copied += copy; |
net/ipv4/tcp_input.c
... | ... | @@ -1295,9 +1295,9 @@ |
1295 | 1295 | TCP_SKB_CB(prev)->end_seq += shifted; |
1296 | 1296 | TCP_SKB_CB(skb)->seq += shifted; |
1297 | 1297 | |
1298 | - skb_shinfo(prev)->gso_segs += pcount; | |
1299 | - BUG_ON(skb_shinfo(skb)->gso_segs < pcount); | |
1300 | - skb_shinfo(skb)->gso_segs -= pcount; | |
1298 | + tcp_skb_pcount_add(prev, pcount); | |
1299 | + BUG_ON(tcp_skb_pcount(skb) < pcount); | |
1300 | + tcp_skb_pcount_add(skb, -pcount); | |
1301 | 1301 | |
1302 | 1302 | /* When we're adding to gso_segs == 1, gso_size will be zero, |
1303 | 1303 | * in theory this shouldn't be necessary but as long as DSACK |
... | ... | @@ -1310,7 +1310,7 @@ |
1310 | 1310 | } |
1311 | 1311 | |
1312 | 1312 | /* CHECKME: To clear or not to clear? Mimics normal skb currently */ |
1313 | - if (skb_shinfo(skb)->gso_segs <= 1) { | |
1313 | + if (tcp_skb_pcount(skb) <= 1) { | |
1314 | 1314 | skb_shinfo(skb)->gso_size = 0; |
1315 | 1315 | skb_shinfo(skb)->gso_type = 0; |
1316 | 1316 | } |
net/ipv4/tcp_output.c
... | ... | @@ -384,7 +384,7 @@ |
384 | 384 | TCP_SKB_CB(skb)->tcp_flags = flags; |
385 | 385 | TCP_SKB_CB(skb)->sacked = 0; |
386 | 386 | |
387 | - shinfo->gso_segs = 1; | |
387 | + tcp_skb_pcount_set(skb, 1); | |
388 | 388 | shinfo->gso_size = 0; |
389 | 389 | shinfo->gso_type = 0; |
390 | 390 | |
... | ... | @@ -972,6 +972,9 @@ |
972 | 972 | TCP_ADD_STATS(sock_net(sk), TCP_MIB_OUTSEGS, |
973 | 973 | tcp_skb_pcount(skb)); |
974 | 974 | |
975 | + /* OK, its time to fill skb_shinfo(skb)->gso_segs */ | |
976 | + skb_shinfo(skb)->gso_segs = tcp_skb_pcount(skb); | |
977 | + | |
975 | 978 | /* Our usage of tstamp should remain private */ |
976 | 979 | skb->tstamp.tv64 = 0; |
977 | 980 | |
978 | 981 | |
... | ... | @@ -1019,11 +1022,11 @@ |
1019 | 1022 | /* Avoid the costly divide in the normal |
1020 | 1023 | * non-TSO case. |
1021 | 1024 | */ |
1022 | - shinfo->gso_segs = 1; | |
1025 | + tcp_skb_pcount_set(skb, 1); | |
1023 | 1026 | shinfo->gso_size = 0; |
1024 | 1027 | shinfo->gso_type = 0; |
1025 | 1028 | } else { |
1026 | - shinfo->gso_segs = DIV_ROUND_UP(skb->len, mss_now); | |
1029 | + tcp_skb_pcount_set(skb, DIV_ROUND_UP(skb->len, mss_now)); | |
1027 | 1030 | shinfo->gso_size = mss_now; |
1028 | 1031 | shinfo->gso_type = sk->sk_gso_type; |
1029 | 1032 | } |
-
mentioned in commit ad971f
-
mentioned in commit ad971f
-
mentioned in commit ad971f
-
mentioned in commit ad971f
-
mentioned in commit ad971f
-
mentioned in commit ad971f
-
mentioned in commit ad971f
-
mentioned in commit ad971f
-
mentioned in commit ad971f
-
mentioned in commit f69ad2
-
mentioned in commit c63264
-
mentioned in commit ad971f
-
mentioned in commit f69ad2
-
mentioned in commit c63264
-
mentioned in commit ad971f
-
mentioned in commit f69ad2
-
mentioned in commit c63264
-
mentioned in commit ad971f
-
mentioned in commit ad971f
-
mentioned in commit f69ad2
-
mentioned in commit c63264
-
mentioned in commit f69ad2
-
mentioned in commit c63264