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

[Stefan Hajnoczi]

On Thu, May 04, 2023 at 11:34:17PM +0200, Kevin Wolf wrote:
> Am 25.04.2023 um 19:27 hat Stefan Hajnoczi geschrieben:
> > 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)
> > 
> > Reviewed-by: Juan Quintela <quintela@redhat.com>
> > Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
> 
> > diff --git a/util/fdmon-epoll.c b/util/fdmon-epoll.c
> > index 1683aa1105..6b6a1a91f8 100644
> > --- a/util/fdmon-epoll.c
> > +++ b/util/fdmon-epoll.c
> > @@ -133,13 +128,12 @@ bool fdmon_epoll_try_upgrade(AioContext *ctx, 
> > unsigned npfd)
> >          return false;
> >      }
> >  
> > -    /* Do not upgrade while external clients are disabled */
> > -    if (qatomic_read(&ctx->external_disable_cnt)) {
> > -        return false;
> > -    }
> > -
> > -    if (npfd < EPOLL_ENABLE_THRESHOLD) {
> > -        return false;
> > +    if (npfd >= EPOLL_ENABLE_THRESHOLD) {
> > +        if (fdmon_epoll_try_enable(ctx)) {
> > +            return true;
> > +        } else {
> > +            fdmon_epoll_disable(ctx);
> > +        }
> >      }
> >  
> >      /* The list must not change while we add fds to epoll */
> 
> I don't understand this hunk. Why are you changing more than just
> deleting the external_disable_cnt check?
> 
> Is this a mismerge with your own commit e62da985?

Yes, it's a mismerge. Thanks for catching that!

Stefan

signature.asc
Description: PGP signature