[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC 3/8] virtio: Add API to batch set host notifiers
From: |
Stefan Hajnoczi |
Subject: |
Re: [RFC 3/8] virtio: Add API to batch set host notifiers |
Date: |
Tue, 30 Mar 2021 14:55:42 +0100 |
On Tue, Mar 30, 2021 at 12:17:40PM +0200, Greg Kurz wrote:
> On Mon, 29 Mar 2021 18:10:57 +0100
> Stefan Hajnoczi <stefanha@redhat.com> wrote:
> > On Thu, Mar 25, 2021 at 04:07:30PM +0100, Greg Kurz wrote:
> > > @@ -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)?
> >
>
> Hmm... appart from the fact that memory_region_transaction_depth is
> a memory internal thing that shouldn't be exposed here, it seems to
> me that memory_region_transaction_depth can be != 0 when, e.g. when
> batching is used... or I'm missing something ?
>
> I was actually thinking of adding some asserts for that in the
> memory_region_*_eventfd_full() functions introduced by patch 1.
>
> if (!transaction) {
> memory_region_transaction_begin();
> }
> assert(memory_region_transaction_depth != 0);
In that case is it safe to call virtio_bus_cleanup_host_notifier()
below? I thought it depends on the transaction committing first.
>
> > > + virtio_bus_set_host_notifier_commit(bus);
> > > + for (i = 0; i < nvqs; i++) {
> > > virtio_bus_cleanup_host_notifier(bus, i + n_offset);
> > > }
> > > }
signature.asc
Description: PGP signature
[RFC 4/8] virtio-pci: Batch add/del ioeventfds in a single MR transaction, Greg Kurz, 2021/03/25
[RFC 5/8] virtio-blk: Fix rollback path in virtio_blk_data_plane_start(), Greg Kurz, 2021/03/25
[RFC 6/8] virtio-blk: Use virtio_bus_set_host_notifiers(), Greg Kurz, 2021/03/25