On Wed, 11/29 12:00, Stefan Hajnoczi wrote: > On Wed, Nov 29, 2017 at 11:55:02AM +0800, Fam Zheng wrote: > > As we move forwards with new features in the block layer, the chances of > > tricky > > bugs happening have been increasing alongside - block jobs, coroutines, > > throttling, AioContext, op blockers and image locking combined together > > make a > > large and complex picture that is hard to fully understand and work with. > > Some > > bugs we've encountered are quite challenging already. Examples are: > > > > - segfault in parallel blockjobs (iotest 30) > > https://lists.gnu.org/archive/html/qemu-devel/2017-11/msg01144.html > > > > - Intermittent hang of iotest 194 (bdrv_drain_all after non-shared storage > > migration) > > https://lists.gnu.org/archive/html/qemu-devel/2017-11/msg01626.html > > > > - Drainage in bdrv_replace_child_noperm() > > https://lists.gnu.org/archive/html/qemu-devel/2017-11/msg00868.html > > > > - Regression from 2.8: stuck in bdrv_drain() > > https://lists.gnu.org/archive/html/qemu-devel/2017-04/msg02193.html > > > > So in principle, what should we do to make the block layer easy to > > understand, > > develop with and debug? > > The assumptions that the code relies on are unclear so it's easy to > introduce new bugs.
Is that one thing we could do better in documenting? > > We are at a point where code review isn't finding certain bugs because > no single person knows all the assumptions. Previously the problem was > contained because maintainers spotted problems before patches were > merged. > > This is not primarily a documentation problem though. We cannot > document our way out of this because no single person (patch author or > code reviewer) can know or check everything anymore due to the scale. > > I think it's a (lack of) design problem because we have many incomplete > abstractions like block jobs, IOThreads, block graph, image locking, > etc. They do not cover all possibly states and interactions today. > Extending them leads to complex bugs. > > A little progress has been made with defining higher-level APIs for > block drivers and block jobs. This way they either don't deal with > low-level details of the concurrency and event loop models (e.g. > bdrv_coroutine_enter()) or there is an interface that prompts them to > integrate properly like bdrv_attach/detach_aio_context(). Sounds good. > > Event loops and coroutines are good but they should not be used directly > by block drivers and block jobs. We need safe, high-level APIs that > implement commonly-used operations. > > > - Documentation > > > > There is no central developer doc about block layer, especially how all > > pieces > > fit together. Having one will make it a lot easier for new contributors to > > understand better. Of course, we're facing the old problem: the code is > > moving, maintaining an updated document needs effort. > > > > Idea: add ./doc/deve/block.txt? > > IOThreads and AioContexts are addressed here: > docs/devel/multiple-iothreads.txt > > The game has become significantly more complex than what the document > describes. It's lacking aio_co_wake() and aio_co_schedule() for > example. > > > - Simplified code, or more orthogonal/modularized architecture. > > > > Each aspect of block layer is complex enough so isolating them as much as > > possible is a reasonable approach to control the complexity. Block jobs > > and > > throttling becoming block filters is a good example, we should identify > > more. > > > > Idea: rethink event loops. Create coroutines ubiquitously (for example for > > each fd handler, BH and timer), so that many nested aio_poll() can be > > removed. > > > > Crazy idea: move the whole block layer to a vhost process, and implement > > existing features differently, especially in terms of multi-threading > > (hint: > > rust?). > > A reimplementation will not solve the problem because: > > 1. If it still has the same feature set and requirements then the level > of complexity will be comparable. > > 2. We can reduce accidental (inessential) complexity by continuing the > various efforts around the block graph, block jobs, multi-queue block > layer with an eye towards higher level APIs. Starting over is certainly not the motivation to do qemu-vhost, but it would be an opportunity to use different async/concurrency paradigms if that is going to happen. I think in current block layer, event loop + coroutine is a good combination, but having nested aio_poll()'s made it worse, then mixing IOThreads in makes it a lot more complicated. Fam
