[PATCH v2 0/2] fix smt=0 fallout

[PATCH v2 0/2] fix smt=0 fallout

[Juergen Gross]

With smt=0 some output of xl is wrong. Fix it.

Juergen Gross (2):
  tools/libxl: correct vcpu affinity output with sparse physical cpu map
  xen: fill topology info for online cpus only

Changes in V2:
- patch 2: print info for offline cpus, too (Jan Beulich)

Juergen Gross (2):
  tools/libxl: correct vcpu affinity output with sparse physical cpu map
  xen: fill topology info for all present cpus

 tools/xl/xl_vcpu.c  | 4 ++--
 xen/common/sysctl.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

-- 
2.16.4


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

[PATCH v2 1/2] tools/libxl: correct vcpu affinity output with sparse physical cpu map

[Juergen Gross]

With not all physical cpus online (e.g. with smt=0) the output of hte
vcpu affinities is wrong, as the affinity bitmaps are capped after
nr_cpus bits, instead of using max_cpu_id.

Signed-off-by: Juergen Gross <jgross@suse.com>
Acked-by: Wei Liu <wei.liu2@citrix.com>
---
 tools/xl/xl_vcpu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/xl/xl_vcpu.c b/tools/xl/xl_vcpu.c
index 3384eeed06..c877f2595f 100644
--- a/tools/xl/xl_vcpu.c
+++ b/tools/xl/xl_vcpu.c
@@ -144,13 +144,13 @@ static void vcpulist(int argc, char **argv)
         }
 
         for (i = 0; i<nb_domain; i++)
-            print_domain_vcpuinfo(dominfo[i].domid, physinfo.nr_cpus);
+            print_domain_vcpuinfo(dominfo[i].domid, physinfo.max_cpu_id + 1);
 
         libxl_dominfo_list_free(dominfo, nb_domain);
     } else {
         for (; argc > 0; ++argv, --argc) {
             uint32_t domid = find_domain(*argv);
-            print_domain_vcpuinfo(domid, physinfo.nr_cpus);
+            print_domain_vcpuinfo(domid, physinfo.max_cpu_id + 1);
         }
     }
   vcpulist_out:
-- 
2.16.4


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

[PATCH v2 2/2] xen: fill topology info for all present cpus

[Juergen Gross]

The topology information obtainable via XEN_SYSCTL_cputopoinfo is
filled rather weird: the size of the array is derived from the highest
online cpu number, so in case there are trailing offline cpus they
will not be included.

On a dual core system with 4 threads booted with smt=0 without this
patch xl info -n will print:

cpu_topology           :
cpu:    core    socket     node
  0:       0        0        0
  1:       0        0        0
  2:       1        0        0

while with this patch the output is:

cpu_topology           :
cpu:    core    socket     node
  0:       0        0        0
  1:       0        0        0
  2:       1        0        0
  3:       1        0        0

Signed-off-by: Juergen Gross <jgross@suse.com>
---
 xen/common/sysctl.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
index 8e83c33a16..c0aa6bde4e 100644
--- a/xen/common/sysctl.c
+++ b/xen/common/sysctl.c
@@ -349,7 +349,7 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) u_sysctl)
         unsigned int i, num_cpus;
         struct xen_sysctl_cputopoinfo *ti = &op->u.cputopoinfo;
 
