Re: [Xen-devel] [PATCH RFC 30/49] xen: let vcpu_create() select processor

[Juergen Gross <jgross@xxxxxxxx>]

On 29/03/2019 20:17, Andrew Cooper wrote:
> On 29/03/2019 15:09, Juergen Gross wrote:
>> Today there are two distinct scenarios for vcpu_create(): either for
>> creation of idle-domain vcpus (vcpuid == processor) or for creation of
>> "normal" domain vcpus (including dom0), where the caller selects the
>> initial processor on a round-robin scheme of the allowed processors
>> (allowed being based on cpupool and affinities).
>>
>> Instead of passing the initial processor to vcpu_create() and passing
>> on to sched_init_vcpu() let sched_init_vcpu() do the processor
>> selection. For supporting dom0 vcpu creation use the node_affinity of
>> the domain as a base for selecting the processors. User domains will
>> have initially all nodes set, so this is no different behavior compared
>> to today.
>>
>> Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> 
> Good riddance to the parameter!  This will definitely simply some of my
> further domcreate changes.
> 
>> index d9836779d1..d5294b0d26 100644
>> --- a/xen/arch/arm/domain_build.c
>> +++ b/xen/arch/arm/domain_build.c
>> @@ -1986,12 +1986,11 @@ static int __init construct_domain(struct domain *d, 
>> struct kernel_info *kinfo)
>>      }
>>  #endif
>>  
>> -    for ( i = 1, cpu = 0; i < d->max_vcpus; i++ )
>> +    for ( i = 1; i < d->max_vcpus; i++ )
>>      {
>> -        cpu = cpumask_cycle(cpu, &cpu_online_map);
>> -        if ( vcpu_create(d, i, cpu) == NULL )
>> +        if ( vcpu_create(d, i) == NULL )
>>          {
>> -            printk("Failed to allocate dom0 vcpu %d on pcpu %d\n", i, cpu);
>> +            printk("Failed to allocate dom0 vcpu %d\n", i);
> 
> Mind adjusting this to d0v%u as it is changing anyway?

Okay.

> 
>> diff --git a/xen/common/schedule.c b/xen/common/schedule.c
>> index ae2a6d0323..9b5527c1eb 100644
>> --- a/xen/common/schedule.c
>> +++ b/xen/common/schedule.c
>> @@ -318,14 +318,40 @@ static struct sched_item *sched_alloc_item(struct vcpu 
>> *v)
>>      return NULL;
>>  }
>>  
>> -int sched_init_vcpu(struct vcpu *v, unsigned int processor)
>> +static unsigned int sched_select_initial_cpu(struct vcpu *v)
>> +{
>> +    struct domain *d = v->domain;
>> +    nodeid_t node;
>> +    cpumask_t cpus;
>> +
>> +    cpumask_clear(&cpus);
>> +    for_each_node_mask ( node, d->node_affinity )
>> +        cpumask_or(&cpus, &cpus, &node_to_cpumask(node));
>> +    cpumask_and(&cpus, &cpus, cpupool_domain_cpumask(d));
>> +    if ( cpumask_empty(&cpus) )
>> +        cpumask_copy(&cpus, cpupool_domain_cpumask(d));
>> +
>> +    if ( v->vcpu_id == 0 )
>> +        return cpumask_first(&cpus);
>> +
>> +    /* We can rely on previous vcpu being available. */
> 
> Only if you ASSERT(!is_idle_domain(d)), which is safe given the sole caller.
> 
> idle->vcpu[] can be sparse in some corner cases.
> 
> Ideally with both of these suggestions, Acked-by: Andrew Cooper
> <andrew.cooper3@xxxxxxxxxx>

Thanks.


Juergen

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel