On 10/25/21 12:17, Emanuele Giuseppe Esposito wrote: [...] > Each function in the GS API will have an assertion, checking > that it is always running under BQL. > I/O functions are instead thread safe (or so should be), meaning > that they *can* run under BQL, but also in an iothread in another > AioContext. Therefore they do not provide any assertion, and > need to be audited manually to verify the correctness. > > Adding assetions has helped finding 2 bugs already, as shown in > my series "Migration: fix missing iothread locking". > > Tested this series by running unit tests, qemu-iotests and qtests > (x86_64). > Some functions in the GS API are used everywhere but not > properly tested. Therefore their assertion is never actually run in > the tests, so despite my very careful auditing, it is not impossible > to exclude that some will trigger while actually using QEMU. > > Patch 1 introduces qemu_in_main_thread(), the function used in > all assertions. This had to be introduced otherwise all unit tests > would fail, since they run in the main loop but use the code in > stubs/iothread.c > Patches 2-14 and 19-25 (with the exception of patch 9, that is an additional > assert) are all structured in the same way: first we split the header > and in the next (even) patch we add assertions. > The rest of the patches ontain either both assertions and split, > or have no assertions.
This seems a lot of assertions added in hot-path code. Does it makes sense to use a BLOCK_ASSERT() macro instead, only expanded when configure with --enable-debug?
