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

[Claudio Fontana]

On 5/23/23 23:39, 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(-)
> 
> diff --git a/block/file-posix.c b/block/file-posix.c
> index 0ab158efba..0a29a6cc43 100644
> --- a/block/file-posix.c
> +++ b/block/file-posix.c
> @@ -227,6 +227,9 @@ typedef struct RawPosixAIOData {
>          struct {
>              unsigned long op;
>          } zone_mgmt;
> +        struct {
> +            struct stat *st;
> +        } fstat;
>      };
>  } RawPosixAIOData;
>  
> @@ -2644,6 +2647,34 @@ static void raw_close(BlockDriverState *bs)
>      }
>  }
>  
> +static int handle_aiocb_fstat(void *opaque)
> +{
> +    RawPosixAIOData *aiocb = opaque;
> +
> +    if (fstat(aiocb->aio_fildes, aiocb->fstat.st) < 0) {
> +        return -errno;
> +    }
> +
> +    return 0;
> +}
> +
> +static int coroutine_fn raw_co_fstat(BlockDriverState *bs, struct stat *st)
> +{
> +    BDRVRawState *s = bs->opaque;
> +    RawPosixAIOData acb;
> +
> +    acb = (RawPosixAIOData) {
> +        .bs             = bs,
> +        .aio_fildes     = s->fd,
> +        .aio_type       = QEMU_AIO_FSTAT,
> +        .fstat          = {
> +            .st = st,
> +        },
> +    };
> +
> +    return raw_thread_pool_submit(handle_aiocb_fstat, &acb);
> +}
> +
>  /**
>   * Truncates the given regular file @fd to @offset and, when growing, fills 
> the
>   * new space according to @prealloc.
> @@ -2883,11 +2914,14 @@ static int64_t coroutine_fn 
> raw_co_getlength(BlockDriverState *bs)
>  static int64_t coroutine_fn raw_co_get_allocated_file_size(BlockDriverState 
> *bs)
>  {
>      struct stat st;
> -    BDRVRawState *s = bs->opaque;
> +    int ret;
>  
> -    if (fstat(s->fd, &st) < 0) {
> -        return -errno;
> +    ret = raw_co_fstat(bs, &st);
> +
> +    if (ret) {
> +        return ret;
>      }
> +
>      return (int64_t)st.st_blocks * 512;
>  }
>  
> diff --git a/include/block/raw-aio.h b/include/block/raw-aio.h
> index 0fe85ade77..7dc6d24e21 100644
> --- a/include/block/raw-aio.h
> +++ b/include/block/raw-aio.h
> @@ -31,6 +31,7 @@
>  #define QEMU_AIO_ZONE_REPORT  0x0100
>  #define QEMU_AIO_ZONE_MGMT    0x0200
>  #define QEMU_AIO_ZONE_APPEND  0x0400
> +#define QEMU_AIO_FSTAT        0x0800
>  #define QEMU_AIO_TYPE_MASK \
>          (QEMU_AIO_READ | \
>           QEMU_AIO_WRITE | \
> @@ -42,7 +43,8 @@
>           QEMU_AIO_TRUNCATE | \
>           QEMU_AIO_ZONE_REPORT | \
>           QEMU_AIO_ZONE_MGMT | \
> -         QEMU_AIO_ZONE_APPEND)
> +         QEMU_AIO_ZONE_APPEND | \
> +         QEMU_AIO_FSTAT)
>  
>  /* AIO flags */
>  #define QEMU_AIO_MISALIGNED   0x1000


Reviewed-by: Claudio Fontana <cfontana@suse.de>