BUG/MEDIUM: lists: add missing store barrier in MT_LIST_ADD/MT_LIST_ADDQ
authorWilly Tarreau <w@1wt.eu>
Thu, 9 Jul 2020 03:01:27 +0000 (05:01 +0200)
committerChristopher Faulet <cfaulet@haproxy.com>
Fri, 17 Jul 2020 08:26:42 +0000 (10:26 +0200)
commit37a8e1785c31fa93de36ef582d047b244335c4ca
tree82306ac04660ec1bd990dd30e0915d8ebe5c8df3
parentbed4aa43d017ca4b2c5cabeef5540dd8cf48fcbb
BUG/MEDIUM: lists: add missing store barrier in MT_LIST_ADD/MT_LIST_ADDQ

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.

(cherry picked from commit 3b8f9b7b880397db03bfa54b26c8b6dbf6b5dc5e)
Signed-off-by: Willy Tarreau <w@1wt.eu>
(cherry picked from commit ae8b8361ce2d50df0fb8c2d4b4d23e13ff497029)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>
include/common/mini-clist.h