Re: [Xen-devel] [PATCH] common/vm_event: Initialize vm_event lists on domain creation

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 26/06/17 15:52, Tamas K Lengyel wrote:
> On Mon, Jun 26, 2017 at 3:48 AM, Razvan Cojocaru
> <rcojocaru@xxxxxxxxxxxxxxx> wrote:
>> Pending livepatch code wants to check if the vm_event wait queues
>> are active, and this is made harder by the fact that they were
>> previously only initialized some time after the domain was created,
>> in vm_event_enable(). This patch initializes the lists immediately
>> after xzalloc()ating the vm_event memory, in domain_create(), in
>> the newly added init_domain_vm_event() function.
>>
>> Signed-off-by: Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>
>> ---
>>  xen/common/domain.c        |  5 ++---
>>  xen/common/vm_event.c      | 23 ++++++++++++++++++++---
>>  xen/include/xen/vm_event.h |  2 ++
>>  3 files changed, 24 insertions(+), 6 deletions(-)
>>
>> diff --git a/xen/common/domain.c b/xen/common/domain.c
>> index b22aacc..89a8f1d 100644
>> --- a/xen/common/domain.c
>> +++ b/xen/common/domain.c
>> @@ -362,11 +362,10 @@ struct domain *domain_create(domid_t domid, unsigned 
>> int domcr_flags,
>>
>>          poolid = 0;
>>
>> -        err = -ENOMEM;
>> -        d->vm_event = xzalloc(struct vm_event_per_domain);
>> -        if ( !d->vm_event )
>> +        if ( (err = init_domain_vm_event(d)) != 0 )
>>              goto fail;
>>
>> +        err = -ENOMEM;
>>          d->pbuf = xzalloc_array(char, DOMAIN_PBUF_SIZE);
>>          if ( !d->pbuf )
>>              goto fail;
>> diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c
>> index 9291db6..294ddd7 100644
>> --- a/xen/common/vm_event.c
>> +++ b/xen/common/vm_event.c
>> @@ -39,6 +39,26 @@
>>  #define vm_event_ring_lock(_ved)       spin_lock(&(_ved)->ring_lock)
>>  #define vm_event_ring_unlock(_ved)     spin_unlock(&(_ved)->ring_lock)
>>
>> +int init_domain_vm_event(struct domain *d)
> We already have a vm_event_init_domain function so the naming of this
> one here is not a particularly good one. It also looks like to me
> these two functions could simply be merged..

They shouldn't be merged.

The current vm_event_init_domain() should probably be
init_domain_arch_vm_event(), as it deals with constructing
d->arch.vm_event, whereas this function deals with d->vm_event.

There is also a difference in timing; vm_event_init_domain() is called
when vm_event is started on the domain, not when the domain is
constructed.  IMO, the two should happen at the same time to be
consistent.  I'm not fussed at which point, as it would be fine (in
principle) for d->vm_event to be NULL in most cases.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel