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

[Benjamin Gray <bgray@linux.ibm.com>]

On Wed, 2022-11-02 at 09:38 +0000, Christophe Leroy wrote:
> Le 25/10/2022 à 06:44, Benjamin Gray a écrit :
> > 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 ?

Yeah that probably works better. If it manages to reach a patch attempt
after that it should fail anyway with strict RWX.