Re: [RFC 3/8] virtio: Add API to batch set host notifiers

[Stefan Hajnoczi]

On Thu, Mar 25, 2021 at 04:07:30PM +0100, Greg Kurz wrote:
> Introduce VirtioBusClass methods to begin and commit a transaction
> of setting/unsetting host notifiers. These handlers will be implemented
> by virtio-pci to batch addition and deletion of ioeventfds for multiqueue
> devices like virtio-scsi-pci or virtio-blk-pci.
> 
> Convert virtio_bus_set_host_notifiers() to use these handlers. Note that
> virtio_bus_cleanup_host_notifier() closes eventfds, which could still be
> passed to the KVM_IOEVENTFD ioctl() when the transaction ends and fail
> with EBADF. The cleanup of the host notifiers is thus pushed to a
> separate loop in virtio_bus_unset_and_cleanup_host_notifiers(), after
> transaction commit.
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---
>  include/hw/virtio/virtio-bus.h |  4 ++++
>  hw/virtio/virtio-bus.c         | 34 ++++++++++++++++++++++++++++++++++
>  2 files changed, 38 insertions(+)
> 
> diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
> index 6d1e4ee3e886..99704b2c090a 100644
> --- a/include/hw/virtio/virtio-bus.h
> +++ b/include/hw/virtio/virtio-bus.h
> @@ -82,6 +82,10 @@ struct VirtioBusClass {
>       */
>      int (*ioeventfd_assign)(DeviceState *d, EventNotifier *notifier,
>                              int n, bool assign);
> +
> +    void (*ioeventfd_assign_begin)(DeviceState *d);
> +    void (*ioeventfd_assign_commit)(DeviceState *d);

Please add doc comments for these new functions.

> +
>      /*
>       * Whether queue number n is enabled.
>       */
> diff --git a/hw/virtio/virtio-bus.c b/hw/virtio/virtio-bus.c
> index c9e7cdb5c161..156484c4ca14 100644
> --- a/hw/virtio/virtio-bus.c
> +++ b/hw/virtio/virtio-bus.c
> @@ -295,6 +295,28 @@ int virtio_bus_set_host_notifier(VirtioBusState *bus, 
> int n, bool assign)
>      return r;
>  }
>  
> +static void virtio_bus_set_host_notifier_begin(VirtioBusState *bus)
> +{
> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(bus);
> +    DeviceState *proxy = DEVICE(BUS(bus)->parent);
> +
> +    if (k->ioeventfd_assign_begin) {
> +        assert(k->ioeventfd_assign_commit);
> +        k->ioeventfd_assign_begin(proxy);
> +    }
> +}
> +
> +static void virtio_bus_set_host_notifier_commit(VirtioBusState *bus)
> +{
> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(bus);
> +    DeviceState *proxy = DEVICE(BUS(bus)->parent);
> +
> +    if (k->ioeventfd_assign_commit) {
> +        assert(k->ioeventfd_assign_begin);
> +        k->ioeventfd_assign_commit(proxy);
> +    }
> +}
> +
>  void virtio_bus_cleanup_host_notifier(VirtioBusState *bus, int n)
>  {
>      VirtIODevice *vdev = virtio_bus_get_device(bus);
> @@ -308,6 +330,7 @@ void virtio_bus_cleanup_host_notifier(VirtioBusState 
> *bus, int n)
>      event_notifier_cleanup(notifier);
>  }
>  
> +/* virtio_bus_set_host_notifier_begin() must have been called */
>  static void virtio_bus_unset_and_cleanup_host_notifiers(VirtioBusState *bus,
>                                                          int nvqs, int 
> n_offset)
>  {
> @@ -315,6 +338,10 @@ static void 
> virtio_bus_unset_and_cleanup_host_notifiers(VirtioBusState *bus,
>  
>      for (i = 0; i < nvqs; i++) {
>          virtio_bus_set_host_notifier(bus, i + n_offset, false);
> +    }
> +    /* Let address_space_update_ioeventfds() run before closing ioeventfds */

assert(memory_region_transaction_depth == 0)?

> +    virtio_bus_set_host_notifier_commit(bus);
> +    for (i = 0; i < nvqs; i++) {
>          virtio_bus_cleanup_host_notifier(bus, i + n_offset);
>      }
>  }
> @@ -327,17 +354,24 @@ int virtio_bus_set_host_notifiers(VirtioBusState *bus, 
> int nvqs, int n_offset,
>      int rc;
>  
>      if (assign) {
> +        virtio_bus_set_host_notifier_begin(bus);
> +
>          for (i = 0; i < nvqs; i++) {
>              rc = virtio_bus_set_host_notifier(bus, i + n_offset, true);
>              if (rc != 0) {
>                  warn_report_once("%s: Failed to set host notifier (%s).\n",
>                                   vdev->name, strerror(-rc));
>  
> +                /* This also calls virtio_bus_set_host_notifier_commit() */
>                  virtio_bus_unset_and_cleanup_host_notifiers(bus, i, 
> n_offset);
>                  return rc;
>              }
>          }
> +
> +        virtio_bus_set_host_notifier_commit(bus);
>      } else {
> +        virtio_bus_set_host_notifier_begin(bus);
> +        /* This also calls virtio_bus_set_host_notifier_commit() */
>          virtio_bus_unset_and_cleanup_host_notifiers(bus, nvqs, n_offset);
>      }
>  
> -- 
> 2.26.3
>

signature.asc
Description: PGP signature