From 35ee710ececff9e87a0b770193d78a8a0cae0a50 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Fri, 8 Jul 2022 11:33:43 +0200 Subject: [PATCH] MEDIUM: fd/poller: make the update-list per-group The update-list needs to be per-group because its inspection is based on a mask and we need to be certain when scanning it if a mask is for the same thread or another one. Once per-group there's no doubt about it, even if the FD's polling changes, the entry remains valid. It will be needed to check the tgid though. Note that a soft-stop or pause/resume might not necessarily work here with tgroups>1, because the operation might be delivered to a thread that doesn't belong to the group and whoe update mask will not reflect one that is interesting here. We can't do better at this stage. --- include/haproxy/fd-t.h | 4 ++-- include/haproxy/fd.h | 7 +++---- src/ev_epoll.c | 2 +- src/ev_evports.c | 2 +- src/ev_kqueue.c | 2 +- src/ev_poll.c | 2 +- src/ev_select.c | 2 +- src/fd.c | 9 +++++---- 8 files changed, 15 insertions(+), 15 deletions(-) diff --git a/include/haproxy/fd-t.h b/include/haproxy/fd-t.h index 64416d6dc..de91f1776 100644 --- a/include/haproxy/fd-t.h +++ b/include/haproxy/fd-t.h @@ -145,11 +145,11 @@ struct fdlist_entry { int prev; } ALIGNED(8); -/* head of the fd cache */ +/* head of the fd cache, per-group */ struct fdlist { int first; int last; -} ALIGNED(8); +} ALIGNED(64); /* info about one given fd. Note: only align on cache lines when using threads; * 32-bit small archs can put everything in 32-bytes when threads are disabled. diff --git a/include/haproxy/fd.h b/include/haproxy/fd.h index 2b2c3ff38..6763200e3 100644 --- a/include/haproxy/fd.h +++ b/include/haproxy/fd.h @@ -43,7 +43,7 @@ extern struct fdinfo *fdinfo; /* less-often used infos for file descri extern int totalconn; /* total # of terminated sessions */ extern int actconn; /* # of active sessions */ -extern volatile struct fdlist update_list; +extern volatile struct fdlist update_list[MAX_TGROUPS]; extern struct polled_mask *polled_mask; extern THREAD_LOCAL int *fd_updt; // FD updates list @@ -134,13 +134,13 @@ static inline void done_update_polling(int fd) update_mask = _HA_ATOMIC_AND_FETCH(&fdtab[fd].update_mask, ~tid_bit); while ((update_mask & all_threads_mask)== 0) { /* If we were the last one that had to update that entry, remove it from the list */ - fd_rm_from_fd_list(&update_list, fd); + fd_rm_from_fd_list(&update_list[tgid - 1], fd); update_mask = (volatile unsigned long)fdtab[fd].update_mask; if ((update_mask & all_threads_mask) != 0) { /* Maybe it's been re-updated in the meanwhile, and we * wrongly removed it from the list, if so, re-add it */ - fd_add_to_fd_list(&update_list, fd); + fd_add_to_fd_list(&update_list[tgid - 1], fd); update_mask = (volatile unsigned long)(fdtab[fd].update_mask); /* And then check again, just in case after all it * should be removed, even if it's very unlikely, given @@ -148,7 +148,6 @@ static inline void done_update_polling(int fd) * care of it yet */ } else break; - } } diff --git a/src/ev_epoll.c b/src/ev_epoll.c index 354a18793..ea2b24f03 100644 --- a/src/ev_epoll.c +++ b/src/ev_epoll.c @@ -166,7 +166,7 @@ static void _do_poll(struct poller *p, int exp, int wake) } fd_nbupdt = 0; /* Scan the global update list */ - for (old_fd = fd = update_list.first; fd != -1; fd = fdtab[fd].update.next) { + for (old_fd = fd = update_list[tgid - 1].first; fd != -1; fd = fdtab[fd].update.next) { if (fd == -2) { fd = old_fd; continue; diff --git a/src/ev_evports.c b/src/ev_evports.c index 05c9ebcca..25cc79b83 100644 --- a/src/ev_evports.c +++ b/src/ev_evports.c @@ -134,7 +134,7 @@ static void _do_poll(struct poller *p, int exp, int wake) } fd_nbupdt = 0; /* Scan the global update list */ - for (old_fd = fd = update_list.first; fd != -1; fd = fdtab[fd].update.next) { + for (old_fd = fd = update_list[tgid - 1].first; fd != -1; fd = fdtab[fd].update.next) { if (fd == -2) { fd = old_fd; continue; diff --git a/src/ev_kqueue.c b/src/ev_kqueue.c index ff377625b..42a1bc5be 100644 --- a/src/ev_kqueue.c +++ b/src/ev_kqueue.c @@ -108,7 +108,7 @@ static void _do_poll(struct poller *p, int exp, int wake) changes = _update_fd(fd, changes); } /* Scan the global update list */ - for (old_fd = fd = update_list.first; fd != -1; fd = fdtab[fd].update.next) { + for (old_fd = fd = update_list[tgid - 1].first; fd != -1; fd = fdtab[fd].update.next) { if (fd == -2) { fd = old_fd; continue; diff --git a/src/ev_poll.c b/src/ev_poll.c index b25d1dc32..093184f6d 100644 --- a/src/ev_poll.c +++ b/src/ev_poll.c @@ -122,7 +122,7 @@ static void _do_poll(struct poller *p, int exp, int wake) } /* Now scan the global update list */ - for (old_fd = fd = update_list.first; fd != -1; fd = fdtab[fd].update.next) { + for (old_fd = fd = update_list[tgid - 1].first; fd != -1; fd = fdtab[fd].update.next) { if (fd == -2) { fd = old_fd; continue; diff --git a/src/ev_select.c b/src/ev_select.c index af14b2e9e..86a89c72b 100644 --- a/src/ev_select.c +++ b/src/ev_select.c @@ -113,7 +113,7 @@ static void _do_poll(struct poller *p, int exp, int wake) _update_fd(fd, &max_add_fd); } /* Now scan the global update list */ - for (old_fd = fd = update_list.first; fd != -1; fd = fdtab[fd].update.next) { + for (old_fd = fd = update_list[tgid - 1].first; fd != -1; fd = fdtab[fd].update.next) { if (fd == -2) { fd = old_fd; continue; diff --git a/src/fd.c b/src/fd.c index 04a1c7b04..6ea3d2ca9 100644 --- a/src/fd.c +++ b/src/fd.c @@ -108,7 +108,7 @@ struct poller pollers[MAX_POLLERS] __read_mostly; struct poller cur_poller __read_mostly; int nbpollers = 0; -volatile struct fdlist update_list; // Global update list +volatile struct fdlist update_list[MAX_TGROUPS]; // Global update list THREAD_LOCAL int *fd_updt = NULL; // FD updates list THREAD_LOCAL int fd_nbupdt = 0; // number of updates in the list @@ -318,7 +318,7 @@ void _fd_delete_orphan(int fd) cur_poller.clo(fd); /* we don't want this FD anymore in the global list */ - fd_rm_from_fd_list(&update_list, fd); + fd_rm_from_fd_list(&update_list[tgid - 1], fd); /* no more updates on this FD are relevant anymore */ HA_ATOMIC_STORE(&fdtab[fd].update_mask, 0); @@ -453,7 +453,7 @@ void updt_fd_polling(const int fd) return; } while (!_HA_ATOMIC_CAS(&fdtab[fd].update_mask, &update_mask, fdtab[fd].thread_mask)); - fd_add_to_fd_list(&update_list, fd); + fd_add_to_fd_list(&update_list[tgid - 1], fd); if (fd_active(fd) && !(fdtab[fd].thread_mask & tid_bit)) { /* we need to wake up another thread to handle it immediately, any will fit, @@ -875,7 +875,8 @@ int init_pollers() goto fail_info; } - update_list.first = update_list.last = -1; + for (p = 0; p < MAX_TGROUPS; p++) + update_list[p].first = update_list[p].last = -1; for (p = 0; p < global.maxsock; p++) { /* Mark the fd as out of the fd cache */