Re: [RFC PATCH 6/6] block: Add a thread-pool version of fstat

[Fabiano Rosas <farosas@suse.de>]

Eric Blake <eblake@redhat.com> writes:

> On Tue, May 23, 2023 at 06:39:03PM -0300, Fabiano Rosas wrote:
>> From: João Silva <jsilva@suse.de>
>> 
>> The fstat call can take a long time to finish when running over
>> NFS. Add a version of it that runs in the thread pool.
>> 
>> Adapt one of its users, raw_co_get_allocated_file size to use the new
>> version. That function is called via QMP under the qemu_global_mutex
>> so it has a large chance of blocking VCPU threads in case it takes too
>> long to finish.
>> 
>> Signed-off-by: João Silva <jsilva@suse.de>
>> Signed-off-by: Fabiano Rosas <farosas@suse.de>
>> ---
>>  block/file-posix.c      | 40 +++++++++++++++++++++++++++++++++++++---
>>  include/block/raw-aio.h |  4 +++-
>>  2 files changed, 40 insertions(+), 4 deletions(-)
>
> Should this change occur earlier in the series, before calling
> commands are marked with QAPI coroutine flags?  Otherwise, you have a
> bisection bug, where something marked coroutine can end up hanging
> when it calls a blocking syscall in the wrong context without the help
> of this patch offloading the syscall into a helper thread.

Hmm, I'm not sure. To submit the work to the thread pool we need to be
in a coroutine already. If the syscall blocks for too long we'd be
trading blocking the coroutine vs. blocking a vcpu thread anyway.

I have tested each patch to avoid bisection issues, but maybe it would
be warranted to merge both parts into a single patch. Or arrange them in
some other way... I'll experiment with it.