Re: [PATCH v4] xen: move per-cpu area management into common code

[Michal Orzel <michal.orzel@xxxxxxx>]

On 30/09/2024 18:39, Oleksii Kurochko wrote:
> 
> 
> Centralize per-cpu area management to reduce code duplication and
> enhance maintainability across architectures.
> 
> The per-cpu area management code, which is largely common among
> architectures, is moved to a shared implementation in
> xen/common/percpu.c. This change includes:
>  * Remove percpu.c from the X86 and Arm architectures.
>  * For x86, define INVALID_PERCPU_AREAS and PARK_OFFLINE_CPUS_VAR.
>  * Drop the declaration of __per_cpu_offset[] from stubs.c in
>    PPC and RISC-V to facilitate the build of the common per-cpu code.
> 
> No functional changes for x86.
> 
> For Arm add support of CPU_RESUME_FAILED, CPU_REMOVE and freeing of
> percpu in the case when system_state != SYS_STATE_suspend.
Behaviorwise there is no change for Arm given that none of these actions can be 
executed.
That said, by looking at the code I realized that we never call CPU_REMOVE so 
it is effectively
a dead code.

As for the change itself:
Reviewed-by: Michal Orzel <michal.orzel@xxxxxxx>

with one question below ...

[...]

> +static int cf_check cpu_percpu_callback(
> +    struct notifier_block *nfb, unsigned long action, void *hcpu)
> +{
> +    unsigned int cpu = (unsigned long)hcpu;
> +    int rc = 0;
> +
> +    switch ( action )
> +    {
> +    case CPU_UP_PREPARE:
> +        rc = init_percpu_area(cpu);
> +        break;
> +    case CPU_UP_CANCELED:
> +    case CPU_DEAD:
> +    case CPU_RESUME_FAILED:
> +        if ( !park_offline_cpus && system_state != SYS_STATE_suspend )
> +            free_percpu_area(cpu);
> +        break;
> +    case CPU_REMOVE:
> +        if ( park_offline_cpus )
> +            free_percpu_area(cpu);
> +        break;
Aren't we missing default statement here as per MISRA C 16.4?
I think we only allow to drop it for enums.

~Michal