MAJOR: poller: only touch/inspect the update_mask under tgid protection

With thread groups and group-local masks, the update_mask cannot be
touched nor even checked if it may change below us. In order to avoid
this, we have to grab a reference to the FD's tgid before checking the
update mask. The operations are cheap enough so that we don't notice it
in performance tests. This is expected because the risk of meeting a
reassigned FD during an update remains very low.

It's worth noting that the tgid cannot be trusted during startup nor
during soft-stop since that may come from anywhere at the moment. Since
soft-stop runs under thread isolation we use that hint to decide whether
or not to check that the FD's tgid matches the current one.

The modification is applied to the 3 thread-aware pollers, i.e. epoll,
kqueue, and evports. Also one poll_drop counter was missing for shared
updates, though it might be hard to trigger it.

With this change applied, thread groups are usable in benchmarks.

diff --git a/src/ev_epoll.c b/src/ev_epoll.c
index 7be659bc1f7309b9edd5b5b94c23acf9798fdfb6..884c03837ff5b9a0be944c5edb262d477665c274 100644 (file)
--- a/src/ev_epoll.c
+++ b/src/ev_epoll.c
@@ -169,16 +169,24 @@ static void _do_poll(struct poller *p, int exp, int wake)
        for (updt_idx = 0; updt_idx < fd_nbupdt; updt_idx++) {
                fd = fd_updt[updt_idx];
 
-               _HA_ATOMIC_AND(&fdtab[fd].update_mask, ~ti->ltid_bit);
-               if (!fdtab[fd].owner) {
+               if (!fd_grab_tgid(fd, tgid)) {
+                       /* was reassigned */
                        activity[tid].poll_drop_fd++;
                        continue;
                }
 
-               _update_fd(fd);
+               _HA_ATOMIC_AND(&fdtab[fd].update_mask, ~ti->ltid_bit);
+
+               if (fdtab[fd].owner)
+                       _update_fd(fd);
+               else
+                       activity[tid].poll_drop_fd++;
+
+               fd_drop_tgid(fd);
        }
        fd_nbupdt = 0;
-       /* Scan the global update list */
+
+       /* Scan the shared update list */
        for (old_fd = fd = update_list[tgid - 1].first; fd != -1; fd = fdtab[fd].update.next) {
                if (fd == -2) {
                        fd = old_fd;
@@ -188,13 +196,24 @@ static void _do_poll(struct poller *p, int exp, int wake)
                        fd = -fd -4;
                if (fd == -1)
                        break;
-               if (fdtab[fd].update_mask & ti->ltid_bit)
-                       done_update_polling(fd);
-               else
+
+               if (!fd_grab_tgid(fd, tgid)) {
+                       /* was reassigned */
+                       activity[tid].poll_drop_fd++;
                        continue;
-               if (!fdtab[fd].owner)
+               }
+
+               if (!(fdtab[fd].update_mask & ti->ltid_bit))
                        continue;
-               _update_fd(fd);
+
+               done_update_polling(fd);
+
+               if (fdtab[fd].owner)
+                       _update_fd(fd);
+               else
+                       activity[tid].poll_drop_fd++;
+
+               fd_drop_tgid(fd);
        }
 
        thread_idle_now();

diff --git a/src/ev_evports.c b/src/ev_evports.c
index 79854702a65acaa696c1e7610f4402d8444c8a02..19d572c66375eace89d648696573ba7fd03555bb 100644 (file)
--- a/src/ev_evports.c
+++ b/src/ev_evports.c
@@ -126,16 +126,24 @@ static void _do_poll(struct poller *p, int exp, int wake)
        for (i = 0; i < fd_nbupdt; i++) {
                fd = fd_updt[i];
 
-               _HA_ATOMIC_AND(&fdtab[fd].update_mask, ~ti->ltid_bit);
-               if (fdtab[fd].owner == NULL) {
+               if (!fd_grab_tgid(fd, tgid)) {
+                       /* was reassigned */
                        activity[tid].poll_drop_fd++;
                        continue;
                }
 
-               _update_fd(fd);
+               _HA_ATOMIC_AND(&fdtab[fd].update_mask, ~ti->ltid_bit);
+
+               if (fdtab[fd].owner)
+                       _update_fd(fd);
+               else
+                       activity[tid].poll_drop_fd++;
+
+               fd_drop_tgid(fd);
        }
        fd_nbupdt = 0;
-       /* Scan the global update list */
+
+       /* Scan the shared update list */
        for (old_fd = fd = update_list[tgid - 1].first; fd != -1; fd = fdtab[fd].update.next) {
                if (fd == -2) {
                        fd = old_fd;
@@ -145,13 +153,24 @@ static void _do_poll(struct poller *p, int exp, int wake)
                        fd = -fd -4;
                if (fd == -1)
                        break;
-               if (fdtab[fd].update_mask & ti->ltid_bit)
-                       done_update_polling(fd);
-               else
+
+               if (!fd_grab_tgid(fd, tgid)) {
+                       /* was reassigned */
+                       activity[tid].poll_drop_fd++;
                        continue;
-               if (!fdtab[fd].owner)
+               }
+
+               if (!(fdtab[fd].update_mask & ti->ltid_bit))
                        continue;
-               _update_fd(fd);
+
+               done_update_polling(fd);
+
+               if (fdtab[fd].owner)
+                       _update_fd(fd);
+               else
+                       activity[tid].poll_drop_fd++;
+
+               fd_drop_tgid(fd);
        }
 
        thread_idle_now();

diff --git a/src/ev_kqueue.c b/src/ev_kqueue.c
index 53bda1fe2c156ca43ff3c74bc9adca627dba6ea9..3d555ec7ef8ae2899733f0a10536dcc495ba603b 100644 (file)
--- a/src/ev_kqueue.c
+++ b/src/ev_kqueue.c
@@ -102,12 +102,20 @@ static void _do_poll(struct poller *p, int exp, int wake)
        for (updt_idx = 0; updt_idx < fd_nbupdt; updt_idx++) {
                fd = fd_updt[updt_idx];
 
-               _HA_ATOMIC_AND(&fdtab[fd].update_mask, ~ti->ltid_bit);
-               if (!fdtab[fd].owner) {
+               if (!fd_grab_tgid(fd, tgid)) {
+                       /* was reassigned */
                        activity[tid].poll_drop_fd++;
                        continue;
                }
-               changes = _update_fd(fd, changes);
+
+               _HA_ATOMIC_AND(&fdtab[fd].update_mask, ~ti->ltid_bit);
+
+               if (fdtab[fd].owner)
+                       changes = _update_fd(fd, changes);
+               else
+                       activity[tid].poll_drop_fd++;
+
+               fd_drop_tgid(fd);
        }
        /* Scan the global update list */
        for (old_fd = fd = update_list[tgid - 1].first; fd != -1; fd = fdtab[fd].update.next) {
@@ -119,13 +127,24 @@ static void _do_poll(struct poller *p, int exp, int wake)
                        fd = -fd -4;
                if (fd == -1)
                        break;
-               if (fdtab[fd].update_mask & ti->ltid_bit)
-                       done_update_polling(fd);
-               else
+
+               if (!fd_grab_tgid(fd, tgid)) {
+                       /* was reassigned */
+                       activity[tid].poll_drop_fd++;
                        continue;
-               if (!fdtab[fd].owner)
+               }
+
+               if (!(fdtab[fd].update_mask & ti->ltid_bit))
                        continue;
-               changes = _update_fd(fd, changes);
+
+               done_update_polling(fd);
+
+               if (fdtab[fd].owner)
+                       changes = _update_fd(fd, changes);
+               else
+                       activity[tid].poll_drop_fd++;
+
+               fd_drop_tgid(fd);
        }
 
        thread_idle_now();