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

[Michal Orzel <michal.orzel@xxxxxxx>]

On 03/10/2024 11:35, oleksii.kurochko@xxxxxxxxx wrote:
> 
> 
> On Wed, 2024-10-02 at 15:41 +0200, Michal Orzel wrote:
>>
>>
>> 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.
> There is no change for now but what I mean by this message if one day,
> for example, enable_nonboot_cpus() will be called and park_offline_cpus
> will be enabled/implemented for Arm then CPU_RESUME_FAILED will be
> handled differently in comparison with original Arm code. In original
> Arm code it will do just break but for common code it will do
> free_percpu_area().
> And the similar for CPU_REMOVE if park_offline_cpus is enabled and
> cpu_notifier_call_chain(..., CPU_REMOVE,...) will be called then the
> behaviour of common code will be different from Arm code.
> 
> Do you think it would be better just drop this part of the commit
> message?
> Or would it be better to add:
>   ...(what I wrote before), however, there is no change in behavior for
> Arm at this time.
Yes, this one lgtm.

~Michal