[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v6 01/33] main-loop.h: introduce qemu_in_main_thread()
From: |
Emanuele Giuseppe Esposito |
Subject: |
Re: [PATCH v6 01/33] main-loop.h: introduce qemu_in_main_thread() |
Date: |
Mon, 31 Jan 2022 12:25:54 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.2.0 |
On 27/01/2022 11:56, Kevin Wolf wrote:
> Am 21.01.2022 um 18:05 hat Emanuele Giuseppe Esposito geschrieben:
>> When invoked from the main loop, this function is the same
>> as qemu_mutex_iothread_locked, and returns true if the BQL is held.
>
> So its name is misleading because it doesn't answer the question whether
> we're in the main thread, but whethere we're holding the BQL (which is
> mostly equivalent to holding an AioContext lock, not being in the home
> thread of that AioContext).
>
>> When invoked from iothreads or tests, it returns true only
>> if the current AioContext is the Main Loop.
>>
>> This essentially just extends qemu_mutex_iothread_locked to work
>> also in unit tests or other users like storage-daemon, that run
>> in the Main Loop but end up using the implementation in
>> stubs/iothread-lock.c.
>>
>> Using qemu_mutex_iothread_locked in unit tests defaults to false
>> because they use the implementation in stubs/iothread-lock,
>> making all assertions added in next patches fail despite the
>> AioContext is still the main loop.
>>
>> Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>
>> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
>
> This adds a new function that is almost the same as an existing
> function, but the documentation is unclear when I should use one or the
> other.
Why do you think it is unclear? as explained in commit and
documentation, unit tests for example use stubs/iothread-lock, which
returns always false. So we can (and should!) use this function in APIs
invoked by
unit tests, because qemu_mutex_iothread_locked will just return false,
making all tests crash. On the other side, I am pretty sure it won't
cause any failure when running QEMU or qemu-iotests, because there most
of the API use the cpu implementation.
>
> What are the use cases for the existing qemu_mutex_iothread_locked()
> stub where we rely on false being returned?
I don't think we ever rely on stub being false. There are only 2 places
where I see
!qemu_mutex_iothread_locked(), and are in helper_regs.c and cpus.c
So being cpu related functions they use the cpu implementation.
However, commit 5f50be9b5810293141bb53cfd0cb46e765367d56 changed the
stub to return false for a specific reason.
>
> If there aren't any, then maybe we should change the stub for that one
> instead of adding a new function that behaves the same in the system
> emulator and different only when it's stubbed out?
I don't think we can replace it, bcause stubs/qemu_in_main_thread()
calls qemu_get_current_aio_context, that in turn calls
qemu_mutex_iothread_locked. So we implicitly rely on that "false".
Thank you,
Emanuele
[PATCH v6 03/33] assertions for block global state API, Emanuele Giuseppe Esposito, 2022/01/21
[PATCH v6 04/33] block/export/fuse.c: allow writable exports to take RESIZE permission, Emanuele Giuseppe Esposito, 2022/01/21
[PATCH v6 05/33] include/sysemu/block-backend: split header into I/O and global state (GS) API, Emanuele Giuseppe Esposito, 2022/01/21
[PATCH v6 06/33] block/block-backend.c: assertions for block-backend, Emanuele Giuseppe Esposito, 2022/01/21
[PATCH v6 10/33] include/block/blockjob_int.h: split header into I/O and GS API, Emanuele Giuseppe Esposito, 2022/01/21