[PATCH v1 0/2] vhost: memslot handling improvements

David Hildenbrand posted 2 patches 1 year, 2 months ago
Failed in applying to current master (apply log)
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>
There is a newer version of this series
hw/virtio/vhost-user.c            | 14 -----
hw/virtio/vhost-vdpa.c            |  1 -
hw/virtio/vhost.c                 | 85 ++++++++++++++++++++-----------
include/hw/virtio/vhost-backend.h |  4 --
4 files changed, 56 insertions(+), 48 deletions(-)
[PATCH v1 0/2] vhost: memslot handling improvements
Posted by David Hildenbrand 1 year, 2 months ago
Following up on my previous work to make virtio-mem consume multiple
memslots dynamically [1] that requires precise accounting between used vs.
reserved memslots, I realized that vhost makes this extra hard by
filtering out some memory region sections (so they don't consume a
memslot) in the vhost-user case, which messes up the whole memslot
accounting.

This series fixes what I found to be broken and prepares for more work on
[1]. Further, it cleanes up the merge checks that I consider unnecessary.

[1] https://lkml.kernel.org/r/20211027124531.57561-8-david@redhat.com

Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>

David Hildenbrand (2):
  vhost: Defer filtering memory sections until building the vhost memory
    structure
  vhost: Remove vhost_backend_can_merge() callback

 hw/virtio/vhost-user.c            | 14 -----
 hw/virtio/vhost-vdpa.c            |  1 -
 hw/virtio/vhost.c                 | 85 ++++++++++++++++++++-----------
 include/hw/virtio/vhost-backend.h |  4 --
 4 files changed, 56 insertions(+), 48 deletions(-)

-- 
2.39.1
Re: [PATCH v1 0/2] vhost: memslot handling improvements
Posted by Michael S. Tsirkin 1 year, 2 months ago
On Thu, Feb 16, 2023 at 12:47:50PM +0100, David Hildenbrand wrote:
> Following up on my previous work to make virtio-mem consume multiple
> memslots dynamically [1] that requires precise accounting between used vs.
> reserved memslots, I realized that vhost makes this extra hard by
> filtering out some memory region sections (so they don't consume a
> memslot) in the vhost-user case, which messes up the whole memslot
> accounting.
> 
> This series fixes what I found to be broken and prepares for more work on
> [1]. Further, it cleanes up the merge checks that I consider unnecessary.
> 
> [1] https://lkml.kernel.org/r/20211027124531.57561-8-david@redhat.com
> 
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>


Igor worked on memslots a lot previously and he asked for
a bit of time to review this, so I'll wait a bit before
applying.

> David Hildenbrand (2):
>   vhost: Defer filtering memory sections until building the vhost memory
>     structure
>   vhost: Remove vhost_backend_can_merge() callback
> 
>  hw/virtio/vhost-user.c            | 14 -----
>  hw/virtio/vhost-vdpa.c            |  1 -
>  hw/virtio/vhost.c                 | 85 ++++++++++++++++++++-----------
>  include/hw/virtio/vhost-backend.h |  4 --
>  4 files changed, 56 insertions(+), 48 deletions(-)
> 
> -- 
> 2.39.1
Re: [PATCH v1 0/2] vhost: memslot handling improvements
Posted by Igor Mammedov 1 year, 2 months ago
On Fri, 17 Feb 2023 09:20:27 -0500
"Michael S. Tsirkin" <mst@redhat.com> wrote:

> On Thu, Feb 16, 2023 at 12:47:50PM +0100, David Hildenbrand wrote:
> > Following up on my previous work to make virtio-mem consume multiple
> > memslots dynamically [1] that requires precise accounting between used vs.
> > reserved memslots, I realized that vhost makes this extra hard by
> > filtering out some memory region sections (so they don't consume a
> > memslot) in the vhost-user case, which messes up the whole memslot
> > accounting.
> > 
> > This series fixes what I found to be broken and prepares for more work on
> > [1]. Further, it cleanes up the merge checks that I consider unnecessary.
> > 
> > [1] https://lkml.kernel.org/r/20211027124531.57561-8-david@redhat.com
> > 
> > Cc: "Michael S. Tsirkin" <mst@redhat.com>
> > Cc: Stefan Hajnoczi <stefanha@redhat.com>
> > Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>  
> 
> 
> Igor worked on memslots a lot previously and he asked for
> a bit of time to review this, so I'll wait a bit before
> applying.

I've reviewed it as much as I could.
(That said, vhost mem map code was mostly rewritten by dgilbert,
since the last time I've touched it, so his review would be
more valuable in this case than mine)

