Re: [PATCH v4 2/5] softmmu: Support concurrent bounce buffers

[Mattias Nissler]

On Tue, Sep 19, 2023 at 7:14 PM Peter Xu <peterx@redhat.com> wrote:
>
> On Tue, Sep 19, 2023 at 09:08:10AM -0700, Mattias Nissler wrote:
> > @@ -3119,31 +3143,35 @@ void *address_space_map(AddressSpace *as,
> >  void address_space_unmap(AddressSpace *as, void *buffer, hwaddr len,
> >                           bool is_write, hwaddr access_len)
> >  {
> > -    if (buffer != as->bounce.buffer) {
> > -        MemoryRegion *mr;
> > -        ram_addr_t addr1;
> > +    MemoryRegion *mr;
> > +    ram_addr_t addr1;
> > +
> > +    mr = memory_region_from_host(buffer, &addr1);
> > +    if (mr == NULL) {
> > +        BounceBuffer *bounce = container_of(buffer, BounceBuffer, buffer);
> > +        assert(bounce->magic == BOUNCE_BUFFER_MAGIC);
> >
> > -        mr = memory_region_from_host(buffer, &addr1);
> > -        assert(mr != NULL);
> >          if (is_write) {
> > -            invalidate_and_set_dirty(mr, addr1, access_len);
> > -        }
> > -        if (xen_enabled()) {
> > -            xen_invalidate_map_cache_entry(buffer);
> > +            address_space_write(as, bounce->addr, MEMTXATTRS_UNSPECIFIED,
> > +                                bounce->buffer, access_len);
> >          }
> > -        memory_region_unref(mr);
> > +
> > +        memory_region_unref(bounce->mr);
> > +        qatomic_sub(&as->bounce_buffer_size, bounce->len);
> > +        /* Write bounce_buffer_size before reading map_client_list. */
> > +        smp_mb();
> > +        address_space_notify_map_clients(as);
> > +        bounce->magic = ~BOUNCE_BUFFER_MAGIC;
> > +        g_free(bounce);
> >          return;
> >      }
> > +
> > +    if (xen_enabled()) {
> > +        xen_invalidate_map_cache_entry(buffer);
> > +    }
> >      if (is_write) {
> > -        address_space_write(as, as->bounce.addr, MEMTXATTRS_UNSPECIFIED,
> > -                            as->bounce.buffer, access_len);
> > -    }
> > -    qemu_vfree(as->bounce.buffer);
> > -    as->bounce.buffer = NULL;
> > -    memory_region_unref(as->bounce.mr);
>
> This line needs to be kept?

Yes, good catch. Thanks!

>
> > -    /* Clear in_use before reading map_client_list.  */
> > -    qatomic_set_mb(&as->bounce.in_use, false);
> > -    address_space_notify_map_clients(as);
> > +        invalidate_and_set_dirty(mr, addr1, access_len);
> > +    }
> >  }
>
> --
> Peter Xu
>