| Message ID | 20191025160444.31632-6-stefanha@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | io_uring: add Linux io_uring AIO engine | expand |
Am 25.10.2019 um 18:04 hat Stefan Hajnoczi geschrieben: > From: Aarushi Mehta <mehta.aaru20@gmail.com> > > Signed-off-by: Aarushi Mehta <mehta.aaru20@gmail.com> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> This commit message needs to answer at least where these stubs are actually used. Aren't all callers protected with #ifdef CONFIG_LINUX_IO_URING? (And if they aren't, why is abort() okay?) Kevin
On Mon, Nov 11, 2019 at 12:13:47PM +0100, Kevin Wolf wrote: > Am 25.10.2019 um 18:04 hat Stefan Hajnoczi geschrieben: > > From: Aarushi Mehta <mehta.aaru20@gmail.com> > > > > Signed-off-by: Aarushi Mehta <mehta.aaru20@gmail.com> > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > > This commit message needs to answer at least where these stubs are > actually used. Aren't all callers protected with #ifdef > CONFIG_LINUX_IO_URING? (And if they aren't, why is abort() okay?) Okay, will look into this. Stefan
On Mon, Nov 11, 2019 at 12:13:47PM +0100, Kevin Wolf wrote: > Am 25.10.2019 um 18:04 hat Stefan Hajnoczi geschrieben: > > From: Aarushi Mehta <mehta.aaru20@gmail.com> > > > > Signed-off-by: Aarushi Mehta <mehta.aaru20@gmail.com> > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > > This commit message needs to answer at least where these stubs are > actually used. Aren't all callers protected with #ifdef > CONFIG_LINUX_IO_URING? (And if they aren't, why is abort() okay?) Okay, I'll clarify in the commit description. I didn't find a program that actually requires these stubs today, but in theory they are required when: 1. A program links against util-obj-y but not block-obj-y (e.g. vhost-user-gpu) AND 2. It uses util/async.o, which depends on luring_*() APIs You can test this by adding a call to qemu_bh_new() into vhost-user-gpu's main.c: ld: libqemuutil.a(async.o): in function `aio_ctx_finalize': qemu/util/async.c:281: undefined reference to `luring_detach_aio_context' ld: /home/stefanha/qemu/util/async.c:282: undefined reference to `luring_cleanup' ld: libqemuutil.a(async.o): in function `aio_setup_linux_io_uring': qemu/util/async.c:358: undefined reference to `luring_init' ld: /home/stefanha/qemu/util/async.c:363: undefined reference to `luring_attach_aio_context' The program may have no intention of using io_uring, just the QEMU main loop and BH, so there is an argument for avoiding linking block-obj-y*. That's the purpose of the stubs and why abort() is okay. * It's possible to argue against this and personally I'm not that convinced by stubs for this scenario. But io_uring.o should be consistent with how things work today with linux-aio.o. If you feel strongly against having stubs then the linux-aio.o stubs should also be removed (see commit c2b38b277a788). Stefan
Am 03.12.2019 um 12:16 hat Stefan Hajnoczi geschrieben: > On Mon, Nov 11, 2019 at 12:13:47PM +0100, Kevin Wolf wrote: > > Am 25.10.2019 um 18:04 hat Stefan Hajnoczi geschrieben: > > > From: Aarushi Mehta <mehta.aaru20@gmail.com> > > > > > > Signed-off-by: Aarushi Mehta <mehta.aaru20@gmail.com> > > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > > > > This commit message needs to answer at least where these stubs are > > actually used. Aren't all callers protected with #ifdef > > CONFIG_LINUX_IO_URING? (And if they aren't, why is abort() okay?) > > Okay, I'll clarify in the commit description. > > I didn't find a program that actually requires these stubs today, but in > theory they are required when: > 1. A program links against util-obj-y but not block-obj-y (e.g. > vhost-user-gpu) > AND > 2. It uses util/async.o, which depends on luring_*() APIs > > You can test this by adding a call to qemu_bh_new() into > vhost-user-gpu's main.c: > > ld: libqemuutil.a(async.o): in function `aio_ctx_finalize': > qemu/util/async.c:281: undefined reference to `luring_detach_aio_context' > ld: /home/stefanha/qemu/util/async.c:282: undefined reference to `luring_cleanup' > ld: libqemuutil.a(async.o): in function `aio_setup_linux_io_uring': > qemu/util/async.c:358: undefined reference to `luring_init' > ld: /home/stefanha/qemu/util/async.c:363: undefined reference to `luring_attach_aio_context' > > The program may have no intention of using io_uring, just the QEMU main > loop and BH, so there is an argument for avoiding linking block-obj-y*. > That's the purpose of the stubs and why abort() is okay. Okay, make sense to me then. > * It's possible to argue against this and personally I'm not that > convinced by stubs for this scenario. But io_uring.o should be > consistent with how things work today with linux-aio.o. If you feel > strongly against having stubs then the linux-aio.o stubs should also be > removed (see commit c2b38b277a788). I don't really like having block-specific things like Linux AIO or io_uring in util/async.c, but given that they have per-AioContext state, it's not clearly wrong either. Maybe we could consider moving these functions to the file-posix driver, it's the only caller of them. But it's nothing that should stop this series. Kevin
On 03/12/19 15:28, Kevin Wolf wrote: >> * It's possible to argue against this and personally I'm not that >> convinced by stubs for this scenario. But io_uring.o should be >> consistent with how things work today with linux-aio.o. If you feel >> strongly against having stubs then the linux-aio.o stubs should also be >> removed (see commit c2b38b277a788). > I don't really like having block-specific things like Linux AIO or > io_uring in util/async.c, but given that they have per-AioContext state, > it's not clearly wrong either. A good reason to have AioContext in util/async.c is that nowadays Linux supports poll() via the AioContext ring too, and that would allow doing busy polling of network sockets. Paolo
diff --git a/MAINTAINERS b/MAINTAINERS index 31b090033c..c5cf2466b6 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -2551,6 +2551,7 @@ M: Stefan Hajnoczi <stefanha@redhat.com> L: qemu-block@nongnu.org S: Maintained F: block/io_uring.c +F: stubs/io_uring.c qcow2 M: Kevin Wolf <kwolf@redhat.com> diff --git a/stubs/Makefile.objs b/stubs/Makefile.objs index 4a50e95ec3..56c177c507 100644 --- a/stubs/Makefile.objs +++ b/stubs/Makefile.objs @@ -13,6 +13,7 @@ stub-obj-y += iothread.o stub-obj-y += iothread-lock.o stub-obj-y += is-daemonized.o stub-obj-$(CONFIG_LINUX_AIO) += linux-aio.o +stub-obj-$(CONFIG_LINUX_IO_URING) += io_uring.o stub-obj-y += machine-init-done.o stub-obj-y += migr-blocker.o stub-obj-y += change-state-handler.o diff --git a/stubs/io_uring.c b/stubs/io_uring.c new file mode 100644 index 0000000000..622d1e4648 --- /dev/null +++ b/stubs/io_uring.c @@ -0,0 +1,32 @@ +/* + * Linux io_uring support. + * + * Copyright (C) 2009 IBM, Corp. + * Copyright (C) 2009 Red Hat, Inc. + * + * This work is licensed under the terms of the GNU GPL, version 2 or later. + * See the COPYING file in the top-level directory. + */ +#include "qemu/osdep.h" +#include "block/aio.h" +#include "block/raw-aio.h" + +void luring_detach_aio_context(LuringState *s, AioContext *old_context) +{ + abort(); +} + +void luring_attach_aio_context(LuringState *s, AioContext *new_context) +{ + abort(); +} + +LuringState *luring_init(Error **errp) +{ + abort(); +} + +void luring_cleanup(LuringState *s) +{ + abort(); +}