From ddedc1662487600d5124cb6d4972396438b9953c Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Thu, 7 Jul 2022 15:05:55 +0200 Subject: [PATCH] MEDIUM: fd: make fd_insert/fd_delete atomically update fd.tgid These functions need to set/reset the FD's tgid but when they're called there may still be wakeups on other threads that discover late updates and have to touch the tgid at the same time. As such, it is not possible to just read/write the tgid there. It must only be done using operations that are compatible with what other threads may be doing. As we're using inc/dec on the refcount, it's safe to AND the area to zero the lower part when resetting the value. However, in order to set the value, there's no other choice but fd_claim_tgid() which will assign it only if possible (via a CAS). This is convenient in the end because it protects the FD's masks from being modified by late threads, so while we hold this refcount we can safely reset the thread_mask and a few other elements. A debug test for non-null masks was added to fd_insert() as it must not be possible to face this situation thanks to the protection offered by the tgid. --- include/haproxy/fd.h | 20 +++++++++++++------- src/fd.c | 2 +- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/include/haproxy/fd.h b/include/haproxy/fd.h index 8c558189d6..6319484d6b 100644 --- a/include/haproxy/fd.h +++ b/include/haproxy/fd.h @@ -422,6 +422,12 @@ static inline long fd_clr_running(int fd) static inline void fd_insert(int fd, void *owner, void (*iocb)(int fd), int tgid, unsigned long thread_mask) { extern void sock_conn_iocb(int); + int newstate; + + /* conn_fd_handler should support edge-triggered FDs */ + newstate = 0; + if ((global.tune.options & GTUNE_FD_ET) && iocb == sock_conn_iocb) + newstate |= FD_ET_POSSIBLE; /* This must never happen and would definitely indicate a bug, in * addition to overwriting some unexpected memory areas. @@ -429,24 +435,24 @@ static inline void fd_insert(int fd, void *owner, void (*iocb)(int fd), int tgid BUG_ON(fd < 0 || fd >= global.maxsock); BUG_ON(fdtab[fd].owner != NULL); BUG_ON(fdtab[fd].state != 0); - BUG_ON(fdtab[fd].refc_tgid != 0); thread_mask &= tg->threads_enabled; BUG_ON(thread_mask == 0); + fd_claim_tgid(fd, tgid); + + BUG_ON(fdtab[fd].running_mask); + fdtab[fd].owner = owner; fdtab[fd].iocb = iocb; fdtab[fd].state = 0; - fdtab[fd].refc_tgid = tgid; + fdtab[fd].thread_mask = thread_mask; + fd_drop_tgid(fd); + #ifdef DEBUG_FD fdtab[fd].event_count = 0; #endif - /* conn_fd_handler should support edge-triggered FDs */ - if ((global.tune.options & GTUNE_FD_ET) && fdtab[fd].iocb == sock_conn_iocb) - fdtab[fd].state |= FD_ET_POSSIBLE; - - fdtab[fd].thread_mask = thread_mask; /* note: do not reset polled_mask here as it indicates which poller * still knows this FD from a possible previous round. */ diff --git a/src/fd.c b/src/fd.c index 5cb4fa1257..db2130412e 100644 --- a/src/fd.c +++ b/src/fd.c @@ -329,7 +329,7 @@ void _fd_delete_orphan(int fd) polled_mask[fd].poll_recv = polled_mask[fd].poll_send = 0; fdtab[fd].state = 0; - fdtab[fd].refc_tgid = 0; + fd_reset_tgid(fd); #ifdef DEBUG_FD fdtab[fd].event_count = 0; -- 2.39.5