Re: [RFC PATCH 3/3] block: Assert we're running in the right thread

["Philippe Mathieu-Daudé" <philmd@redhat.com>]

On 5/14/20 4:30 PM, Kevin Wolf wrote:
> Am 14.05.2020 um 15:52 hat Stefan Reiter geschrieben:
>> On 5/12/20 4:43 PM, Kevin Wolf wrote:
>>> tracked_request_begin() is called for most I/O operations, so it's a
>>> good place to assert that we're indeed running in the home thread of the
>>> node's AioContext.
>>>
>>
>> Is this patch supposed to be always correct or only together with nr. 2?
>>
>> I changed our code to call bdrv_flush_all from the main AIO context and it
>> certainly works just fine (even without this series, so I suppose that would
>> be the 'correct' way to fix it you mention on the cover), though of course
>> it trips this assert without patch 2.
> 
> Yes, I think this is a basic assumption that should always be true.
> This series shouldn't fix anything for upstream QEMU (at least I'm not
> aware of anything that needs it), so the assertion could be added even
> without the other patches.
> 
> In fact, I'm currently thinking that committing just patch 1 (because
> it's a nice cleanup anyway) and patch 3 (because it will let us know
> when we mess up) might make sense.

FWIW applying 1+3 as no-RFC LGTM.

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> 
> Kevin
> 
>>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>>> ---
>>>    block/io.c | 5 ++++-
>>>    1 file changed, 4 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/block/io.c b/block/io.c
>>> index 7808e8bdc0..924bf5ba46 100644
>>> --- a/block/io.c
>>> +++ b/block/io.c
>>> @@ -695,14 +695,17 @@ static void tracked_request_begin(BdrvTrackedRequest *req,
>>>                                      uint64_t bytes,
>>>                                      enum BdrvTrackedRequestType type)
>>>    {
>>> +    Coroutine *self = qemu_coroutine_self();
>>> +
>>>        assert(bytes <= INT64_MAX && offset <= INT64_MAX - bytes);
>>> +    assert(bs->aio_context == qemu_coroutine_get_aio_context(self));
>>>        *req = (BdrvTrackedRequest){
>>>            .bs = bs,
>>>            .offset         = offset,
>>>            .bytes          = bytes,
>>>            .type           = type,
>>> -        .co             = qemu_coroutine_self(),
>>> +        .co             = self,
>>>            .serialising    = false,
>>>            .overlap_offset = offset,
>>>            .overlap_bytes  = bytes,
>>>
>>
> 
>