Re: [RFC 3/3] aio-posix: call ->poll_end() when removing AioHandler

[Stefan Hajnoczi]

On Wed, Dec 13, 2023 at 10:52:58PM +0100, Paolo Bonzini wrote:
> On Wed, Dec 13, 2023 at 10:15 PM Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > -    /* If a read is in progress, just mark the node as deleted */
> > -    if (ctx->walking_handlers > 0) {
> > -        QLIST_INSERT_HEAD_RCU(&ctx->deleted_aio_handlers, node, 
> > node_deleted);
> > -        return false;
> > +    /* If polling was started on the node then end it now */
> > +    if (ctx->poll_started && node->io_poll_end) {
> > +        node->io_poll_end(node->opaque);
> > +
> > +        /* Poll one last time in case ->io_poll_end() raced with the event 
> > */
> > +        if (node->io_poll(node->opaque)) {
> > +            poll_ready = true;
> > +        }
> 
> Do you need this at all? If the caller is removing the handlers, they
> should have put themselves in a state where they don't care about the
> file descriptor becoming readable.

The caller no longer wishes to monitor the fd. This may be temporary
though. The fd must stay readable so that the caller can monitor it
again in the future and handle pending events.

->io_poll_begin() and ->io_poll_end() are used to bypass the fd while in
polling mode (e.g. disabling virtqueue kicks). This is a performance
optimization that avoids wasting cycles servicing fds when we're polling
anyway.

When the caller does:
1. aio_set_fd_handler(ctx, fd, NULL, ...)    // stop monitoring
2. aio_set_fd_handler(ctx, fd, read_fd, ...) // start monitoring

Then read_fd() should be called if the fd became readable between 1 and 2.

Since the fd may be bypassed until ->io_poll_end() returns, we must poll
one last time to check if an event snuck in right at the end without
making the fd readable. If polling detected an event, then we must do
something. We cannot drop the event.

(An alternative is to poll once before monitoring the fd again. That way
pending events are detected even if the fd wasn't readable. That is
currently not the way aio-posix.c works though.)

Stefan

signature.asc
Description: PGP signature