Skip to content

Commit

Permalink
issue: 3844385 Fix new TCP timers registration lock contention
Browse files Browse the repository at this point in the history
Avoid calling register_socket_timer_event when a socket is already registered (TIME-WAIT).
Although there is no functionality issue with that, it produces too high rate of posting events for internal-thread.
This leads to lock contantion inside internal-thread and degraded performance of HTTP CPS.

Signed-off-by: Alexander Grissik <agrissik@nvidia.com>
  • Loading branch information
AlexanderGrissik authored and galnoam committed Apr 4, 2024
1 parent 6f485a1 commit ea38dd7
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 5 deletions.
17 changes: 12 additions & 5 deletions src/core/sock/sockinfo_tcp.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2482,10 +2482,15 @@ ssize_t sockinfo_tcp::rx(const rx_call_t call_type, iovec *p_iov, ssize_t sz_iov

void sockinfo_tcp::register_timer()
{
si_tcp_logdbg("Registering TCP socket timer: socket: %p, thread-col: %p, global-col: %p", this,
get_tcp_timer_collection(), g_tcp_timers_collection);
// A reused time-wait socket wil try to add a timer although it is already registered.
// We should avoid calling register_socket_timer_event unnecessarily because it introduces
// internal-thread locks contention.
if (!is_timer_registered()) {
si_tcp_logdbg("Registering TCP socket timer: socket: %p, thread-col: %p, global-col: %p",
this, get_tcp_timer_collection(), g_tcp_timers_collection);

get_event_mgr()->register_socket_timer_event(this);
get_event_mgr()->register_socket_timer_event(this);
}
}

void sockinfo_tcp::queue_rx_ctl_packet(struct tcp_pcb *pcb, mem_buf_desc_t *p_desc)
Expand Down Expand Up @@ -6017,15 +6022,16 @@ void tcp_timers_collection::add_new_timer(sockinfo_tcp *sock)
return;
}

// A reused time-wait socket wil try to add a timer although it is already registered.
if (m_sock_remove_map.find(sock) != m_sock_remove_map.end()) {
if (sock->is_timer_registered()) {
__log_warn("Trying to add timer twice for TCP socket %p", sock);
return;
}

sock_list &bucket = m_p_intervals[m_n_next_insert_bucket];
bucket.emplace_back(sock);
m_sock_remove_map.emplace(sock, std::make_tuple(m_n_next_insert_bucket, --(bucket.end())));
m_n_next_insert_bucket = (m_n_next_insert_bucket + 1) % m_n_intervals_size;
sock->set_timer_registered(true);

if (0 == m_n_count++) {
m_timer_handle = get_event_mgr()->register_timer_event(safe_mce_sys().timer_resolution_msec,
Expand All @@ -6041,6 +6047,7 @@ void tcp_timers_collection::remove_timer(sockinfo_tcp *sock)
if (node != m_sock_remove_map.end()) {
m_p_intervals[std::get<0>(node->second)].erase(std::get<1>(node->second));
m_sock_remove_map.erase(node);
sock->set_timer_registered(false);

if (!(--m_n_count)) {
if (m_timer_handle) {
Expand Down
3 changes: 3 additions & 0 deletions src/core/sock/sockinfo_tcp.h
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,8 @@ class sockinfo_tcp : public sockinfo {
}

bool is_incoming() override { return m_b_incoming; }
bool is_timer_registered() const { return m_timer_registered; }
void set_timer_registered(bool v) { m_timer_registered = v; }

bool is_connected() { return m_sock_state == TCP_SOCK_CONNECTED_RDWR; }

Expand Down Expand Up @@ -618,6 +620,7 @@ class sockinfo_tcp : public sockinfo {
bool m_xlio_thr;
bool m_b_incoming;
bool m_b_attached;
bool m_timer_registered = false;
/* connection state machine */
int m_conn_timeout;
/* RCVBUF acconting */
Expand Down

0 comments on commit ea38dd7

Please sign in to comment.