From: Willy Tarreau Date: Mon, 29 Jun 2020 19:37:57 +0000 (+0200) Subject: Revert "BUG/MEDIUM: lists: Lock the element while we check if it is in a list." X-Git-Tag: v2.2-dev12~52 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d59946e673bf1192dde3e136756f7da8cf604e2d;p=thirdparty%2Fhaproxy.git Revert "BUG/MEDIUM: lists: Lock the element while we check if it is in a list." This reverts previous commit 347bbf79d20e1cff57075a8a378355dfac2475e2i. The original code was correct. This patch resulted from a mistaken analysis and breaks the scheduler: ########################## Starting vtest ########################## Testing with haproxy version: 2.2-dev11-90b7d9-23 # top TEST reg-tests/lua/close_wait_lf.vtc TIMED OUT (kill -9) # top TEST reg-tests/lua/close_wait_lf.vtc FAILED (10.008) signal=9 1 tests failed, 0 tests skipped, 88 tests passed Program terminated with signal SIGABRT, Aborted. [Current thread is 1 (Thread 0x7fb0dac2c700 (LWP 11292))] (gdb) bt #0 0x00007fb0e7c143f8 in raise () from /lib64/libc.so.6 #1 0x00007fb0e7c15ffa in abort () from /lib64/libc.so.6 #2 0x000000000053f5d6 in ha_panic () at src/debug.c:269 #3 0x00000000005a6248 in wdt_handler (sig=14, si=, arg=) at src/wdt.c:119 #4 #5 0x00000000004fbccd in tasklet_wakeup (tl=0x1b5abc0) at include/haproxy/task.h:351 #6 listener_accept (fd=) at src/listener.c:999 #7 0x00000000004262df in fd_update_events (evts=, fd=6) at include/haproxy/fd.h:418 #8 _do_poll (p=, exp=, wake=) at src/ev_epoll.c:251 #9 0x0000000000548d0f in run_poll_loop () at src/haproxy.c:2949 #10 0x000000000054908b in run_thread_poll_loop (data=) at src/haproxy.c:3067 #11 0x00007fb0e902b684 in start_thread () from /lib64/libpthread.so.0 #12 0x00007fb0e7ce5eed in clone () from /lib64/libc.so.6 (gdb) up #5 0x00000000004fbccd in tasklet_wakeup (tl=0x1b5abc0) at include/haproxy/task.h:351 351 if (MT_LIST_ADDQ(&task_per_thread[tl->tid].shared_tasklet_list, (struct mt_list *)&tl->list) == 1) { If the commit above is ever backported, this one must be as well! --- diff --git a/include/haproxy/list.h b/include/haproxy/list.h index 7f05be2f93..2f2699f17c 100644 --- a/include/haproxy/list.h +++ b/include/haproxy/list.h @@ -230,8 +230,8 @@ int _ret = 0; \ struct mt_list *lh = (_lh), *el = (_el); \ while (1) { \ - struct mt_list *n, *n2; \ - struct mt_list *p, *p2; \ + struct mt_list *n; \ + struct mt_list *p; \ n = _HA_ATOMIC_XCHG(&(lh)->next, MT_LIST_BUSY); \ if (n == MT_LIST_BUSY) \ continue; \ @@ -241,23 +241,9 @@ __ha_barrier_store(); \ continue; \ } \ - n2 = _HA_ATOMIC_XCHG(&(el)->next, MT_LIST_BUSY); \ - if (n2 != (el)) { \ + if ((el)->next != (el) || (el)->prev != (el)) { \ (n)->prev = p; \ (lh)->next = n; \ - (el)->next = n2; \ - if (n2 == MT_LIST_BUSY) \ - continue; \ - break; \ - } \ - p2 = _HA_ATOMIC_XCHG(&(el)->prev, MT_LIST_BUSY); \ - if (p2 != (el)) { \ - n->prev = p; \ - (lh)->next = n; \ - (el)->next = n2; \ - (el)->prev = p2; \ - if (p2 == MT_LIST_BUSY) \ - continue; \ break; \ } \ (el)->next = n; \ @@ -283,8 +269,8 @@ int _ret = 0; \ struct mt_list *lh = (_lh), *el = (_el); \ while (1) { \ - struct mt_list *n, *n2; \ - struct mt_list *p, *p2; \ + struct mt_list *n; \ + struct mt_list *p; \ p = _HA_ATOMIC_XCHG(&(lh)->prev, MT_LIST_BUSY); \ if (p == MT_LIST_BUSY) \ continue; \ @@ -294,23 +280,9 @@ __ha_barrier_store(); \ continue; \ } \ - n2 = _HA_ATOMIC_XCHG(&(el)->next, MT_LIST_BUSY); \ - if (n2 != (el)) { \ + if ((el)->next != (el) || (el)->prev != (el)) { \ p->next = n; \ (lh)->prev = p; \ - (el)->next = n2; \ - if (n2 == MT_LIST_BUSY) \ - continue; \ - break; \ - } \ - p2 = _HA_ATOMIC_XCHG(&(el)->prev, MT_LIST_BUSY); \ - if (p2 != (el)) { \ - p->next = n; \ - (lh)->prev = p; \ - (el)->next = n2; \ - (el)->prev = p2; \ - if (p2 == MT_LIST_BUSY) \ - continue; \ break; \ } \ (el)->next = n; \