subprojects/libvhost-user/libvhost-user.c | 593 ++++++++++++---------- subprojects/libvhost-user/libvhost-user.h | 10 +- 2 files changed, 332 insertions(+), 271 deletions(-)
This series adds support for more memslots (509) to libvhost-user, to
make it fully compatible with virtio-mem that uses up to 256 memslots
accross all memory devices in "dynamic-memslot" mode (more details
in patch #3).
One simple fix upfront.
With that in place, this series optimizes and extens memory region
handling in libvhost-user:
* Heavily deduplicate and clean up the memory region handling code
* Speeds up GPA->VA translation with many memslots using binary search
* Optimize mmap_offset handling to use it as fd_offset for mmap()
* Avoid ring remapping when adding a single memory region
* Avoid dumping all guest memory, possibly allocating memory in sparse
memory mappings when the process crashes
I'm being very careful to not break some weird corner case that modern
QEMU might no longer trigger, but older one could have triggered or some
other frontend might trigger.
The only thing where I am not careful is to forbid memory regions that
overlap in GPA space: it doesn't make any sense.
With this series, virtio-mem (with dynamic-memslots=on) +
qemu-storage-daemon works flawlessly and as expected in my tests.
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Jason Wang <jasowang@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Stefano Garzarella <sgarzare@redhat.com>
Cc: Germano Veit Michel <germano@redhat.com>
Cc: Raphael Norwitz <raphael.norwitz@nutanix.com>
David Hildenbrand (15):
libvhost-user: Fix msg_region->userspace_addr computation
libvhost-user: Dynamically allocate memory for memory slots
libvhost-user: Bump up VHOST_USER_MAX_RAM_SLOTS to 509
libvhost-user: Factor out removing all mem regions
libvhost-user: Merge vu_set_mem_table_exec_postcopy() into
vu_set_mem_table_exec()
libvhost-user: Factor out adding a memory region
libvhost-user: No need to check for NULL when unmapping
libvhost-user: Don't zero out memory for memory regions
libvhost-user: Don't search for duplicates when removing memory
regions
libvhost-user: Factor out search for memory region by GPA and simplify
libvhost-user: Speedup gpa_to_mem_region() and vu_gpa_to_va()
libvhost-user: Use most of mmap_offset as fd_offset
libvhost-user: Factor out vq usability check
libvhost-user: Dynamically remap rings after (temporarily?) removing
memory regions
libvhost-user: Mark mmap'ed region memory as MADV_DONTDUMP
subprojects/libvhost-user/libvhost-user.c | 593 ++++++++++++----------
subprojects/libvhost-user/libvhost-user.h | 10 +-
2 files changed, 332 insertions(+), 271 deletions(-)
--
2.43.0
On Fri, Feb 02, 2024 at 10:53:17PM +0100, David Hildenbrand wrote: >This series adds support for more memslots (509) to libvhost-user, to >make it fully compatible with virtio-mem that uses up to 256 memslots >accross all memory devices in "dynamic-memslot" mode (more details >in patch #3). > >One simple fix upfront. > >With that in place, this series optimizes and extens memory region >handling in libvhost-user: >* Heavily deduplicate and clean up the memory region handling code >* Speeds up GPA->VA translation with many memslots using binary search >* Optimize mmap_offset handling to use it as fd_offset for mmap() >* Avoid ring remapping when adding a single memory region >* Avoid dumping all guest memory, possibly allocating memory in sparse > memory mappings when the process crashes > >I'm being very careful to not break some weird corner case that modern >QEMU might no longer trigger, but older one could have triggered or some >other frontend might trigger. > >The only thing where I am not careful is to forbid memory regions that >overlap in GPA space: it doesn't make any sense. > >With this series, virtio-mem (with dynamic-memslots=on) + >qemu-storage-daemon works flawlessly and as expected in my tests. I don't know much about this code, but I didn't find anything wrong with it. Thank you also for the great cleanup! Acked-by: Stefano Garzarella <sgarzare@redhat.com>
On 07.02.24 12:40, Stefano Garzarella wrote: > On Fri, Feb 02, 2024 at 10:53:17PM +0100, David Hildenbrand wrote: >> This series adds support for more memslots (509) to libvhost-user, to >> make it fully compatible with virtio-mem that uses up to 256 memslots >> accross all memory devices in "dynamic-memslot" mode (more details >> in patch #3). >> >> One simple fix upfront. >> >> With that in place, this series optimizes and extens memory region >> handling in libvhost-user: >> * Heavily deduplicate and clean up the memory region handling code >> * Speeds up GPA->VA translation with many memslots using binary search >> * Optimize mmap_offset handling to use it as fd_offset for mmap() >> * Avoid ring remapping when adding a single memory region >> * Avoid dumping all guest memory, possibly allocating memory in sparse >> memory mappings when the process crashes >> >> I'm being very careful to not break some weird corner case that modern >> QEMU might no longer trigger, but older one could have triggered or some >> other frontend might trigger. >> >> The only thing where I am not careful is to forbid memory regions that >> overlap in GPA space: it doesn't make any sense. >> >> With this series, virtio-mem (with dynamic-memslots=on) + >> qemu-storage-daemon works flawlessly and as expected in my tests. > > I don't know much about this code, but I didn't find anything wrong with > it. Thank you also for the great cleanup! > > Acked-by: Stefano Garzarella <sgarzare@redhat.com> Thanks Stefano for the review, highly appreciated! -- Cheers, David / dhildenb
On Fri, Feb 02, 2024 at 10:53:17PM +0100, David Hildenbrand wrote: > This series adds support for more memslots (509) to libvhost-user, to > make it fully compatible with virtio-mem that uses up to 256 memslots > accross all memory devices in "dynamic-memslot" mode (more details > in patch #3). Breaks build on some systems. E.g. https://gitlab.com/mstredhat/qemu/-/jobs/6163591599 > One simple fix upfront. > > With that in place, this series optimizes and extens memory region > handling in libvhost-user: > * Heavily deduplicate and clean up the memory region handling code > * Speeds up GPA->VA translation with many memslots using binary search > * Optimize mmap_offset handling to use it as fd_offset for mmap() > * Avoid ring remapping when adding a single memory region > * Avoid dumping all guest memory, possibly allocating memory in sparse > memory mappings when the process crashes > > I'm being very careful to not break some weird corner case that modern > QEMU might no longer trigger, but older one could have triggered or some > other frontend might trigger. > > The only thing where I am not careful is to forbid memory regions that > overlap in GPA space: it doesn't make any sense. > > With this series, virtio-mem (with dynamic-memslots=on) + > qemu-storage-daemon works flawlessly and as expected in my tests. > > Cc: Michael S. Tsirkin <mst@redhat.com> > Cc: Jason Wang <jasowang@redhat.com> > Cc: Stefan Hajnoczi <stefanha@redhat.com> > Cc: Stefano Garzarella <sgarzare@redhat.com> > Cc: Germano Veit Michel <germano@redhat.com> > Cc: Raphael Norwitz <raphael.norwitz@nutanix.com> > > David Hildenbrand (15): > libvhost-user: Fix msg_region->userspace_addr computation > libvhost-user: Dynamically allocate memory for memory slots > libvhost-user: Bump up VHOST_USER_MAX_RAM_SLOTS to 509 > libvhost-user: Factor out removing all mem regions > libvhost-user: Merge vu_set_mem_table_exec_postcopy() into > vu_set_mem_table_exec() > libvhost-user: Factor out adding a memory region > libvhost-user: No need to check for NULL when unmapping > libvhost-user: Don't zero out memory for memory regions > libvhost-user: Don't search for duplicates when removing memory > regions > libvhost-user: Factor out search for memory region by GPA and simplify > libvhost-user: Speedup gpa_to_mem_region() and vu_gpa_to_va() > libvhost-user: Use most of mmap_offset as fd_offset > libvhost-user: Factor out vq usability check > libvhost-user: Dynamically remap rings after (temporarily?) removing > memory regions > libvhost-user: Mark mmap'ed region memory as MADV_DONTDUMP > > subprojects/libvhost-user/libvhost-user.c | 593 ++++++++++++---------- > subprojects/libvhost-user/libvhost-user.h | 10 +- > 2 files changed, 332 insertions(+), 271 deletions(-) > > -- > 2.43.0
On 13.02.24 18:33, Michael S. Tsirkin wrote:
> On Fri, Feb 02, 2024 at 10:53:17PM +0100, David Hildenbrand wrote:
>> This series adds support for more memslots (509) to libvhost-user, to
>> make it fully compatible with virtio-mem that uses up to 256 memslots
>> accross all memory devices in "dynamic-memslot" mode (more details
>> in patch #3).
>
>
> Breaks build on some systems. E.g.
> https://gitlab.com/mstredhat/qemu/-/jobs/6163591599
>
>
./subprojects/libvhost-user/libvhost-user.c:369:27: error: comparison of
integer expressions of different signedness: ‘long int’ and ‘unsigned
int’ [-Werror=sign-compare]
369 | if (!ret && fs.f_type == HUGETLBFS_MAGIC) {
| ^~
So easy to fix in v2, thanks!
--
Cheers,
David / dhildenb
On Tue, Feb 13, 2024 at 07:27:44PM +0100, David Hildenbrand wrote:
> On 13.02.24 18:33, Michael S. Tsirkin wrote:
> > On Fri, Feb 02, 2024 at 10:53:17PM +0100, David Hildenbrand wrote:
> > > This series adds support for more memslots (509) to libvhost-user, to
> > > make it fully compatible with virtio-mem that uses up to 256 memslots
> > > accross all memory devices in "dynamic-memslot" mode (more details
> > > in patch #3).
> >
> >
> > Breaks build on some systems. E.g.
> > https://gitlab.com/mstredhat/qemu/-/jobs/6163591599
> >
> >
>
> ./subprojects/libvhost-user/libvhost-user.c:369:27: error: comparison of
> integer expressions of different signedness: ‘long int’ and ‘unsigned int’
> [-Werror=sign-compare]
> 369 | if (!ret && fs.f_type == HUGETLBFS_MAGIC) {
> | ^~
>
> So easy to fix in v2, thanks!
I think there is another problem around plugins though.
> --
> Cheers,
>
> David / dhildenb
On 13.02.24 19:55, Michael S. Tsirkin wrote:
> On Tue, Feb 13, 2024 at 07:27:44PM +0100, David Hildenbrand wrote:
>> On 13.02.24 18:33, Michael S. Tsirkin wrote:
>>> On Fri, Feb 02, 2024 at 10:53:17PM +0100, David Hildenbrand wrote:
>>>> This series adds support for more memslots (509) to libvhost-user, to
>>>> make it fully compatible with virtio-mem that uses up to 256 memslots
>>>> accross all memory devices in "dynamic-memslot" mode (more details
>>>> in patch #3).
>>>
>>>
>>> Breaks build on some systems. E.g.
>>> https://gitlab.com/mstredhat/qemu/-/jobs/6163591599
>>>
>>>
>>
>> ./subprojects/libvhost-user/libvhost-user.c:369:27: error: comparison of
>> integer expressions of different signedness: ‘long int’ and ‘unsigned int’
>> [-Werror=sign-compare]
>> 369 | if (!ret && fs.f_type == HUGETLBFS_MAGIC) {
>> | ^~
>>
>> So easy to fix in v2, thanks!
>
>
> I think there is another problem around plugins though.
There is a wrong checkpatch error:
https://gitlab.com/mstredhat/qemu/-/jobs/6162397277
d96f29518232719b0c444ab93913e8515a6cb5c6:100: ERROR: use
qemu_real_host_page_size() instead of getpagesize()
total: 1 errors, 1 warnings, 81 lines checked
qemu_real_host_page_size() is not available in libvhost-user. But I
could just change that code to not require getpagesize() at all.
Apart from that, I don't spot anything libvhost-user related (some qtest
timeouts, a "error_setv: Assertion `*errp == NULL' failed."). Did I miss
something?
--
Cheers,
David / dhildenb
© 2016 - 2024 Red Hat, Inc.