RepoMirrors/haproxy
1
0
Fork 0
mirror of http://git.haproxy.org/git/haproxy.git/ synced 2024-12-21 03:00:35 +00:00
Code Issues Packages Projects Releases Wiki Activity

BUG/MEDIUM: lists: add missing store barrier in MT_LIST_ADD/MT_LIST_ADDQ

Browse Source 
The torture test run for previous commit 787dc20 ("BUG/MEDIUM: lists: add
missing store barrier on MT_LIST_BEHEAD()") finally broke again after 34M
connections. It appeared that MT_LIST_ADD and MT_LIST_ADDQ were suffering
from the same missing barrier when restoring the original pointers before
giving up, when checking if the element was already added. This is indeed
something which seldom happens with the shared scheduler, in case two
threads simultaneously try to wake up the same tasklet.

With a store barrier there after reverting the pointers, the torture test
survived 750M connections on the NanoPI-Fire3, so it looks good this time.
Probably that MT_LIST_BEHEAD should be added to test-list.c since it seems
to be more sensitive to concurrent accesses with MT_LIST_ADDQ.

It's worth noting that there is no barrier between the last two pointers
update, while there is one in MT_LIST_POP and MT_LIST_BEHEAD, the latter
having shown to be needed, but I cannot demonstrate why we would need
one here. Given that the code seems solid here, let's stick to what is
shown to work.

This fix should be backported to 2.1, just for the sake of safety since
the issue couldn't be triggered there, but it could change with the
compiler or when backporting a fix for example.
This commit is contained in:
Willy Tarreau 2020-07-09 05:01:27 +02:00
parent 787dc20952
commit 3b8f9b7b88
1 changed files with 2 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
include/haproxy/list.h
View File

@ -244,6 +244,7 @@
if ((el)->next != (el) || (el)->prev != (el)) { \
(n)->prev = p; \
(lh)->next = n; \
__ha_barrier_store(); \
break; \
} \
(el)->next = n; \
@ -283,6 +284,7 @@
if ((el)->next != (el) || (el)->prev != (el)) { \
p->next = n; \
(lh)->prev = p; \
__ha_barrier_store(); \
break; \
} \
(el)->next = n; \