Skip to content

Commit

Permalink
tcp: improve latencies of timer triggered events
Browse files Browse the repository at this point in the history 
Modern TCP stack highly depends on tcp_write_timer() having a small
latency, but current implementation doesn't exactly meet the
expectations.

When a timer fires but finds the socket is owned by the user, it rearms
itself for an additional delay hoping next run will be more
successful.

tcp_write_timer() for example uses a 50ms delay for next try, and it
defeats many attempts to get predictable TCP behavior in term of
latencies.

Use the recently introduced tcp_release_cb(), so that the user owning
the socket will call various handlers right before socket release.

This will permit us to post a followup patch to address the
tcp_tso_should_defer() syndrome (some deferred packets have to wait
RTO timer to be transmitted, while cwnd should allow us to send them
sooner)

Signed-off-by: Eric Dumazet <[email protected]>
Cc: Tom Herbert <[email protected]>
Cc: Yuchung Cheng <[email protected]>
Cc: Neal Cardwell <[email protected]>
Cc: Nandita Dukkipati <[email protected]>
Cc: H.K. Jerry Chu <[email protected]>
Cc: John Heffner <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
  • Loading branch information
@davem330
Eric Dumazet authored and davem330 committed Jul 20, 2012
1 parent 9dc2741 commit 6f458df
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 51 deletions.
4 changes: 3 additions & 1 deletion include/linux/tcp.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,9 @@ struct tcp_sock {
enum tsq_flags {
TSQ_THROTTLED,
TSQ_QUEUED,
TSQ_OWNED, /* tcp_tasklet_func() found socket was locked */
TCP_TSQ_DEFERRED, /* tcp_tasklet_func() found socket was owned */
TCP_WRITE_TIMER_DEFERRED, /* tcp_write_timer() found socket was owned */
TCP_DELACK_TIMER_DEFERRED, /* tcp_delack_timer() found socket was owned */
};

static inline struct tcp_sock *tcp_sk(const struct sock *sk)
Expand Down
2 changes: 2 additions & 0 deletions include/net/tcp.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,8 @@ extern int tcp_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
extern int tcp_sendpage(struct sock *sk, struct page *page, int offset,
size_t size, int flags);
extern void tcp_release_cb(struct sock *sk);
extern void tcp_write_timer_handler(struct sock *sk);
extern void tcp_delack_timer_handler(struct sock *sk);
extern int tcp_ioctl(struct sock *sk, int cmd, unsigned long arg);
extern int tcp_rcv_state_process(struct sock *sk, struct sk_buff *skb,
const struct tcphdr *th, unsigned int len);
Expand Down
46 changes: 29 additions & 17 deletions net/ipv4/tcp_output.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -837,6 +837,13 @@ struct tsq_tasklet {
};
static DEFINE_PER_CPU(struct tsq_tasklet, tsq_tasklet);

static void tcp_tsq_handler(struct sock *sk)
{
if ((1 << sk->sk_state) &
(TCPF_ESTABLISHED | TCPF_FIN_WAIT1 | TCPF_CLOSING |
TCPF_CLOSE_WAIT | TCPF_LAST_ACK))
tcp_write_xmit(sk, tcp_current_mss(sk), 0, 0, GFP_ATOMIC);
}
/*
* One tasklest per cpu tries to send more skbs.
* We run in tasklet context but need to disable irqs when
Expand Down Expand Up @@ -864,16 +871,10 @@ static void tcp_tasklet_func(unsigned long data)
bh_lock_sock(sk);

if (!sock_owned_by_user(sk)) {
if ((1 << sk->sk_state) &
(TCPF_ESTABLISHED | TCPF_FIN_WAIT1 |
TCPF_CLOSING | TCPF_CLOSE_WAIT | TCPF_LAST_ACK))
tcp_write_xmit(sk,
tcp_current_mss(sk),
0, 0,
GFP_ATOMIC);
tcp_tsq_handler(sk);
} else {
/* defer the work to tcp_release_cb() */
set_bit(TSQ_OWNED, &tp->tsq_flags);
set_bit(TCP_TSQ_DEFERRED, &tp->tsq_flags);
}
bh_unlock_sock(sk);

Expand All @@ -882,6 +883,9 @@ static void tcp_tasklet_func(unsigned long data)
}
}

#define TCP_DEFERRED_ALL ((1UL << TCP_TSQ_DEFERRED) | \
(1UL << TCP_WRITE_TIMER_DEFERRED) | \
(1UL << TCP_DELACK_TIMER_DEFERRED))
/**
* tcp_release_cb - tcp release_sock() callback
* @sk: socket
Expand All @@ -892,16 +896,24 @@ static void tcp_tasklet_func(unsigned long data)
void tcp_release_cb(struct sock *sk)
{
struct tcp_sock *tp = tcp_sk(sk);
unsigned long flags, nflags;

if (test_and_clear_bit(TSQ_OWNED, &tp->tsq_flags)) {
if ((1 << sk->sk_state) &
(TCPF_ESTABLISHED | TCPF_FIN_WAIT1 |
TCPF_CLOSING | TCPF_CLOSE_WAIT | TCPF_LAST_ACK))
tcp_write_xmit(sk,
tcp_current_mss(sk),
0, 0,
GFP_ATOMIC);
}
/* perform an atomic operation only if at least one flag is set */
do {
flags = tp->tsq_flags;
if (!(flags & TCP_DEFERRED_ALL))
return;
nflags = flags & ~TCP_DEFERRED_ALL;
} while (cmpxchg(&tp->tsq_flags, flags, nflags) != flags);

