Re: [PATCH 13/13] aio: remove aio_disable_external() API

2023-04-04 Thread Dr. David Alan Gilbert 
* Juan Quintela (quint...@redhat.com) wrote:
> Stefan Hajnoczi  wrote:
> > All callers now pass is_external=false to aio_set_fd_handler() and
> > aio_set_event_notifier(). The aio_disable_external() API that
> > temporarily disables fd handlers that were registered is_external=true
> > is therefore dead code.
> >
> > Remove aio_disable_external(), aio_enable_external(), and the
> > is_external arguments to aio_set_fd_handler() and
> > aio_set_event_notifier().
> >
> > The entire test-fdmon-epoll test is removed because its sole purpose was
> > testing aio_disable_external().
> >
> > Parts of this patch were generated using the following coccinelle
> > (https://coccinelle.lip6.fr/) semantic patch:
> >
> >   @@
> >   expression ctx, fd, is_external, io_read, io_write, io_poll, 
> > io_poll_ready, opaque;
> >   @@
> >   - aio_set_fd_handler(ctx, fd, is_external, io_read, io_write, io_poll, 
> > io_poll_ready, opaque)
> >   + aio_set_fd_handler(ctx, fd, io_read, io_write, io_poll, io_poll_ready, 
> > opaque)
> >
> >   @@
> >   expression ctx, notifier, is_external, io_read, io_poll, io_poll_ready;
> >   @@
> >   - aio_set_event_notifier(ctx, notifier, is_external, io_read, io_poll, 
> > io_poll_ready)
> >   + aio_set_event_notifier(ctx, notifier, io_read, io_poll, io_poll_ready)
> >
> > Signed-off-by: Stefan Hajnoczi 
> 
> []
> 
> > diff --git a/migration/rdma.c b/migration/rdma.c
> > index df646be35e..aee41ca43e 100644
> > --- a/migration/rdma.c
> > +++ b/migration/rdma.c
> > @@ -3104,15 +3104,15 @@ static void 
> > qio_channel_rdma_set_aio_fd_handler(QIOChannel *ioc,
> >  {
> >  QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(ioc);
> >  if (io_read) {
> > -aio_set_fd_handler(ctx, rioc->rdmain->recv_comp_channel->fd,
> > -   false, io_read, io_write, NULL, NULL, opaque);
> > -aio_set_fd_handler(ctx, rioc->rdmain->send_comp_channel->fd,
> > -   false, io_read, io_write, NULL, NULL, opaque);
> > +aio_set_fd_handler(ctx, rioc->rdmain->recv_comp_channel->fd, 
> > io_read,
> > +   io_write, NULL, NULL, opaque);
> > +aio_set_fd_handler(ctx, rioc->rdmain->send_comp_channel->fd, 
> > io_read,
> > +   io_write, NULL, NULL, opaque);
> >  } else {
> > -aio_set_fd_handler(ctx, rioc->rdmaout->recv_comp_channel->fd,
> > -   false, io_read, io_write, NULL, NULL, opaque);
> > -aio_set_fd_handler(ctx, rioc->rdmaout->send_comp_channel->fd,
> > -   false, io_read, io_write, NULL, NULL, opaque);
> > +aio_set_fd_handler(ctx, rioc->rdmaout->recv_comp_channel->fd, 
> > io_read,
> > +   io_write, NULL, NULL, opaque);
> > +aio_set_fd_handler(ctx, rioc->rdmaout->send_comp_channel->fd, 
> > io_read,
> > +   io_write, NULL, NULL, opaque);
> >  }
> >  }
> 
> Reviewed-by: Juan Quintela 
> 
> For the migration bits.
> I don't even want to know why the RDMA code uses a low level block layer API.

I don't think it's block specific.
It looks like it's because qio_channel uses aio in the case where
something QIO_CHANNEL_ERR_BLOCK and then waits for the recovery; see
4d9f675 that added it.

Dave
-- 
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK

Re: [PATCH 13/13] aio: remove aio_disable_external() API

2023-04-04 Thread Juan Quintela 
Stefan Hajnoczi  wrote:
> All callers now pass is_external=false to aio_set_fd_handler() and
> aio_set_event_notifier(). The aio_disable_external() API that
> temporarily disables fd handlers that were registered is_external=true
> is therefore dead code.
>
> Remove aio_disable_external(), aio_enable_external(), and the
> is_external arguments to aio_set_fd_handler() and
> aio_set_event_notifier().
>
> The entire test-fdmon-epoll test is removed because its sole purpose was
> testing aio_disable_external().
>
> Parts of this patch were generated using the following coccinelle
> (https://coccinelle.lip6.fr/) semantic patch:
>
>   @@
>   expression ctx, fd, is_external, io_read, io_write, io_poll, io_poll_ready, 
> opaque;
>   @@
>   - aio_set_fd_handler(ctx, fd, is_external, io_read, io_write, io_poll, 
> io_poll_ready, opaque)
>   + aio_set_fd_handler(ctx, fd, io_read, io_write, io_poll, io_poll_ready, 
> opaque)
>
>   @@
>   expression ctx, notifier, is_external, io_read, io_poll, io_poll_ready;
>   @@
>   - aio_set_event_notifier(ctx, notifier, is_external, io_read, io_poll, 
> io_poll_ready)
>   + aio_set_event_notifier(ctx, notifier, io_read, io_poll, io_poll_ready)
>
> Signed-off-by: Stefan Hajnoczi 

[]

> diff --git a/migration/rdma.c b/migration/rdma.c
> index df646be35e..aee41ca43e 100644
> --- a/migration/rdma.c
> +++ b/migration/rdma.c
> @@ -3104,15 +3104,15 @@ static void 
> qio_channel_rdma_set_aio_fd_handler(QIOChannel *ioc,
>  {
>  QIOChannelRDMA *rioc = QIO_CHANNEL_RDMA(ioc);
>  if (io_read) {
> -aio_set_fd_handler(ctx, rioc->rdmain->recv_comp_channel->fd,
> -   false, io_read, io_write, NULL, NULL, opaque);
> -aio_set_fd_handler(ctx, rioc->rdmain->send_comp_channel->fd,
> -   false, io_read, io_write, NULL, NULL, opaque);
> +aio_set_fd_handler(ctx, rioc->rdmain->recv_comp_channel->fd, io_read,
> +   io_write, NULL, NULL, opaque);
> +aio_set_fd_handler(ctx, rioc->rdmain->send_comp_channel->fd, io_read,
> +   io_write, NULL, NULL, opaque);
>  } else {
> -aio_set_fd_handler(ctx, rioc->rdmaout->recv_comp_channel->fd,
> -   false, io_read, io_write, NULL, NULL, opaque);
> -aio_set_fd_handler(ctx, rioc->rdmaout->send_comp_channel->fd,
> -   false, io_read, io_write, NULL, NULL, opaque);
> +aio_set_fd_handler(ctx, rioc->rdmaout->recv_comp_channel->fd, 
> io_read,
> +   io_write, NULL, NULL, opaque);
> +aio_set_fd_handler(ctx, rioc->rdmaout->send_comp_channel->fd, 
> io_read,
> +   io_write, NULL, NULL, opaque);
>  }
>  }

Reviewed-by: Juan Quintela 

For the migration bits.
I don't even want to know why the RDMA code uses a low level block layer API.

[PATCH 13/13] aio: remove aio_disable_external() API

2023-04-03 Thread Stefan Hajnoczi 
All callers now pass is_external=false to aio_set_fd_handler() and
aio_set_event_notifier(). The aio_disable_external() API that
temporarily disables fd handlers that were registered is_external=true
is therefore dead code.

Remove aio_disable_external(), aio_enable_external(), and the
is_external arguments to aio_set_fd_handler() and
aio_set_event_notifier().

The entire test-fdmon-epoll test is removed because its sole purpose was
testing aio_disable_external().

Parts of this patch were generated using the following coccinelle
(https://coccinelle.lip6.fr/) semantic patch:

  @@
  expression ctx, fd, is_external, io_read, io_write, io_poll, io_poll_ready, 
opaque;
  @@
  - aio_set_fd_handler(ctx, fd, is_external, io_read, io_write, io_poll, 
io_poll_ready, opaque)
  + aio_set_fd_handler(ctx, fd, io_read, io_write, io_poll, io_poll_ready, 
opaque)

  @@
  expression ctx, notifier, is_external, io_read, io_poll, io_poll_ready;
  @@
  - aio_set_event_notifier(ctx, notifier, is_external, io_read, io_poll, 
io_poll_ready)
  + aio_set_event_notifier(ctx, notifier, io_read, io_poll, io_poll_ready)

Signed-off-by: Stefan Hajnoczi 
---
 include/block/aio.h   | 55 --
 util/aio-posix.h  |  1 -
 block.c   |  7 
 block/blkio.c | 15 +++
 block/curl.c  | 10 ++---
 block/export/fuse.c   |  8 ++--
 block/export/vduse-blk.c  | 10 ++---
 block/io.c|  2 -
 block/io_uring.c  |  4 +-
 block/iscsi.c |  3 +-
 block/linux-aio.c |  4 +-
 block/nfs.c   |  5 +--
 block/nvme.c  |  8 ++--
 block/ssh.c   |  4 +-
 block/win32-aio.c |  6 +--
 hw/i386/kvm/xen_xenstore.c|  2 +-
 hw/virtio/virtio.c|  6 +--
 hw/xen/xen-bus.c  |  6 +--
 io/channel-command.c  |  6 +--
 io/channel-file.c |  3 +-
 io/channel-socket.c   |  3 +-
 migration/rdma.c  | 16 
 tests/unit/test-aio.c | 27 +
 tests/unit/test-fdmon-epoll.c | 73 ---
 util/aio-posix.c  | 20 +++---
 util/aio-win32.c  |  8 +---
 util/async.c  |  3 +-
 util/fdmon-epoll.c| 10 -
 util/fdmon-io_uring.c |  8 +---
 util/fdmon-poll.c |  3 +-
 util/main-loop.c  |  7 ++--
 util/qemu-coroutine-io.c  |  7 ++--
 util/vhost-user-server.c  | 11 +++---
 tests/unit/meson.build|  3 --
 34 files changed, 75 insertions(+), 289 deletions(-)
 delete mode 100644 tests/unit/test-fdmon-epoll.c

diff --git a/include/block/aio.h b/include/block/aio.h
index e267d918fd..d4ce01ea08 100644
--- a/include/block/aio.h
+++ b/include/block/aio.h
@@ -467,7 +467,6 @@ bool aio_poll(AioContext *ctx, bool blocking);
  */
 void aio_set_fd_handler(AioContext *ctx,
 int fd,
-bool is_external,
 IOHandler *io_read,
 IOHandler *io_write,
 AioPollFn *io_poll,
@@ -483,7 +482,6 @@ void aio_set_fd_handler(AioContext *ctx,
  */
 void aio_set_event_notifier(AioContext *ctx,
 EventNotifier *notifier,
-bool is_external,
 EventNotifierHandler *io_read,
 AioPollFn *io_poll,
 EventNotifierHandler *io_poll_ready);
@@ -612,59 +610,6 @@ static inline void aio_timer_init(AioContext *ctx,
  */
 int64_t aio_compute_timeout(AioContext *ctx);
 
-/**
- * aio_disable_external:
- * @ctx: the aio context
- *
- * Disable the further processing of external clients.
- */
-static inline void aio_disable_external(AioContext *ctx)
-{
-qatomic_inc(&ctx->external_disable_cnt);
-}
-
-/**
- * aio_enable_external:
- * @ctx: the aio context
- *
- * Enable the processing of external clients.
- */
-static inline void aio_enable_external(AioContext *ctx)
-{
-int old;
-
-old = qatomic_fetch_dec(&ctx->external_disable_cnt);
-assert(old > 0);
-if (old == 1) {
-/* Kick event loop so it re-arms file descriptors */
-aio_notify(ctx);
-}
-}
-
-/**
- * aio_external_disabled:
- * @ctx: the aio context
- *
- * Return true if the external clients are disabled.
- */
-static inline bool aio_external_disabled(AioContext *ctx)
-{
-return qatomic_read(&ctx->external_disable_cnt);
-}
-
-/**
- * aio_node_check:
- * @ctx: the aio context
- * @is_external: Whether or not the checked node is an external event source.
- *
- * Check if the node's is_external flag is okay to be polled by the ctx at this
- * moment. True means green light.
- */
-static inline bool aio_node_check(AioContext *ctx, bool is_external)
-{
-return !is_external || !qatomic_read(&ctx->external_disable_cnt);
-}
-
 /**
  * aio_co_schedule:
  * @ctx: the aio contex