You're right, I will reorder it in v3. The fix came last because I only discovered the deadlock while testing the shmem/blob support, but it makes sense to have it first since patch 2 depends on it.
On Fri, Jul 17, 2026 at 12:00 PM Michael S. Tsirkin <[email protected]> wrote: > On Fri, Jul 17, 2026 at 11:39:25AM +0200, Dorinda Bassey wrote: > > Commit the memory region transaction before sending the SHMEM_MAP reply > > so the KVM memory slot exists before the guest can access the mapped > > region. > > > > This moves the commit to while the backend is still blocked waiting for > > the reply, which can trigger ADD_MEM_REG back to the backend and > > deadlock. Filter shmem mapping regions out of vhost_section() by > > checking the MR owner type to prevent this. > > > > The explicit transaction begin/commit wrapping is dropped since > > virtio_add_shmem_map() already commits via memory_region_add_subregion(). > > > > Suggested-by: Albert Esteve <[email protected]> > > Signed-off-by: Dorinda Bassey <[email protected]> > > i do not get this ordering) of patches) why are there features and last > patch > is a bugfix? if this is a dependency it should come first. > > > --- > > hw/virtio/vhost-user.c | 20 ++------------------ > > hw/virtio/vhost.c | 10 ++++++++++ > > 2 files changed, 12 insertions(+), 18 deletions(-) > > > > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c > > index 517cc4ca716..01468f7441a 100644 > > --- a/hw/virtio/vhost-user.c > > +++ b/hw/virtio/vhost-user.c > > @@ -1977,8 +1977,6 @@ vhost_user_backend_handle_shmem_map(struct > vhost_dev *dev, > > } > > } > > > > - memory_region_transaction_begin(); > > - > > /* Create VirtioSharedMemoryMapping object */ > > VirtioSharedMemoryMapping *mapping = > virtio_shared_memory_mapping_new( > > vu_mmap->shmid, fd, vu_mmap->fd_offset, vu_mmap->shm_offset, > > @@ -1986,7 +1984,7 @@ vhost_user_backend_handle_shmem_map(struct > vhost_dev *dev, > > > > if (!mapping) { > > ret = -EFAULT; > > - goto send_reply_commit; > > + goto send_reply; > > } > > > > /* Add the mapping to the shared memory region */ > > @@ -1994,22 +1992,8 @@ vhost_user_backend_handle_shmem_map(struct > vhost_dev *dev, > > error_report("Failed to add shared memory mapping"); > > object_unref(OBJECT(mapping)); > > ret = -EFAULT; > > - goto send_reply_commit; > > - } > > - > > -send_reply_commit: > > - /* Send reply and commit after transaction started */ > > - if (hdr->flags & VHOST_USER_NEED_REPLY_MASK) { > > - payload->u64 = !!ret; > > - hdr->size = sizeof(payload->u64); > > - if (!vhost_user_send_resp(ioc, hdr, payload, &local_err)) { > > - error_report_err(local_err); > > - memory_region_transaction_commit(); > > - return -EFAULT; > > - } > > + goto send_reply; > > } > > - memory_region_transaction_commit(); > > - return 0; > > > > send_reply: > > if (hdr->flags & VHOST_USER_NEED_REPLY_MASK) { > > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c > > index af41841b529..e5c219be095 100644 > > --- a/hw/virtio/vhost.c > > +++ b/hw/virtio/vhost.c > > @@ -636,6 +636,16 @@ static bool vhost_section(struct vhost_dev *dev, > MemoryRegionSection *section) > > { > > MemoryRegion *mr = section->mr; > > > > + /* > > + * Skip shmem mapping regions, they are managed via SHMEM_MAP/UNMAP. > > + * Including them triggers ADD_MEM_REG during the SHMEM_MAP > transaction > > + * commit, deadlocking the backend. > > + */ > > + if (object_dynamic_cast(memory_region_owner(mr), > > + TYPE_VIRTIO_SHARED_MEMORY_MAPPING)) { > > + return false; > > + } > > + > > if (memory_region_is_ram(mr) && !memory_region_is_rom(mr)) { > > uint8_t dirty_mask = memory_region_get_dirty_log_mask(mr); > > uint8_t handled_dirty; > > -- > > 2.52.0 > >