> 
> > David Hildenbrand (2):
> >   vhost: Defer filtering memory sections until building the vhost memory
> >     structure
> >   vhost: Remove vhost_backend_can_merge() callback
> > 
> >  hw/virtio/vhost-user.c            | 14 -----
> >  hw/virtio/vhost-vdpa.c            |  1 -
> >  hw/virtio/vhost.c                 | 85 ++++++++++++++++++++-----------
> >  include/hw/virtio/vhost-backend.h |  4 --
> >  4 files changed, 56 insertions(+), 48 deletions(-)
> > 
> > -- 
> > 2.39.1  
> 
>
Re: [PATCH v1 0/2] vhost: memslot handling improvements
Posted by David Hildenbrand 1 year, 1 month ago
On 07.03.23 12:14, Igor Mammedov wrote:
> On Fri, 17 Feb 2023 09:20:27 -0500
> "Michael S. Tsirkin" <mst@redhat.com> wrote:
> 
>> On Thu, Feb 16, 2023 at 12:47:50PM +0100, David Hildenbrand wrote:
>>> Following up on my previous work to make virtio-mem consume multiple
>>> memslots dynamically [1] that requires precise accounting between used vs.
>>> reserved memslots, I realized that vhost makes this extra hard by
>>> filtering out some memory region sections (so they don't consume a
>>> memslot) in the vhost-user case, which messes up the whole memslot
>>> accounting.
>>>
>>> This series fixes what I found to be broken and prepares for more work on
>>> [1]. Further, it cleanes up the merge checks that I consider unnecessary.
>>>
>>> [1] https://lkml.kernel.org/r/20211027124531.57561-8-david@redhat.com
>>>
>>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>>> Cc: Stefan Hajnoczi <stefanha@redhat.com>
>>> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
>>
>>
>> Igor worked on memslots a lot previously and he asked for
>> a bit of time to review this, so I'll wait a bit before
>> applying.
> 
> I've reviewed it as much as I could.
> (That said, vhost mem map code was mostly rewritten by dgilbert,
> since the last time I've touched it, so his review would be
> more valuable in this case than mine)

Thanks for the review! I'll resend (extending the patch description) and 
will move patch #1 last, so we can decide if we want to leave that 
broken in corner cases.

-- 
Thanks,

David / dhildenb
Re: [PATCH v1 0/2] vhost: memslot handling improvements
Posted by David Hildenbrand 1 year, 2 months ago
On 17.02.23 15:20, Michael S. Tsirkin wrote:
> On Thu, Feb 16, 2023 at 12:47:50PM +0100, David Hildenbrand wrote:
>> Following up on my previous work to make virtio-mem consume multiple
>> memslots dynamically [1] that requires precise accounting between used vs.
>> reserved memslots, I realized that vhost makes this extra hard by
>> filtering out some memory region sections (so they don't consume a
>> memslot) in the vhost-user case, which messes up the whole memslot
>> accounting.
>>
>> This series fixes what I found to be broken and prepares for more work on
>> [1]. Further, it cleanes up the merge checks that I consider unnecessary.
>>
>> [1] https://lkml.kernel.org/r/20211027124531.57561-8-david@redhat.com
>>
>> Cc: "Michael S. Tsirkin" <mst@redhat.com>
>> Cc: Stefan Hajnoczi <stefanha@redhat.com>
>> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> 
> 
> Igor worked on memslots a lot previously and he asked for
> a bit of time to review this, so I'll wait a bit before
> applying.

Sure, no need to rush. I'm still working on the other bits of the 
virtio-mem approach.

-- 
Thanks,

David / dhildenb
Re: [PATCH v1 0/2] vhost: memslot handling improvements
Posted by Stefan Hajnoczi 1 year, 2 months ago
On Thu, Feb 16, 2023 at 12:47:50PM +0100, David Hildenbrand wrote:
> Following up on my previous work to make virtio-mem consume multiple
> memslots dynamically [1] that requires precise accounting between used vs.
> reserved memslots, I realized that vhost makes this extra hard by
> filtering out some memory region sections (so they don't consume a
> memslot) in the vhost-user case, which messes up the whole memslot
> accounting.
> 
> This series fixes what I found to be broken and prepares for more work on
> [1]. Further, it cleanes up the merge checks that I consider unnecessary.
> 
> [1] https://lkml.kernel.org/r/20211027124531.57561-8-david@redhat.com
> 
> Cc: "Michael S. Tsirkin" <mst@redhat.com>
> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> Cc: Dr. David Alan Gilbert <dgilbert@redhat.com>
> 
> David Hildenbrand (2):
>   vhost: Defer filtering memory sections until building the vhost memory
>     structure
>   vhost: Remove vhost_backend_can_merge() callback
> 
>  hw/virtio/vhost-user.c            | 14 -----
>  hw/virtio/vhost-vdpa.c            |  1 -
>  hw/virtio/vhost.c                 | 85 ++++++++++++++++++++-----------
>  include/hw/virtio/vhost-backend.h |  4 --
>  4 files changed, 56 insertions(+), 48 deletions(-)
> 
> -- 
> 2.39.1
> 

I'm not familiar enough with the memslot code to review this properly,
but overall it looks okay:

Acked-by: Stefan Hajnoczi <stefanha@redhat.com>
Re: [PATCH v1 0/2] vhost: memslot handling improvements
Posted by David Hildenbrand 1 year, 2 months ago
On 16.02.23 17:04, Stefan Hajnoczi wrote:
> Acked-by: Stefan Hajnoczi<stefanha@redhat.com>

Thanks!

-- 
Thanks,

David / dhildenb