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.

include/proto/connection.h

@@ -111,9 +111,7 @@ static inline void conn_ctrl_init(struct connection *conn)
 	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);

include/proto/fd.h

@@ -393,9 +393,11 @@ static inline void fd_update_events(int fd, int evts)
 }
 
 /* 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;

src/dns.c

@@ -193,9 +193,7 @@ static int dns_connect_namesaver(struct dns_nameserver *ns)
 
 	/* 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;
 }

src/haproxy.c

@@ -2347,9 +2347,7 @@ void mworker_pipe_register()
 		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]);
 }
 

src/hathreads.c

@@ -48,10 +48,7 @@ int thread_sync_init(unsigned long mask)
 
 	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;

src/proto_tcp.c

@@ -1105,12 +1105,9 @@ int tcp_bind_listener(struct listener *listener, char *errmsg, int errlen)
 	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) {

src/proto_uxst.c

@@ -331,13 +331,10 @@ static int uxst_bind_listener(struct listener *listener, char *errmsg, int errle
 	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:

src/ssl_sock.c

@@ -488,7 +488,7 @@ static void ssl_async_fd_free(int fd)
  */
 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 @@ static void inline ssl_async_process_fds(struct connection *conn, SSL *ssl)
 
 	/* 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;