Le 25/10/2022 à 06:44, Benjamin Gray a écrit :
> BUG_ON() when failing to initialise the code patching window is
> excessive, as most critical patching happens during boot before strict
> RWX control is enabled. Failure to patch after boot is not inherently
> fatal, so aborting the kernel is better determined by the caller.
>
> The return value of cpuhp_setup_state() is also >= 0 on success,
> so check for < 0.
>
> Signed-off-by: Benjamin Gray <bg...@linux.ibm.com>
> Reviewed-by: Russell Currey <rus...@russell.cc>
> ---
> v9: * Reword commit message to explain why init failure is not fatal
> ---
> arch/powerpc/lib/code-patching.c | 13 +++++--------
> 1 file changed, 5 insertions(+), 8 deletions(-)
>
> diff --git a/arch/powerpc/lib/code-patching.c
> b/arch/powerpc/lib/code-patching.c
> index 54e145247643..3b3b09d5d2e1 100644
> --- a/arch/powerpc/lib/code-patching.c
> +++ b/arch/powerpc/lib/code-patching.c
> @@ -82,16 +82,13 @@ static int text_area_cpu_down(unsigned int cpu)
>
> static __ro_after_init DEFINE_STATIC_KEY_FALSE(poking_init_done);
>
> -/*
> - * Although BUG_ON() is rude, in this case it should only happen if ENOMEM,
> and
> - * we judge it as being preferable to a kernel that will crash later when
> - * someone tries to use patch_instruction().
> - */
> void __init poking_init(void)
> {
> - BUG_ON(!cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
> - "powerpc/text_poke:online", text_area_cpu_up,
> - text_area_cpu_down));
> + WARN_ON(cpuhp_setup_state(CPUHP_AP_ONLINE_DYN,
> + "powerpc/text_poke:online",
> + text_area_cpu_up,
> + text_area_cpu_down) < 0);
> +
> static_branch_enable(&poking_init_done);
Wouldn't it be better to not enable the poking_init_done branch if the
allocation failed ?
> }
>
