Re: [PATCH v6 00/33] block layer: split block APIs in global state and I/O

[Emanuele Giuseppe Esposito]

On 08/02/2022 14:08, Kevin Wolf wrote:
> Am 08.02.2022 um 12:42 hat Emanuele Giuseppe Esposito geschrieben:
>>
>>
>> On 07/02/2022 19:30, Kevin Wolf wrote:
>>> Am 21.01.2022 um 18:05 hat Emanuele Giuseppe Esposito geschrieben:
>>>> 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.
>>>
>>> I wonder if we could actually do something to catch at least some kinds
>>> of bugs. The first conclusion from thinking about it is that we probably
>>> shouldn't open-code assert(qemu_in_main_thread()) everywhere, but have a
>>> macro or inline function for each category to be called in each function.
>>>
>>> So an IO_CODE() macro could increase a counter in the coroutine object
>>> (that is decreased again at the end of the function with g_auto), and
>>> then GLOBAL_STATE_CODE() could not only assert that we're holding the
>>> BQL, but also that the counter is still 0, i.e. it is not (indirectly)
>>> called by an I/O function.
>>>
>>> We may want to enable this only in debug builds, but maybe still worth a
>>> thought anyway?
>>
>> I don't understand what is the point of the counter, do you want to use
>> it as a boolean flag?
> 
> It would only be checked as a boolean flag, but it needs to be a counter
> because of nesting where e.g. one I/O function calls another I/O
> function.
> 
>> Would a single counter work in a multi-threaded context? Shouldn't we
>> have it per-thread? And why you increase it only in coroutines?
> 
> I don't mean increasing it only in coroutine context, but having a
> per-coroutine counter, including the leader coroutine which exists for
> non-coroutine context in every thread.
> 

As agreed also on IRC, while we wait also additional feedback from
others on the counter logic, I am going to add GLOBAL_STATE_CODE,
IO_CODE and IO_OR_GS_CODE macros in the respective functions.

GLOBAL_STATE_CODE will just replace the assert(qemu_in_main_thread()),
while IO_CODE and IO_OR_GS_CODE will be nop.

This will also visually help understanding in which category each
function is, without looking at the header.
In the future we can extend the macro to support also additional logic,
like counters.

Emanuele