-        num_cpus = cpumask_last(&cpu_online_map) + 1;
+        num_cpus = cpumask_last(&cpu_present_map) + 1;
         if ( !guest_handle_is_null(ti->cputopo) )
         {
             struct xen_sysctl_cputopo cputopo = { };
-- 
2.16.4


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

Re: [PATCH v2 2/2] xen: fill topology info for all present cpus

[Wei Liu]

On Fri, Aug 31, 2018 at 05:22:05PM +0200, Juergen Gross wrote:
> The topology information obtainable via XEN_SYSCTL_cputopoinfo is
> filled rather weird: the size of the array is derived from the highest
> online cpu number, so in case there are trailing offline cpus they
> will not be included.
> 
> On a dual core system with 4 threads booted with smt=0 without this
> patch xl info -n will print:
> 
> cpu_topology           :
> cpu:    core    socket     node
>   0:       0        0        0
>   1:       0        0        0
>   2:       1        0        0
> 
> while with this patch the output is:
> 
> cpu_topology           :
> cpu:    core    socket     node
>   0:       0        0        0
>   1:       0        0        0
>   2:       1        0        0
>   3:       1        0        0
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

Reviewed-by: Wei Liu <wei.liu2@citrix.com>

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

Re: [PATCH v2 2/2] xen: fill topology info for all present cpus

[Jan Beulich]

>>> On 31.08.18 at 17:22, <jgross@suse.com> wrote:
> The topology information obtainable via XEN_SYSCTL_cputopoinfo is
> filled rather weird: the size of the array is derived from the highest
> online cpu number, so in case there are trailing offline cpus they
> will not be included.
> 
> On a dual core system with 4 threads booted with smt=0 without this
> patch xl info -n will print:
> 
> cpu_topology           :
> cpu:    core    socket     node
>   0:       0        0        0
>   1:       0        0        0
>   2:       1        0        0
> 
> while with this patch the output is:
> 
> cpu_topology           :
> cpu:    core    socket     node
>   0:       0        0        0
>   1:       0        0        0
>   2:       1        0        0
>   3:       1        0        0
> 
> Signed-off-by: Juergen Gross <jgross@suse.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
for x86. I'd like this to have an ARM ack as well, though, for
there possibly being differences in how cpu_present_map is
handled in their case. In particular I notice that the only access
to the variable is in smp_prepare_cpus().

Jan



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

Re: [PATCH v2 2/2] xen: fill topology info for all present cpus

[Juergen Gross]

On 03/09/18 15:41, Jan Beulich wrote:
>>>> On 31.08.18 at 17:22, <jgross@suse.com> wrote:
>> The topology information obtainable via XEN_SYSCTL_cputopoinfo is
>> filled rather weird: the size of the array is derived from the highest
>> online cpu number, so in case there are trailing offline cpus they
>> will not be included.
>>
>> On a dual core system with 4 threads booted with smt=0 without this
>> patch xl info -n will print:
>>
>> cpu_topology           :
>> cpu:    core    socket     node
>>   0:       0        0        0
>>   1:       0        0        0
>>   2:       1        0        0
>>
>> while with this patch the output is:
>>
>> cpu_topology           :
>> cpu:    core    socket     node
>>   0:       0        0        0
>>   1:       0        0        0
>>   2:       1        0        0
>>   3:       1        0        0
>>
>> Signed-off-by: Juergen Gross <jgross@suse.com>
> 
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> for x86. I'd like this to have an ARM ack as well, though, for
> there possibly being differences in how cpu_present_map is
> handled in their case. In particular I notice that the only access
> to the variable is in smp_prepare_cpus().

How relevant is that information for arm at all?

cpu_to_core(i) et al always return 0. So topology information is not
really helpful.


Juergen

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

Re: [PATCH v2 2/2] xen: fill topology info for all present cpus

[Jan Beulich]

>>> On 04.09.18 at 09:38, <jgross@suse.com> wrote:
> On 03/09/18 15:41, Jan Beulich wrote:
>>>>> On 31.08.18 at 17:22, <jgross@suse.com> wrote:
>>> The topology information obtainable via XEN_SYSCTL_cputopoinfo is
>>> filled rather weird: the size of the array is derived from the highest
>>> online cpu number, so in case there are trailing offline cpus they
>>> will not be included.
>>>
>>> On a dual core system with 4 threads booted with smt=0 without this
>>> patch xl info -n will print:
>>>
>>> cpu_topology           :
>>> cpu:    core    socket     node
>>>   0:       0        0        0
>>>   1:       0        0        0
>>>   2:       1        0        0
>>>
>>> while with this patch the output is:
>>>
>>> cpu_topology           :
>>> cpu:    core    socket     node
>>>   0:       0        0        0
>>>   1:       0        0        0
>>>   2:       1        0        0
>>>   3:       1        0        0
>>>
>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>> 
>> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>> for x86. I'd like this to have an ARM ack as well, though, for
>> there possibly being differences in how cpu_present_map is
>> handled in their case. In particular I notice that the only access
>> to the variable is in smp_prepare_cpus().
> 
> How relevant is that information for arm at all?
> 
> cpu_to_core(i) et al always return 0. So topology information is not
> really helpful.

Stefano, Julien?

Jan



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

Re: [PATCH v2 2/2] xen: fill topology info for all present cpus

[Julien Grall]

Hi,

On 09/04/2018 09:49 AM, Jan Beulich wrote:
>>>> On 04.09.18 at 09:38, <jgross@suse.com> wrote:
>> On 03/09/18 15:41, Jan Beulich wrote:
>>>>>> On 31.08.18 at 17:22, <jgross@suse.com> wrote:
>>>> The topology information obtainable via XEN_SYSCTL_cputopoinfo is
>>>> filled rather weird: the size of the array is derived from the highest
>>>> online cpu number, so in case there are trailing offline cpus they
>>>> will not be included.
>>>>
>>>> On a dual core system with 4 threads booted with smt=0 without this
>>>> patch xl info -n will print:
>>>>
>>>> cpu_topology           :
>>>> cpu:    core    socket     node
>>>>    0:       0        0        0
>>>>    1:       0        0        0
>>>>    2:       1        0        0
>>>>
>>>> while with this patch the output is:
>>>>
>>>> cpu_topology           :
>>>> cpu:    core    socket     node
>>>>    0:       0        0        0
>>>>    1:       0        0        0
>>>>    2:       1        0        0
>>>>    3:       1        0        0
>>>>
>>>> Signed-off-by: Juergen Gross <jgross@suse.com>
>>>
>>> Reviewed-by: Jan Beulich <jbeulich@suse.com>
>>> for x86. I'd like this to have an ARM ack as well, though, for
>>> there possibly being differences in how cpu_present_map is
>>> handled in their case. In particular I notice that the only access
>>> to the variable is in smp_prepare_cpus().
>>
>> How relevant is that information for arm at all?
>>
>> cpu_to_core(i) et al always return 0. So topology information is not
>> really helpful.

Well, this is common code and at some point we will read the CPU 
topology to improve scheduling. This has not been done so far because 
there are other important part to focus first.

> 
> Stefano, Julien?

I think the patch makes sense for Arm too:

Acked-by: Julien Grall <julien.grall@arm.com>

At the moment, cpu_present_map is a copy of cpu_online_map. I would 
expect this to change when hotplug will be supported.

Cheers,

-- 
Julien Grall

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