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

[Hanna Czenczek <hreitz@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 1709 bytes --]

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

[-- Attachment #2: Type: text/html, Size: 2307 bytes --]