]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
Revert "BUG/MEDIUM: lists: Lock the element while we check if it is in a list."
authorWilly Tarreau <w@1wt.eu>
Mon, 29 Jun 2020 19:37:57 +0000 (21:37 +0200)
committerWilly Tarreau <w@1wt.eu>
Mon, 29 Jun 2020 19:54:37 +0000 (21:54 +0200)
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=<optimized out>, arg=<optimized out>) at src/wdt.c:119
 #4  <signal handler called>
 #5  0x00000000004fbccd in tasklet_wakeup (tl=0x1b5abc0) at include/haproxy/task.h:351
 #6  listener_accept (fd=<optimized out>) at src/listener.c:999
 #7  0x00000000004262df in fd_update_events (evts=<optimized out>, fd=6) at include/haproxy/fd.h:418
 #8  _do_poll (p=<optimized out>, exp=<optimized out>, wake=<optimized out>) 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=<optimized out>) 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!

include/haproxy/list.h

index 7f05be2f93c7efe6cb57ae7dedbcc882ae4671e1..2f2699f17c5d3b9b775d97ad9fc3571daeebc957 100644 (file)
         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;                                          \
                        __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;                                            \
        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;                                          \
                        __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;                                            \