Skip to content

Commit 2f65168

Browse files
kuba-mookuba-moo
committed
Merge branch 'tcp-add-fast-path-in-timer-handlers'
Eric Dumazet says: ==================== tcp: add fast path in timer handlers As mentioned in Netconf 2024: TCP retransmit and delack timers are not stopped from inet_csk_clear_xmit_timer() because we do not define INET_CSK_CLEAR_TIMERS. Enabling INET_CSK_CLEAR_TIMERS leads to lower performance, mainly because del_timer() and mod_timer() happen from different cpus quite often. What we can do instead is to add fast paths to tcp_write_timer() and tcp_delack_timer() to avoid socket spinlock acquisition. ==================== Link: https://patch.msgid.link/20241002173042.917928-1-edumazet@google.com Signed-off-by: Jakub Kicinski <kuba@kernel.org>
2 parents d454184 + 81df4fa commit 2f65168

File tree

8 files changed

+49
-24
lines changed

8 files changed

+49
-24
lines changed

‎include/net/inet_connection_sock.h‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -197,12 +197,12 @@ static inline void inet_csk_clear_xmit_timer(struct sock *sk, const int what)
197197
struct inet_connection_sock *icsk = inet_csk(sk);
198198

199199
if (what == ICSK_TIME_RETRANS || what == ICSK_TIME_PROBE0) {
200-
icsk->icsk_pending = 0;
200+
smp_store_release(&icsk->icsk_pending, 0);
201201
#ifdef INET_CSK_CLEAR_TIMERS
202202
sk_stop_timer(sk, &icsk->icsk_retransmit_timer);
203203
#endif
204204
} else if (what == ICSK_TIME_DACK) {
205-
icsk->icsk_ack.pending = 0;
205+
smp_store_release(&icsk->icsk_ack.pending, 0);
206206
icsk->icsk_ack.retry = 0;
207207
#ifdef INET_CSK_CLEAR_TIMERS
208208
sk_stop_timer(sk, &icsk->icsk_delack_timer);
@@ -229,11 +229,12 @@ static inline void inet_csk_reset_xmit_timer(struct sock *sk, const int what,
229229

230230
if (what == ICSK_TIME_RETRANS || what == ICSK_TIME_PROBE0 ||
231231
what == ICSK_TIME_LOSS_PROBE || what == ICSK_TIME_REO_TIMEOUT) {
232-
icsk->icsk_pending = what;
232+
smp_store_release(&icsk->icsk_pending, what);
233233
icsk->icsk_timeout = jiffies + when;
234234
sk_reset_timer(sk, &icsk->icsk_retransmit_timer, icsk->icsk_timeout);
235235
} else if (what == ICSK_TIME_DACK) {
236-
icsk->icsk_ack.pending |= ICSK_ACK_TIMER;
236+
smp_store_release(&icsk->icsk_ack.pending,
237+
icsk->icsk_ack.pending | ICSK_ACK_TIMER);
237238
icsk->icsk_ack.timeout = jiffies + when;
238239
sk_reset_timer(sk, &icsk->icsk_delack_timer, icsk->icsk_ack.timeout);
239240
} else {

‎net/ipv4/inet_connection_sock.c‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -775,7 +775,8 @@ void inet_csk_clear_xmit_timers(struct sock *sk)
775775
{
776776
struct inet_connection_sock *icsk = inet_csk(sk);
777777

778-
icsk->icsk_pending = icsk->icsk_ack.pending = 0;
778+
smp_store_release(&icsk->icsk_pending, 0);
779+
smp_store_release(&icsk->icsk_ack.pending, 0);
779780

780781
sk_stop_timer(sk, &icsk->icsk_retransmit_timer);
781782
sk_stop_timer(sk, &icsk->icsk_delack_timer);
@@ -790,7 +791,8 @@ void inet_csk_clear_xmit_timers_sync(struct sock *sk)
790791
/* ongoing timer handlers need to acquire socket lock. */
791792
sock_not_owned_by_me(sk);
792793

793-
icsk->icsk_pending = icsk->icsk_ack.pending = 0;
794+
smp_store_release(&icsk->icsk_pending, 0);
795+
smp_store_release(&icsk->icsk_ack.pending, 0);
794796

795797
sk_stop_timer_sync(sk, &icsk->icsk_retransmit_timer);
796798
sk_stop_timer_sync(sk, &icsk->icsk_delack_timer);

‎net/ipv4/inet_diag.c‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,7 @@ int inet_sk_diag_fill(struct sock *sk, struct inet_connection_sock *icsk,
247247
struct nlmsghdr *nlh;
248248
struct nlattr *attr;
249249
void *info = NULL;
250+
u8 icsk_pending;
250251
int protocol;
251252

252253
cb_data = cb->data;
@@ -307,14 +308,15 @@ int inet_sk_diag_fill(struct sock *sk, struct inet_connection_sock *icsk,
307308
goto out;
308309
}
309310

310-
if (icsk->icsk_pending == ICSK_TIME_RETRANS ||
311-
icsk->icsk_pending == ICSK_TIME_REO_TIMEOUT ||
312-
icsk->icsk_pending == ICSK_TIME_LOSS_PROBE) {
311+
icsk_pending = smp_load_acquire(&icsk->icsk_pending);
312+
if (icsk_pending == ICSK_TIME_RETRANS ||
313+
icsk_pending == ICSK_TIME_REO_TIMEOUT ||
314+
icsk_pending == ICSK_TIME_LOSS_PROBE) {
313315
r->idiag_timer = 1;
314316
r->idiag_retrans = icsk->icsk_retransmits;
315317
r->idiag_expires =
316318
jiffies_delta_to_msecs(icsk->icsk_timeout - jiffies);
317-
} else if (icsk->icsk_pending == ICSK_TIME_PROBE0) {
319+
} else if (icsk_pending == ICSK_TIME_PROBE0) {
318320
r->idiag_timer = 4;
319321
r->idiag_retrans = icsk->icsk_probes_out;
320322
r->idiag_expires =

‎net/ipv4/tcp_ipv4.c‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2900,15 +2900,17 @@ static void get_tcp4_sock(struct sock *sk, struct seq_file *f, int i)
29002900
__be32 src = inet->inet_rcv_saddr;
29012901
__u16 destp = ntohs(inet->inet_dport);
29022902
__u16 srcp = ntohs(inet->inet_sport);
2903+
u8 icsk_pending;
29032904
int rx_queue;
29042905
int state;
29052906

2906-
if (icsk->icsk_pending == ICSK_TIME_RETRANS ||
2907-
icsk->icsk_pending == ICSK_TIME_REO_TIMEOUT ||
2908-
icsk->icsk_pending == ICSK_TIME_LOSS_PROBE) {
2907+
icsk_pending = smp_load_acquire(&icsk->icsk_pending);
2908+
if (icsk_pending == ICSK_TIME_RETRANS ||
2909+
icsk_pending == ICSK_TIME_REO_TIMEOUT ||
2910+
icsk_pending == ICSK_TIME_LOSS_PROBE) {
29092911
timer_active = 1;
29102912
timer_expires = icsk->icsk_timeout;
2911-
} else if (icsk->icsk_pending == ICSK_TIME_PROBE0) {
2913+
} else if (icsk_pending == ICSK_TIME_PROBE0) {
29122914
timer_active = 4;
29132915
timer_expires = icsk->icsk_timeout;
29142916
} else if (timer_pending(&sk->sk_timer)) {

‎net/ipv4/tcp_output.c‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2960,7 +2960,7 @@ void tcp_send_loss_probe(struct sock *sk)
29602960
WARN_ONCE(tp->packets_out,
29612961
"invalid inflight: %u state %u cwnd %u mss %d\n",
29622962
tp->packets_out, sk->sk_state, tcp_snd_cwnd(tp), mss);
2963-
inet_csk(sk)->icsk_pending = 0;
2963+
smp_store_release(&inet_csk(sk)->icsk_pending, 0);
29642964
return;
29652965
}
29662966

@@ -2993,7 +2993,7 @@ void tcp_send_loss_probe(struct sock *sk)
29932993

29942994
NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPLOSSPROBES);
29952995
/* Reset s.t. tcp_rearm_rto will restart timer from now */
2996-
inet_csk(sk)->icsk_pending = 0;
2996+
smp_store_release(&inet_csk(sk)->icsk_pending, 0);
29972997
rearm_timer:
29982998
tcp_rearm_rto(sk);
29992999
}
@@ -4224,7 +4224,8 @@ void tcp_send_delayed_ack(struct sock *sk)
42244224
if (!time_before(timeout, icsk->icsk_ack.timeout))
42254225
timeout = icsk->icsk_ack.timeout;
42264226
}
4227-
icsk->icsk_ack.pending |= ICSK_ACK_SCHED | ICSK_ACK_TIMER;
4227+
smp_store_release(&icsk->icsk_ack.pending,
4228+
icsk->icsk_ack.pending | ICSK_ACK_SCHED | ICSK_ACK_TIMER);
42284229
icsk->icsk_ack.timeout = timeout;
42294230
sk_reset_timer(sk, &icsk->icsk_delack_timer, timeout);
42304231
}

‎net/ipv4/tcp_timer.c‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -361,6 +361,14 @@ static void tcp_delack_timer(struct timer_list *t)
361361
from_timer(icsk, t, icsk_delack_timer);
362362
struct sock *sk = &icsk->icsk_inet.sk;
363363

364+
/* Avoid taking socket spinlock if there is no ACK to send.
365+
* The compressed_ack check is racy, but a separate hrtimer
366+
* will take care of it eventually.
367+
*/
368+
if (!(smp_load_acquire(&icsk->icsk_ack.pending) & ICSK_ACK_TIMER) &&
369+
!READ_ONCE(tcp_sk(sk)->compressed_ack))
370+
goto out;
371+
364372
bh_lock_sock(sk);
365373
if (!sock_owned_by_user(sk)) {
366374
tcp_delack_timer_handler(sk);
@@ -371,6 +379,7 @@ static void tcp_delack_timer(struct timer_list *t)
371379
sock_hold(sk);
372380
}
373381
bh_unlock_sock(sk);
382+
out:
374383
sock_put(sk);
375384
}
376385

@@ -701,11 +710,11 @@ void tcp_write_timer_handler(struct sock *sk)
701710
tcp_send_loss_probe(sk);
702711
break;
703712
case ICSK_TIME_RETRANS:
704-
icsk->icsk_pending = 0;
713+
smp_store_release(&icsk->icsk_pending, 0);
705714
tcp_retransmit_timer(sk);
706715
break;
707716
case ICSK_TIME_PROBE0:
708-
icsk->icsk_pending = 0;
717+
smp_store_release(&icsk->icsk_pending, 0);
709718
tcp_probe_timer(sk);
710719
break;
711720
}
@@ -717,6 +726,10 @@ static void tcp_write_timer(struct timer_list *t)
717726
from_timer(icsk, t, icsk_retransmit_timer);
718727
struct sock *sk = &icsk->icsk_inet.sk;
719728

729+
/* Avoid locking the socket when there is no pending event. */
730+
if (!smp_load_acquire(&icsk->icsk_pending))
731+
goto out;
732+
720733
bh_lock_sock(sk);
721734
if (!sock_owned_by_user(sk)) {
722735
tcp_write_timer_handler(sk);
@@ -726,6 +739,7 @@ static void tcp_write_timer(struct timer_list *t)
726739
sock_hold(sk);
727740
}
728741
bh_unlock_sock(sk);
742+
out:
729743
sock_put(sk);
730744
}
731745

‎net/ipv6/tcp_ipv6.c‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2177,6 +2177,7 @@ static void get_tcp6_sock(struct seq_file *seq, struct sock *sp, int i)
21772177
const struct tcp_sock *tp = tcp_sk(sp);
21782178
const struct inet_connection_sock *icsk = inet_csk(sp);
21792179
const struct fastopen_queue *fastopenq = &icsk->icsk_accept_queue.fastopenq;
2180+
u8 icsk_pending;
21802181
int rx_queue;
21812182
int state;
21822183

@@ -2185,12 +2186,13 @@ static void get_tcp6_sock(struct seq_file *seq, struct sock *sp, int i)
21852186
destp = ntohs(inet->inet_dport);
21862187
srcp = ntohs(inet->inet_sport);
21872188

2188-
if (icsk->icsk_pending == ICSK_TIME_RETRANS ||
2189-
icsk->icsk_pending == ICSK_TIME_REO_TIMEOUT ||
2190-
icsk->icsk_pending == ICSK_TIME_LOSS_PROBE) {
2189+
icsk_pending = smp_load_acquire(&icsk->icsk_pending);
2190+
if (icsk_pending == ICSK_TIME_RETRANS ||
2191+
icsk_pending == ICSK_TIME_REO_TIMEOUT ||
2192+
icsk_pending == ICSK_TIME_LOSS_PROBE) {
21912193
timer_active = 1;
21922194
timer_expires = icsk->icsk_timeout;
2193-
} else if (icsk->icsk_pending == ICSK_TIME_PROBE0) {
2195+
} else if (icsk_pending == ICSK_TIME_PROBE0) {
21942196
timer_active = 4;
21952197
timer_expires = icsk->icsk_timeout;
21962198
} else if (timer_pending(&sp->sk_timer)) {

‎net/mptcp/protocol.c‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3504,7 +3504,8 @@ static void schedule_3rdack_retransmission(struct sock *ssk)
35043504
timeout += jiffies;
35053505

35063506
WARN_ON_ONCE(icsk->icsk_ack.pending & ICSK_ACK_TIMER);
3507-
icsk->icsk_ack.pending |= ICSK_ACK_SCHED | ICSK_ACK_TIMER;
3507+
smp_store_release(&icsk->icsk_ack.pending,
3508+
icsk->icsk_ack.pending | ICSK_ACK_SCHED | ICSK_ACK_TIMER);
35083509
icsk->icsk_ack.timeout = timeout;
35093510
sk_reset_timer(ssk, &icsk->icsk_delack_timer, timeout);
35103511
}

0 commit comments

Comments
 (0)