Am 01.02.2022 um 11:30 hat Paolo Bonzini geschrieben: > On 2/1/22 10:45, Emanuele Giuseppe Esposito wrote: > > > That said, even if they are a different category, I think it makes sense > > > to leave them in the same header file as I/O functions, because I/O > > > functions are locked out between drained_begin and drained_end. > > > > Proposed category description: > > /* > > * "Global OR I/O" API functions. These functions can run without > > * the BQL, but only in one specific iothread/main loop. > > * > > * More specifically, these functions use BDRV_POLL_WHILE(bs), which > > * requires the caller to be either in the main thread and hold > > * the BlockdriverState (bs) AioContext lock, or directly in the > > * home thread that runs the bs AioContext. Calling them from > > * another thread in another AioContext would cause deadlocks. > > * > > * Therefore, these functions are not proper I/O, because they > > * can't run in *any* iothreads, but only in a specific one. > > */ > > > > Functions that will surely go under this category: > > > > BDRV_POLL_WHILE > > bdrv_parent_drained_begin_single > > bdrv_parent_drained_end_single > > bdrv_drain_poll > > bdrv_drained_begin > > bdrv_do_drained_begin_quiesce > > bdrv_subtree_drained_begin > > bdrv_drained_end > > bdrv_drained_end_no_poll > > bdrv_subtree_drained_end > > > > (all generated_co_wrapper) > > bdrv_truncate > > bdrv_check > > bdrv_invalidate_cache > > bdrv_flush > > bdrv_pdiscard > > bdrv_readv_vmstate > > bdrv_writev_vmstate > > > > > > What I am not sure: > > > > * bdrv_drain_all_begin - bdrv_drain_all_end - bdrv_drain_all: these were > > classified as GS, because thay are always called from the main loop. > > Should they go in this new category? > > 1) They look at the list of BDS's, and 2) you can't in general be sure that > all BDS's are in *your* AioContext if you call them from a specific > AioContext. > > So they should be GS.
I agree, calling drain_all functions can only work from the main thread, so they are GS. > > * how should I interpret "all the callers of BDRV_POLL_WHILE"? > > Meaning, if I consider also the callers of the callers, we end up > > covering much much more functions. Should I only consider the direct > > callers (ie the above)? > > In general it is safe to make a function GS even if it is potentially "GS or > I/O", because that _reduces_ the number of places you can call it from. > It's likewise safe to make it I/O-only, but probably it makes less sense. Basically, we have a hierarchy of categories where you can always call functions in other categories with less restrictions, but never the opposite direction. 1. Common functions 2. I/O functions 3. I/O or GS functions 4. GS functions So common functions must never call any of the other categories. Global state functions can call functions in any category. And "I/O or GS" functions like BDRV_POLL_WHILE() can be called by other "I/O or GS" or just GS functions, but if it's ever (directly or indirectly) called by an I/O or common function, that would be a bug. Kevin
