MINOR: fd: pass the iocb and owner to fd_insert()
fd_insert() is currently called just after setting the owner and iocb,
but proceeding like this prevents the operation from being atomic and
requires a lock to protect the maxfd computation in another thread from
meeting an incompletely initialized FD and computing a wrong maxfd.
Fortunately for now all fdtab[].owner are set before calling fd_insert(),
and the first lock in fd_insert() enforces a memory barrier so the code
is safe.
This patch moves the initialization of the owner and iocb to fd_insert()
so that the function will be able to properly arrange its operations and
remain safe even when modified to become lockless. There's no other change
beyond the internal API.
diff --git a/include/proto/connection.h b/include/proto/connection.h
index 6411764..bc8d884 100644
--- a/include/proto/connection.h
+++ b/include/proto/connection.h
@@ -111,9 +111,7 @@
if (!conn_ctrl_ready(conn)) {
int fd = conn->handle.fd;
- fdtab[fd].owner = conn;
- fdtab[fd].iocb = conn_fd_handler;
- fd_insert(fd, tid_bit);
+ fd_insert(fd, conn, conn_fd_handler, tid_bit);
/* mark the fd as ready so as not to needlessly poll at the beginning */
fd_may_recv(fd);
fd_may_send(fd);
diff --git a/include/proto/fd.h b/include/proto/fd.h
index 0dd3e84..a7e70b7 100644
--- a/include/proto/fd.h
+++ b/include/proto/fd.h
@@ -393,9 +393,11 @@
}
/* Prepares <fd> for being polled */
-static inline void fd_insert(int fd, unsigned long thread_mask)
+static inline void fd_insert(int fd, void *owner, void (*iocb)(int fd), unsigned long thread_mask)
{
HA_SPIN_LOCK(FD_LOCK, &fdtab[fd].lock);
+ fdtab[fd].owner = owner;
+ fdtab[fd].iocb = iocb;
fdtab[fd].ev = 0;
fdtab[fd].update_mask &= ~tid_bit;
fdtab[fd].linger_risk = 0;
diff --git a/src/dns.c b/src/dns.c
index a957710..280bc15 100644
--- a/src/dns.c
+++ b/src/dns.c
@@ -193,9 +193,7 @@
/* Add the fd in the fd list and update its parameters */
dgram->t.sock.fd = fd;
- fdtab[fd].owner = dgram;
- fdtab[fd].iocb = dgram_fd_handler;
- fd_insert(fd, (unsigned long)-1);
+ fd_insert(fd, dgram, dgram_fd_handler, MAX_THREADS_MASK);
fd_want_recv(fd);
return 0;
}
diff --git a/src/haproxy.c b/src/haproxy.c
index 11849fe..f1a2fb9 100644
--- a/src/haproxy.c
+++ b/src/haproxy.c
@@ -2347,9 +2347,7 @@
return;
fcntl(mworker_pipe[0], F_SETFL, O_NONBLOCK);
- fdtab[mworker_pipe[0]].owner = mworker_pipe;
- fdtab[mworker_pipe[0]].iocb = mworker_pipe_handler;
- fd_insert(mworker_pipe[0], MAX_THREADS_MASK);
+ fd_insert(mworker_pipe[0], mworker_pipe, mworker_pipe_handler, MAX_THREADS_MASK);
fd_want_recv(mworker_pipe[0]);
}
diff --git a/src/hathreads.c b/src/hathreads.c
index fea4ffe..fd0eb68 100644
--- a/src/hathreads.c
+++ b/src/hathreads.c
@@ -48,10 +48,7 @@
rfd = threads_sync_pipe[0];
fcntl(rfd, F_SETFL, O_NONBLOCK);
-
- fdtab[rfd].owner = thread_sync_io_handler;
- fdtab[rfd].iocb = thread_sync_io_handler;
- fd_insert(rfd, MAX_THREADS_MASK);
+ fd_insert(rfd, thread_sync_io_handler, thread_sync_io_handler, MAX_THREADS_MASK);
all_threads_mask = mask;
return 0;
diff --git a/src/proto_tcp.c b/src/proto_tcp.c
index f4ff698..9a3689c 100644
--- a/src/proto_tcp.c
+++ b/src/proto_tcp.c
@@ -1105,12 +1105,9 @@
listener->fd = fd;
listener->state = LI_LISTEN;
- fdtab[fd].owner = listener; /* reference the listener instead of a task */
- fdtab[fd].iocb = listener->proto->accept;
- if (listener->bind_conf->bind_thread[relative_pid-1])
- fd_insert(fd, listener->bind_conf->bind_thread[relative_pid-1]);
- else
- fd_insert(fd, MAX_THREADS_MASK);
+ fd_insert(fd, listener, listener->proto->accept,
+ listener->bind_conf->bind_thread[relative_pid-1] ?
+ listener->bind_conf->bind_thread[relative_pid-1] : MAX_THREADS_MASK);
tcp_return:
if (msg && errlen) {
diff --git a/src/proto_uxst.c b/src/proto_uxst.c
index 754acd2..3ab637f 100644
--- a/src/proto_uxst.c
+++ b/src/proto_uxst.c
@@ -331,13 +331,10 @@
listener->fd = fd;
listener->state = LI_LISTEN;
- /* the function for the accept() event */
- fdtab[fd].iocb = listener->proto->accept;
- fdtab[fd].owner = listener; /* reference the listener instead of a task */
- if (listener->bind_conf->bind_thread[relative_pid-1])
- fd_insert(fd, listener->bind_conf->bind_thread[relative_pid-1]);
- else
- fd_insert(fd, MAX_THREADS_MASK);
+ fd_insert(fd, listener, listener->proto->accept,
+ listener->bind_conf->bind_thread[relative_pid-1] ?
+ listener->bind_conf->bind_thread[relative_pid-1] : MAX_THREADS_MASK);
+
return err;
err_rename:
diff --git a/src/ssl_sock.c b/src/ssl_sock.c
index aecf3dd..c2fa45f 100644
--- a/src/ssl_sock.c
+++ b/src/ssl_sock.c
@@ -488,7 +488,7 @@
*/
static void inline ssl_async_process_fds(struct connection *conn, SSL *ssl)
{
- OSSL_ASYNC_FD add_fd[32], afd;
+ OSSL_ASYNC_FD add_fd[32];
OSSL_ASYNC_FD del_fd[32];
size_t num_add_fds = 0;
size_t num_del_fds = 0;
@@ -509,10 +509,7 @@
/* We add new fds to the fdtab */
for (i=0 ; i < num_add_fds ; i++) {
- afd = add_fd[i];
- fdtab[afd].owner = conn;
- fdtab[afd].iocb = ssl_async_fd_handler;
- fd_insert(afd, tid_bit);
+ fd_insert(add_fd[i], conn, ssl_async_fd_handler, tid_bit);
}
num_add_fds = 0;