On 03/04/2019 09:53, Jan Beulich wrote:
>>>> On 02.04.19 at 21:57, <andrew.coop...@citrix.com> wrote:
>> --- a/xen/arch/x86/sysctl.c
>> +++ b/xen/arch/x86/sysctl.c
>> @@ -137,27 +137,35 @@ long arch_do_sysctl(
>> case XEN_SYSCTL_cpu_hotplug:
>> {
>> unsigned int cpu = sysctl->u.cpu_hotplug.cpu;
>> + bool plug;
>> + long (*fn)(void *) = NULL;
>> + void *hcpu = NULL;
> May I ask that you consistently initialize (or not) all three new
> variables you introduce?
I noticed this while posting. Not would allow the compiler to notice
when fn wasn't selected, but hvcpu is going to end up with an
initialiser by the next patch.
>
>> switch ( sysctl->u.cpu_hotplug.op )
>> {
>> case XEN_SYSCTL_CPU_HOTPLUG_ONLINE:
>> - ret = xsm_resource_plug_core(XSM_HOOK);
>> - if ( ret )
>> - break;
>> - ret = continue_hypercall_on_cpu(
>> - 0, cpu_up_helper, (void *)(unsigned long)cpu);
>> + plug = true;
>> + fn = cpu_up_helper;
>> + hcpu = (void *)(unsigned long)cpu;
> I wonder whether it wouldn't be better to have this cast just
> once, ...
>
>> break;
>> +
>> case XEN_SYSCTL_CPU_HOTPLUG_OFFLINE:
>> - ret = xsm_resource_unplug_core(XSM_HOOK);
>> - if ( ret )
>> - break;
>> - ret = continue_hypercall_on_cpu(
>> - 0, cpu_down_helper, (void *)(unsigned long)cpu);
>> + plug = false;
>> + fn = cpu_down_helper;
>> + hcpu = (void *)(unsigned long)cpu;
>> break;
>> +
>> default:
>> - ret = -EINVAL;
>> + ret = -EOPNOTSUPP;
>> break;
>> }
>> +
>> + if ( !ret )
>> + ret = plug ? xsm_resource_plug_core(XSM_HOOK)
>> + : xsm_resource_unplug_core(XSM_HOOK);
>> +
>> + if ( !ret )
>> + ret = continue_hypercall_on_cpu(0, fn, hcpu);
> ... here. This would the even eliminate the need for "hcpu"
> as a local variable.
The SMT versions don't take a CPU parameter, so the selection of the
parameter has to be done before this point.
> Preferably with both remarks addressed
> Reviewed-by: Jan Beulich <jbeul...@suse.com>
I'll drop the initialisers here, but the initialisation of hcpu is going
to have to stay where it is.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
