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.
This commit is contained in:
Willy Tarreau 2022-07-09 23:55:43 +02:00
parent d95f18fa39
commit 1f947cb39e
3 changed files with 86 additions and 29 deletions

View File

@ -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);
if (!fd_grab_tgid(fd, tgid)) {
/* was reassigned */
activity[tid].poll_drop_fd++;
continue;
}
if (!(fdtab[fd].update_mask & ti->ltid_bit))
continue;
done_update_polling(fd);
if (fdtab[fd].owner)
_update_fd(fd);
else
continue;
if (!fdtab[fd].owner)
continue;
_update_fd(fd);
activity[tid].poll_drop_fd++;
fd_drop_tgid(fd);
}
thread_idle_now();

View File

@ -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);
if (!fd_grab_tgid(fd, tgid)) {
/* was reassigned */
activity[tid].poll_drop_fd++;
continue;
}
if (!(fdtab[fd].update_mask & ti->ltid_bit))
continue;
done_update_polling(fd);
if (fdtab[fd].owner)
_update_fd(fd);
else
continue;
if (!fdtab[fd].owner)
continue;
_update_fd(fd);
activity[tid].poll_drop_fd++;
fd_drop_tgid(fd);
}
thread_idle_now();

View File

@ -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);
if (!fd_grab_tgid(fd, tgid)) {
/* was reassigned */
activity[tid].poll_drop_fd++;
continue;
}
if (!(fdtab[fd].update_mask & ti->ltid_bit))
continue;
done_update_polling(fd);
if (fdtab[fd].owner)
changes = _update_fd(fd, changes);
else
continue;
if (!fdtab[fd].owner)
continue;
changes = _update_fd(fd, changes);
activity[tid].poll_drop_fd++;
fd_drop_tgid(fd);
}
thread_idle_now();