On Mon, 29 Mar 2021 18:03:49 +0100
Stefan Hajnoczi <stefanha@redhat.com> wrote:

> On Thu, Mar 25, 2021 at 04:07:28PM +0100, Greg Kurz wrote:
> > diff --git a/include/exec/memory.h b/include/exec/memory.h
> > index 5728a681b27d..98ed552e001c 100644
> > --- a/include/exec/memory.h
> > +++ b/include/exec/memory.h
> > @@ -1848,13 +1848,25 @@ void memory_region_clear_flush_coalesced(MemoryRegion *mr);
> >   * @match_data: whether to match against @data, instead of just @addr
> >   * @data: the data to match against the guest write
> >   * @e: event notifier to be triggered when @addr, @size, and @data all match.
> > + * @transaction: whether to start a transaction for the change
> 
> "start" is unclear. Does it begin a transaction and return with the
> transaction unfinished? I think instead the function performs the
> eventfd addition within a transaction. It would be nice to clarify this.
> 

What about: 

 * @transaction: if true, the eventfd is added within a nested transaction,
 *               if false, it is up to the caller to ensure this is called
 *               within a transaction.

> >   **/
> > -void memory_region_add_eventfd(MemoryRegion *mr,
> > -                               hwaddr addr,
> > -                               unsigned size,
> > -                               bool match_data,
> > -                               uint64_t data,
> > -                               EventNotifier *e);
> > +void memory_region_add_eventfd_full(MemoryRegion *mr,
> > +                                    hwaddr addr,
> > +                                    unsigned size,
> > +                                    bool match_data,
> > +                                    uint64_t data,
> > +                                    EventNotifier *e,
> > +                                    bool transaction);
> > +
> > +static inline void memory_region_add_eventfd(MemoryRegion *mr,
> > +                                             hwaddr addr,
> > +                                             unsigned size,
> > +                                             bool match_data,
> > +                                             uint64_t data,
> > +                                             EventNotifier *e)
> > +{
> > +    memory_region_add_eventfd_full(mr, addr, size, match_data, data, e, true);
> > +}
> >  
> >  /**
> >   * memory_region_del_eventfd: Cancel an eventfd.
> > @@ -1868,13 +1880,25 @@ void memory_region_add_eventfd(MemoryRegion *mr,
> >   * @match_data: whether to match against @data, instead of just @addr
> >   * @data: the data to match against the guest write
> >   * @e: event notifier to be triggered when @addr, @size, and @data all match.
> > + * @transaction: whether to start a transaction for the change
> 
> Same here.

and:

 * @transaction: if true, the eventfd is cancelled within a nested transaction,
 *               if false, it is up to the caller to ensure this is called
 *               within a transaction.

?