Re: [PATCH v2] core-parking: shrink core_parking_cpunum[]

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

You possibly want to adjust the subject, instead of shrink I would use
"dynamically allocate" or similar.

On Mon, Mar 09, 2026 at 04:35:34PM +0100, Jan Beulich wrote:
> This NR_CPUS-dimensioned array is likely unused on most installations.
> Therefore it is especially wasteful for it to consume more space than
> really needed. Allocate it dynamically ahead of registering the hooks.
> 
> Further the array having all fields set to -1 is actually useless. Nothing
> relies on it, and core_parking_remove() doesn't restore the sentinel for
> vacated slots. Drop the initializers altogether, rather than replacing
> them.
> 
> Also take the opportunity and update an adjacent variable's type, where
> a fixed-width type was pretty clearly inappropriate to use.
> 
> Finally drop the redundant initializer from core_parking_init().
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> I assume there is a reason this is acting (mostly) as a LIFO. Else a
> simple cpumask_t would suffice.
> 
> num_possible_cpus() would be the more correct thing to use as array
> dimension, yet we don't maintain cpu_possible_map on x86.
> ---
> v2: Use dynamic allocation.
> 
> --- a/xen/common/core_parking.c
> +++ b/xen/common/core_parking.c
> @@ -20,6 +20,7 @@
>  #include <xen/cpumask.h>
>  #include <xen/init.h>
>  #include <xen/param.h>
> +#include <xen/xvmalloc.h>
>  
>  #include <asm/smp.h>
>  
> @@ -27,8 +28,8 @@
>  #define CORE_PARKING_DECREMENT 2
>  
>  static DEFINE_SPINLOCK(accounting_lock);
> -static uint32_t cur_idle_nums;
> -static unsigned int core_parking_cpunum[NR_CPUS] = {[0 ... NR_CPUS-1] = -1};
> +static unsigned int cur_idle_nums;
> +static unsigned int *__ro_after_init core_parking_cpunum;

Don't you need some kind of check in core_parking_remove() to prevent a
NULL pointer dereference if core_parking_cpunum hasn't been allocated?

Callers of XEN_SYSCTL_cpu_hotplug can set fn = smt_up_down_helper, and
that would call core_parking_remove().  core_parking_helper() already
contains a check that prevents accessing core_parking_cpunum if no
policy has been registered.

Thanks, Roger.