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

[Hanna Czenczek]

On 02.01.24 16:53, Paolo Bonzini wrote:

> On Tue, Jan 2, 2024 at 4:24 PM Hanna Czenczek <hreitz@redhat.com> wrote:
> > I’ve attached the preliminary patch that I didn’t get to send (or test
> > much) last year.  Not sure if it has the same CPU-usage-spike issue
> > Fiona was seeing, the only functional difference is that I notify the vq
> > after attaching the notifiers instead of before.
> >       
> I think the patch makes sense and cleaning up the logic of aio_poll
> (which is one of those functions that grew and grew without much
> clarity into who did what) can be done on top.
>
> Just one small thing, the virtio_queue_notify_vq() call is required
> because the virtqueue interrupt and eventfd are edge-triggered rather
> than level-triggered; so perhaps it should be placed in the
> function(s) that establish the handlers,
> virtio_queue_aio_attach_host_notifier() and
> virtio_queue_aio_attach_host_notifier_no_poll()? Neither
> virtio_blk_drained_end() nor virtio_scsi_drained_end() are
> particularly special, and the comment applies just as well:
>
>     /*
>      * We will have ignored notifications about new requests from the guest
>      * while handlers were not attached, so "kick" the virt queue to process
>      * those requests now.
>      */
>     
    
    I wasn’t entirely whether we want to call notify_vq() if we have
    instated the handlers for the first time (in which case someone
    ought to look for existing unprocessed requests anyway), so I
    decided to limit it to drained_end.
    
    But considering that it must be safe to call notify_vq() right after
    instating the handlers (virtio_queue_host_notifier_read() may then
    be called after all), we might as well do so every time, yes.
    
    Hanna