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.

diff --git a/include/haproxy/fd.h b/include/haproxy/fd.h
index 8c558189d6d1e7a69f99c0fb0e38abee1b46e8ea..6319484d6b603438482860efa1a648db37b28669 100644 (file)
--- 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 5cb4fa1257889bf375ef13ab6aeebe0f9a4e54ac..db2130412ee255eb180d7631814065595c9852d4 100644 (file)
--- 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;