Re: [PATCH 2/3] x86/sysctl: Clean up XEN_SYSCTL_cpu_hotplug

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

>>> On 02.04.19 at 21:57, <andrew.cooper3@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?

>          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.

Preferably with both remarks addressed
Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan



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