Message ID | 20221109045112.187069-3-bgray@linux.ibm.com (mailing list archive) |
---|---|
State | Accepted |
Commit | 071c95c1acbd96e76bab8b25b5cad0d71a011f37 |
Headers | show |
Series | powerpc/code-patching: Use temporary mm for Radix MMU | expand |
Le 09/11/2022 à 05:51, Benjamin Gray a écrit : > BUG_ON() when failing to initialise the code patching window is > unnecessary, and use of BUG_ON is discouraged. We don't set > poking_init_done in this case, so failure to init the boot CPU will > result in a strict RWX error when a following patch_instruction uses > raw_patch_instruction. If it only fails for later CPUs, they won't be > onlined in the first place. > > The return value of cpuhp_setup_state() is also >= 0 on success, > so check for < 0. > > Signed-off-by: Benjamin Gray <bgray@linux.ibm.com> Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu> > --- > v10: * Don't mention caller recovery > * Don't set poking_init_done on failure > * Add setup return code comment from later patch > * Reviewed-by not applied because of above changes > v9: * Reword commit message to explain why init failure is not fatal > --- > arch/powerpc/lib/code-patching.c | 17 +++++++++-------- > 1 file changed, 9 insertions(+), 8 deletions(-) > > diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c > index ad0cf3108dd0..3055eef7dcdc 100644 > --- a/arch/powerpc/lib/code-patching.c > +++ b/arch/powerpc/lib/code-patching.c > @@ -81,16 +81,17 @@ 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)); > + int ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, > + "powerpc/text_poke:online", > + text_area_cpu_up, > + text_area_cpu_down); > + > + /* cpuhp_setup_state returns >= 0 on success */ > + if (WARN_ON(ret < 0)) > + return; > + > static_branch_enable(&poking_init_done); > } >
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c index ad0cf3108dd0..3055eef7dcdc 100644 --- a/arch/powerpc/lib/code-patching.c +++ b/arch/powerpc/lib/code-patching.c @@ -81,16 +81,17 @@ 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)); + int ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, + "powerpc/text_poke:online", + text_area_cpu_up, + text_area_cpu_down); + + /* cpuhp_setup_state returns >= 0 on success */ + if (WARN_ON(ret < 0)) + return; + static_branch_enable(&poking_init_done); }
BUG_ON() when failing to initialise the code patching window is unnecessary, and use of BUG_ON is discouraged. We don't set poking_init_done in this case, so failure to init the boot CPU will result in a strict RWX error when a following patch_instruction uses raw_patch_instruction. If it only fails for later CPUs, they won't be onlined in the first place. The return value of cpuhp_setup_state() is also >= 0 on success, so check for < 0. Signed-off-by: Benjamin Gray <bgray@linux.ibm.com> --- v10: * Don't mention caller recovery * Don't set poking_init_done on failure * Add setup return code comment from later patch * Reviewed-by not applied because of above changes v9: * Reword commit message to explain why init failure is not fatal --- arch/powerpc/lib/code-patching.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-)