ip: do not set RFS core on error queue reads
Subsystems (4)
PATCH 1/2 ip: do not set RFS core on error queue reads
From: Soheil Hassas Yeganeh <[email protected]>
We should only record RPS on normal reads and writes. In single threaded processes, all calls record the same state. In multi-threaded processes where a separate thread processes errors, the RFS table mispredicts.
Note that, when CONFIG_RPS is disabled, sock_rps_record_flow is a noop and no branch is added as a result of this patch.
--- net/ipv4/af_inet.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
net/ipv4/af_inet.c +2 -1
@@ -790,7 +790,8 @@ int inet_recvmsg(struct socket *sock, struct msghdr *msg, size_t size,int addr_len =0 ;int err; -sock_rps_record_flow (sk); +if (likely (!(flags &MSG_ERRQUEUE ))) +sock_rps_record_flow (sk); err = sk->sk_prot->recvmsg (sk, msg, size, flags &MSG_DONTWAIT , flags & ~MSG_DONTWAIT , &addr_len);
Signature · 1 line
2.16.0.rc0.223.g4a4ac83678-goog
PATCH 2/2 net: revert "Update RFS target at poll for tcp/udp"
From: Soheil Hassas Yeganeh <[email protected]>
On multi-threaded processes, one common architecture is to have one (or a small number of) threads polling sockets, and a considerably larger pool of threads reading form and writing to the sockets. When we set RPS core on tcp_poll() or udp_poll() we essentially steer all packets of all the polled FDs to one (or small number of) cores, creaing a bottleneck and/or RPS misprediction.
Another common architecture is to shard FDs among threads pinned to cores. In such a setting, setting RPS core in tcp_poll() and udp_poll() is redundant because the RFS core is correctly set in recvmsg and sendmsg.
Thus, revert the following commit:c3f1dbaf6e28 ("net: Update RFS target at poll for tcp/udp").
--- net/ipv4/tcp.c | 2 -- net/ipv4/udp.c | 2 -- 2 files changed, 4 deletions(-)
net/ipv4/tcp.c +0 -2
@@ -498,8 +498,6 @@ unsigned int tcp_poll(struct file *file, struct socket *sock, poll_table *wait)const struct tcp_sock *tp =tcp_sk (sk);int state; -sock_rps_record_flow (sk); -sock_poll_wait (file,sk_sleep (sk), wait); state =inet_sk_state_load (sk);
net/ipv4/udp.c +0 -2
@@ -2490,8 +2490,6 @@ unsigned int udp_poll(struct file *file, struct socket *sock, poll_table *wait)if (!skb_queue_empty (&udp_sk (sk)->reader_queue)) mask |=POLLIN |POLLRDNORM ; -sock_rps_record_flow (sk); -/* Check for false positives due to checksum errors */ if ((mask &POLLRDNORM ) && !(file->f_flags &O_NONBLOCK ) && !(sk->sk_shutdown &RCV_SHUTDOWN ) &&first_packet_length (sk) ==-1 )
Signature · 1 line
2.16.0.rc0.223.g4a4ac83678-goog
Re: [PATCH net-next 2/2] net: revert "Update RFS target at poll for tcp/udp"
From: Soheil Hassas Yeganeh <[email protected]>
Date: Wed, 3 Jan 2018 21:47:11 -0500
Quoted 14 lines
From: Soheil Hassas Yeganeh <[email protected]>
On multi-threaded processes, one common architecture is to have one (or a small number of) threads polling sockets, and a considerably larger pool of threads reading form and writing to the sockets. When we set RPS core on
tcp_poll()orudp_poll()we essentially steer all packets of all the polled FDs to one (or small number of) cores, creaing a bottleneck and/or RPS misprediction.Another common architecture is to shard FDs among threads pinned to cores. In such a setting, setting RPS core in
tcp_poll()andudp_poll()is redundant because the RFS core is correctly set in recvmsg and sendmsg.
Quoted 7 lines
Thus, revert the following commit:
c3f1dbaf6e28("net: Update RFS target at poll for tcp/udp").Signed-off-by Soheil Hassas Yeganeh <[email protected]>Signed-off-by Willem de Bruijn <[email protected]>Signed-off-by Eric Dumazet <[email protected]>Signed-off-by Neal Cardwell <[email protected]>
Applied.
From: Soheil Hassas Yeganeh <[email protected]>
Date: Wed, 3 Jan 2018 21:47:10 -0500
Quoted 7 lines
From: Soheil Hassas Yeganeh <[email protected]>
We should only record RPS on normal reads and writes. In single threaded processes, all calls record the same state. In multi-threaded processes where a separate thread processes errors, the RFS table mispredicts.
Quoted 7 lines
Note that, when
CONFIG_RPSis disabled,sock_rps_record_flowis a noop and no branch is added as a result of this patch.Signed-off-by Soheil Hassas Yeganeh <[email protected]>Signed-off-by Willem de Bruijn <[email protected]>Signed-off-by Eric Dumazet <[email protected]>Signed-off-by Neal Cardwell <[email protected]>
Applied.