Re: [Qemu-devel] [RFC] Replace posix-aio with custom thread pool

["Blue Swirl" <blauwirbel@gmail.com>]

On 12/5/08, Anthony Liguori <aliguori@us.ibm.com> wrote:
> glibc implements posix-aio as a thread pool and imposes a number of limitations.
>
>  1) it limits one request per-file descriptor.  we hack around this by dup()'ing
>  file descriptors which is hideously ugly
>
>  2) it's impossible to add new interfaces and we need a vectored read/write
>  operation to properly support a zero-copy API.
>
>  What has been suggested to me by glibc folks, is to implement whatever new
>  interfaces we want and then it can eventually be proposed for standardization.
>  This requires that we implement our own posix-aio implementation though.
>
>  This patch implements posix-aio using pthreads.  It immediately eliminates the
>  need for fd pooling.
>
>  It performs at least as well as the current posix-aio code (in some
>  circumstances, even better).
>
>  My only concern here is non-Linux Unices like FreeBSD.  They have kernel support
>  for posix-aio.  Since we cannot extend those interfaces though, I think that
>  even on those platforms we should still use a thread pool.
>
>  Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

>  @@ -895,6 +824,7 @@ BlockDriver bdrv_raw = {
>      .bdrv_aio_cancel = raw_aio_cancel,
>      .aiocb_size = sizeof(RawAIOCB),
>   #endif
>  +
>  @@ -1252,6 +1178,7 @@ BlockDriver bdrv_host_device = {
>      .bdrv_aio_cancel = raw_aio_cancel,
>      .aiocb_size = sizeof(RawAIOCB),
>   #endif
>  +

Some cleanup needed here?

>  +int _compat_aio_init(struct aioinit *aioinit)
>  +static int _compat_aio_submit(struct aiocb *aiocb, int is_write)
>  +int _compat_aio_read(struct aiocb *aiocb)
>  +int _compat_aio_write(struct aiocb *aiocb)
>  +ssize_t _compat_aio_return(struct aiocb *aiocb)
>  +int _compat_aio_error(struct aiocb *aiocb)
>  +int _compat_aio_cancel(int fd, struct aiocb *aiocb)

The names should not begin with an underscore.

>  +struct aiocb
>  +{
>  +    int aio_fildes;
>  +    void *aio_buf;
>  +    size_t aio_nbytes;
>  +    struct sigevent aio_sigevent;
>  +    off_t aio_offset;
>  +
>  +    /* private */
>  +    TAILQ_ENTRY(aiocb) node;
>  +    int is_write;
>  +    ssize_t ret;
>  +    int active;
>  +};
>  +
>  +struct aioinit
>  +{
>  +    int aio_threads;
>  +    int aio_num;
>  +    int aio_idle_time;
>  +};

These structs should probably be named qemu_aiocb and qemu_aioinit to
avoid conflict with system types.

I like to use unsigned types whenever possible, IIRC compilers may
generate better code with those.