Re: [PATCH v6 02/33] include/block/block: split header into I/O and global state API

[Paolo Bonzini]

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.

> * 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.

Paolo