On 02.01.24 16:53, Paolo Bonzini wrote:
On Tue, Jan 2, 2024 at 4:24 PM Hanna Czenczek<hre...@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