Re: [PATCH RFC 7/8] libxl: introduce topology fields

[Joao Martins <joao.m.martins@oracle.com>]

On 02/25/2016 04:29 PM, Wei Liu wrote:
> On Mon, Feb 22, 2016 at 09:02:13PM +0000, Joao Martins wrote:
>> Currently there is "smt" option that changes from a flat core topology
>> to a core+thread topology. This patch adds more expressive options for
>> describing the topology as seen by the guest i.e. sockets, cores and
>> threads to adjust cpu topology as seen by the guest.
>>
>> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
>> ---
>> CC: Ian Jackson <ian.jackson@eu.citrix.com>
>> CC: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
>> CC: Ian Campbell <ian.campbell@citrix.com>
>> CC: Wei Liu <wei.liu2@citrix.com>
>> ---
>>  tools/libxl/libxl_dom.c     | 18 ++++++++++++------
>>  tools/libxl/libxl_types.idl |  4 ++++
>>  2 files changed, 16 insertions(+), 6 deletions(-)
>>
>> diff --git a/tools/libxl/libxl_dom.c b/tools/libxl/libxl_dom.c
>> index ff9356d..1e6d9ab 100644
>> --- a/tools/libxl/libxl_dom.c
>> +++ b/tools/libxl/libxl_dom.c
>> @@ -507,14 +507,20 @@ int libxl__build_post(libxl__gc *gc, uint32_t domid,
>>      }
>>  
>>      libxl_cpuid_apply_policy(ctx, domid);
>> -    if (info->type == LIBXL_DOMAIN_TYPE_HVM
>> -        && libxl_defbool_val(info->smt)) {
>> +    if (info->type == LIBXL_DOMAIN_TYPE_HVM) {
>>  
>> -        uint32_t threads = 0;
>> +        uint32_t threads = 0, cores = 0;
>> +
>> +        if (libxl_defbool_val(info->smt)
>> +            && !libxl__count_threads_per_core(gc, &threads))
>> +            cores = info->max_vcpus / threads;
>> +        else if (info->topology.cores) {
>> +            cores = info->topology.cores;
>> +            threads = info->topology.threads;
>> +        }
>>  
>> -        if (!libxl__count_threads_per_core(gc, &threads))
>> -            libxl__cpuid_set_topology(ctx, domid,
>> -                                      info->max_vcpus / threads, threads);
>> +        if (cores && threads)
>> +            libxl__cpuid_set_topology(ctx, domid, cores, threads);
>>      }
>>  
>>      if (info->cpuid != NULL)
>> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
>> index fa4725a..caba626 100644
>> --- a/tools/libxl/libxl_types.idl
>> +++ b/tools/libxl/libxl_types.idl
>> @@ -422,6 +422,10 @@ libxl_domain_build_info = Struct("domain_build_info",[
>>      ("vcpu_hard_affinity", Array(libxl_bitmap, "num_vcpu_hard_affinity")),
>>      ("vcpu_soft_affinity", Array(libxl_bitmap, "num_vcpu_soft_affinity")),
>>      ("smt",             libxl_defbool),
>> +    ("topology",        Struct(None, [("sockets", integer),
>> +                                      ("cores",   integer),
>> +                                      ("threads", integer),
>> +                                      ])),
> 
> Maybe having topology is a good enough sophisticated configuration
> interface? (See my previous email)
Yeah. That's what I think too, though I proposed both to see what you folks
would prefer. For other potentially users of libxl (libvirt) the topology
field(s) is also a better fit. BTW, the reason for the smt parameter was mostly
for the default case, where it's a single flat topology.

>>      ("numa_placement",  libxl_defbool),
>>      ("tsc_mode",        libxl_tsc_mode),
>>      ("max_memkb",       MemKB),
>> -- 
>> 2.1.4
>>

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