Re: [PATCH] vhost-user: add separate memslot counter for vhost-user

[chenjiajun]

On 2020/10/2 10:05, Raphael Norwitz wrote:
> On Mon, Sep 28, 2020 at 9:17 AM Jiajun Chen <chenjiajun8@huawei.com> wrote:
>>
>> Used_memslots is equal to dev->mem->nregions now, it is true for
>> vhost kernel, but not for vhost user, which uses the memory regions
>> that have file descriptor. In fact, not all of the memory regions
>> have file descriptor.
>> It is usefully in some scenarios, e.g. used_memslots is 8, and only
>> 5 memory slots can be used by vhost user, it is failed to hot plug
>> a new memory RAM because vhost_has_free_slot just returned false,
>> but we can hot plug it safely in fact.
>>
>> --
>> ChangeList:
>> v3:
>> -make used_memslots a member of struct vhost_dev instead of a global static 
>> value
>>
>> v2:
>> -eliminating useless used_memslots_exceeded variable and 
>> used_memslots_is_exceeded() API
>>
>> v1:
>> -vhost-user: add separate memslot counter for vhost-user
>>
>> Signed-off-by: Jiajun Chen <chenjiajun8@huawei.com>
>> Signed-off-by: Jianjay Zhou <jianjay.zhou@huawei.com>
> 
> I'm happy with this from a vhost/vhost-user perspective. vhost-backend
> change looks good too. I'm a little confused by what's going on with
> net/vhost-user.c.
> 
>> ---
>>  hw/virtio/vhost-backend.c         | 12 ++++++++++
>>  hw/virtio/vhost-user.c            | 25 +++++++++++++++++++++
>>  hw/virtio/vhost.c                 | 37 +++++++++++++++++++++++--------
>>  include/hw/virtio/vhost-backend.h |  5 +++++
>>  include/hw/virtio/vhost.h         |  1 +
>>  net/vhost-user.c                  |  7 ++++++
>>  6 files changed, 78 insertions(+), 9 deletions(-)
>>
> 
>> diff --git a/net/vhost-user.c b/net/vhost-user.c
>> index 17532daaf3..7e93955537 100644
>> --- a/net/vhost-user.c
>> +++ b/net/vhost-user.c
>> @@ -20,6 +20,7 @@
>>  #include "qemu/error-report.h"
>>  #include "qemu/option.h"
>>  #include "trace.h"
>> +#include "include/hw/virtio/vhost.h"
>>
>>  typedef struct NetVhostUserState {
>>      NetClientState nc;
>> @@ -347,6 +348,12 @@ static int net_vhost_user_init(NetClientState *peer, 
>> const char *device,
>>          qemu_chr_fe_set_handlers(&s->chr, NULL, NULL,
>>                                   net_vhost_user_event, NULL, nc0->name, 
>> NULL,
>>                                   true);
> 
> Can you elaborate on this check here? What does it have to do with
> fixing memslots accounting? Maybe it should be in a separate change?
> 
When the number of virtual machine memslots exceeds the upper limit of the 
back-end support,
QEMU main thread may enters an endless loop and cannot process other requests.
And number of memslots will not automatically decrease, so add a check here to 
exit from loop
in this scenario. For the newly started virtual machine, boot fails; for the 
hot plug network card,
hot plug fails.
>> +
>> +        if (!vhost_has_free_slot()) {
>> +            error_report("used memslots exceeded the backend limit, quit "
>> +                         "loop");
>> +            goto err;
>> +        }
>>      } while (!s->started);
>>
>>      assert(s->vhost_net);
>> --
>> 2.27.0.dirty
>>
> .
>