if (flags & (1UL << TCP_TSQ_DEFERRED))
tcp_tsq_handler(sk);

if (flags & (1UL << TCP_WRITE_TIMER_DEFERRED))
tcp_write_timer_handler(sk);

if (flags & (1UL << TCP_DELACK_TIMER_DEFERRED))
tcp_delack_timer_handler(sk);
}
EXPORT_SYMBOL(tcp_release_cb);

Expand Down
70 changes: 37 additions & 33 deletions net/ipv4/tcp_timer.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,17 +32,6 @@ int sysctl_tcp_retries2 __read_mostly = TCP_RETR2;
int sysctl_tcp_orphan_retries __read_mostly;
int sysctl_tcp_thin_linear_timeouts __read_mostly;

static void tcp_write_timer(unsigned long);
static void tcp_delack_timer(unsigned long);
static void tcp_keepalive_timer (unsigned long data);

void tcp_init_xmit_timers(struct sock *sk)
{
inet_csk_init_xmit_timers(sk, &tcp_write_timer, &tcp_delack_timer,
&tcp_keepalive_timer);
}
EXPORT_SYMBOL(tcp_init_xmit_timers);

static void tcp_write_err(struct sock *sk)
{
sk->sk_err = sk->sk_err_soft ? : ETIMEDOUT;
Expand Down Expand Up @@ -205,21 +194,11 @@ static int tcp_write_timeout(struct sock *sk)
return 0;
}

static void tcp_delack_timer(unsigned long data)
void tcp_delack_timer_handler(struct sock *sk)
{
struct sock *sk = (struct sock *)data;
struct tcp_sock *tp = tcp_sk(sk);
struct inet_connection_sock *icsk = inet_csk(sk);

bh_lock_sock(sk);
if (sock_owned_by_user(sk)) {
/* Try again later. */
icsk->icsk_ack.blocked = 1;
NET_INC_STATS_BH(sock_net(sk), LINUX_MIB_DELAYEDACKLOCKED);
sk_reset_timer(sk, &icsk->icsk_delack_timer, jiffies + TCP_DELACK_MIN);
goto out_unlock;
}

sk_mem_reclaim_partial(sk);

if (sk->sk_state == TCP_CLOSE || !(icsk->icsk_ack.pending & ICSK_ACK_TIMER))
Expand Down Expand Up @@ -260,7 +239,21 @@ static void tcp_delack_timer(unsigned long data)
out:
if (sk_under_memory_pressure(sk))
sk_mem_reclaim(sk);
out_unlock:
}

static void tcp_delack_timer(unsigned long data)
{
struct sock *sk = (struct sock *)data;

bh_lock_sock(sk);
if (!sock_owned_by_user(sk)) {
tcp_delack_timer_handler(sk);
} else {
inet_csk(sk)->icsk_ack.blocked = 1;
NET_INC_STATS_BH(sock_net(sk), LINUX_MIB_DELAYEDACKLOCKED);
/* deleguate our work to tcp_release_cb() */
set_bit(TCP_WRITE_TIMER_DEFERRED, &tcp_sk(sk)->tsq_flags);
}
bh_unlock_sock(sk);
sock_put(sk);
}
Expand Down Expand Up @@ -450,19 +443,11 @@ void tcp_retransmit_timer(struct sock *sk)
out:;
}

static void tcp_write_timer(unsigned long data)
void tcp_write_timer_handler(struct sock *sk)
{
struct sock *sk = (struct sock *)data;
struct inet_connection_sock *icsk = inet_csk(sk);
int event;

bh_lock_sock(sk);
if (sock_owned_by_user(sk)) {
/* Try again later */
sk_reset_timer(sk, &icsk->icsk_retransmit_timer, jiffies + (HZ / 20));
goto out_unlock;
}

if (sk->sk_state == TCP_CLOSE || !icsk->icsk_pending)
goto out;

Expand All @@ -485,7 +470,19 @@ static void tcp_write_timer(unsigned long data)

out:
sk_mem_reclaim(sk);
out_unlock:
}

static void tcp_write_timer(unsigned long data)
{
struct sock *sk = (struct sock *)data;

bh_lock_sock(sk);
if (!sock_owned_by_user(sk)) {
tcp_write_timer_handler(sk);
} else {
/* deleguate our work to tcp_release_cb() */
set_bit(TCP_WRITE_TIMER_DEFERRED, &tcp_sk(sk)->tsq_flags);
}
bh_unlock_sock(sk);
sock_put(sk);
}
Expand Down Expand Up @@ -602,3 +599,10 @@ static void tcp_keepalive_timer (unsigned long data)
bh_unlock_sock(sk);
sock_put(sk);
}

void tcp_init_xmit_timers(struct sock *sk)
{
inet_csk_init_xmit_timers(sk, &tcp_write_timer, &tcp_delack_timer,
&tcp_keepalive_timer);
}
EXPORT_SYMBOL(tcp_init_xmit_timers);

0 comments on commit 6f458df

Please sign in to comment.