Re: [PATCH 3/3] x86/smt: Support for enabling/disabling SMT at runtime

["Jan Beulich" <JBeulich@suse.com>]

>>> On 03.04.19 at 12:17, <andrew.cooper3@citrix.com> wrote:
> On 03/04/2019 10:33, Jan Beulich wrote:
>>>>> On 02.04.19 at 21:57, <andrew.cooper3@citrix.com> wrote:
>>> +        if ( x86_cpu_to_apicid[cpu] & sibling_mask )
>>> +            ret = cpu_up_helper(_p(cpu));
>> Shouldn't this be restricted to CPUs a sibling of which is already
>> online? And widened at the same time, to also online thread 0
>> if one of the other threads is already online?
> 
> Unfortunately, that turns into a rats nest very very quickly, which is
> why I gave up and simplified the semantics to strictly "this shall
> {of,off}line the nonzero siblings threads".
> 
> This is a convenience for people wanting to do a one-time
> reconfiguration of the system, and indeed, how has multiple end user
> requests behind its coming into existence.  Users who are already
> hotplugging aren't going to be interested in this functionality.

I'd like to come back to this: I assume by "hotplugging" you
really mean hot {on,off}lining. Thinking about the actual physical
hotplug case, the flow of events is (if I'm not mistaken) for the
Dom0 kernel to issue XENPF_cpu_hotadd (in response to respective
ACPI events), which then would still need to be followed by
xen-hptool invocations to actually bring up the new cores/threads.

While we invoke remove_siblinginfo() from __cpu_disable(), I don't
see why we shouldn't be able to clone cpu_sibling_mask into an
instance not getting altered when a CPU gets parked. This could
then be used here, albeit the context of me thinking about this
again is my intended attempt to make core parking work with
opt_smt set to false, seeing that Jürgen had pointed out this case
as not working.

I further wonder whether these new sysctl-s should be constrained
to the park_offline_cpus == true case. I don't think it was the
intention to bring up/down secondary compute units of AMD Fam15
CPUs, and I also don't think fiddling with AMD Fam17 secondary
threads is really intended/necessary here. I wouldn't be worried
much about the other, opt_mce, dependency: People shouldn't use
"mce=0" on production systems anyway; we should perhaps name
this an unsupported configuration.

Jan


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