[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v4 00/25] block layer: split block APIs in global state and I
From: |
Stefan Hajnoczi |
Subject: |
Re: [PATCH v4 00/25] block layer: split block APIs in global state and I/O |
Date: |
Thu, 28 Oct 2021 16:45:10 +0100 |
On Mon, Oct 25, 2021 at 04:09:41PM +0200, Philippe Mathieu-Daudé wrote:
> 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?
I think the assertions are only in the slow path (functions that must be
run with the BQL held from the main thread). The I/O request code path
does not have new assertions.
Stefan
signature.asc
Description: PGP signature
- [PATCH v4 18/25] block/coroutines: I/O API, (continued)
- [PATCH v4 18/25] block/coroutines: I/O API, Emanuele Giuseppe Esposito, 2021/10/25
- [PATCH v4 17/25] block/copy-before-write.h: global state API + assertions, Emanuele Giuseppe Esposito, 2021/10/25
- [PATCH v4 22/25] block_int-common.h: assertions in the callers of BdrvChildClass function pointers, Emanuele Giuseppe Esposito, 2021/10/25
- [PATCH v4 21/25] block_int-common.h: split function pointers in BdrvChildClass, Emanuele Giuseppe Esposito, 2021/10/25
- [PATCH v4 20/25] block_int-common.h: assertion in the callers of BlockDriver function pointers, Emanuele Giuseppe Esposito, 2021/10/25
- [PATCH v4 25/25] job.h: assertions in the callers of JobDriver funcion pointers, Emanuele Giuseppe Esposito, 2021/10/25
- [PATCH v4 23/25] block-backend-common.h: split function pointers in BlockDevOps, Emanuele Giuseppe Esposito, 2021/10/25
- [PATCH v4 24/25] job.h: split function pointers in JobDriver, Emanuele Giuseppe Esposito, 2021/10/25
- Re: [PATCH v4 00/25] block layer: split block APIs in global state and I/O, Philippe Mathieu-Daudé, 2021/10/25
- Re: [PATCH v4 00/25] block layer: split block APIs in global state and I/O,
Stefan Hajnoczi <=
- Re: [PATCH v4 00/25] block layer: split block APIs in global state and I/O, Stefan Hajnoczi, 2021/10/28