[PATCH v9 3/7] powerpc/code-patching: Use WARN_ON and fix check in poking_init

[Christophe Leroy]

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 <bgray at linux.ibm.com>
> Reviewed-by: Russell Currey <ruscur at 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 ?

>   }
>