[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Emanuele Giuseppe Esposito
Subject: Re: [PATCH v6 02/33] include/block/block: split header into I/O and global state API
Date: Tue, 8 Feb 2022 11:50:49 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.2.0

On 07/02/2022 17:53, Kevin Wolf wrote:
> 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_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.

Added in the respective category documentation:
> 1. Common functions
 * These functions must never call any function from other categories
 * (I/O, "I/O or GS", Global State) except this one, but can be invoked by
 * all of them.

> 2. I/O functions
 * These functions can only call functions from I/O and common categories,
 * but can be invoked by GS, "I/O or GS" and I/O APIs.

> 3. I/O or GS functions
 * These functions can call any function from I/O, common and this
 * categories, but must be invoked only by other "I/O or GS" and GS APIs.

> 4. GS functions
 * These functions can call any function from this and other categories
 * (I/O, "I/O or GS", common), but must be invoked only by other GS APIs.

> 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

reply via email to

[Prev in Thread] Current Thread [Next in Thread]