On 09/23/2020 11:34 AM, Gavin Shan wrote: > Hi Anshuman, > > On 9/21/20 10:05 PM, Anshuman Khandual wrote: >> This moves memory notifier registration earlier in the boot process from >> device_initcall() to early_initcall() which will help in guarding against >> potential early boot memory offline requests. Even though there should not >> be any actual offlinig requests till memory block devices are initialized >> with memory_dev_init() but then generic init sequence might just change in >> future. Hence an early registration for the memory event notifier would be >> helpful. While here, just skip the registration if CONFIG_MEMORY_HOTREMOVE >> is not enabled and also call out when memory notifier registration fails. >> >> Cc: Catalin Marinas <[email protected]> >> Cc: Will Deacon <[email protected]> >> Cc: Mark Rutland <[email protected]> >> Cc: Marc Zyngier <[email protected]> >> Cc: Steve Capper <[email protected]> >> Cc: Mark Brown <[email protected]> >> Cc: [email protected] >> Cc: [email protected] >> Signed-off-by: Anshuman Khandual <[email protected]> >> --- >> arch/arm64/mm/mmu.c | 14 ++++++++++++-- >> 1 file changed, 12 insertions(+), 2 deletions(-) >> > > With the following nit-picky comments resolved: > > Reviewed-by: Gavin Shan <[email protected]> > >> diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c >> index 75df62fea1b6..df3b7415b128 100644 >> --- a/arch/arm64/mm/mmu.c >> +++ b/arch/arm64/mm/mmu.c >> @@ -1499,7 +1499,17 @@ static struct notifier_block >> prevent_bootmem_remove_nb = { >> static int __init prevent_bootmem_remove_init(void) >> { >> - return register_memory_notifier(&prevent_bootmem_remove_nb); >> + int ret = 0; >> + >> + if (!IS_ENABLED(CONFIG_MEMORY_HOTREMOVE)) >> + return ret; >> + >> + ret = register_memory_notifier(&prevent_bootmem_remove_nb); >> + if (!ret) >> + return ret; >> + >> + pr_err("Notifier registration failed - boot memory can be removed\n"); >> + return ret; >> } > > It might be cleaner if the duplicated return statements can be > avoided. Besides, it's always nice to print the errno even though Thought about it, just that the error message was too long. > zero is always returned from register_memory_notifier(). So I guess > you probably need something like below: > > ret = register_memory_notifier(&prevent_bootmem_remove_nb); > if (ret) > pr_err("%s: Error %d registering notifier\n", __func__, ret) > > return ret; Sure, will do. > > > register_memory_notifier # 0 is returned on > !CONFIG_MEMORY_HOTPLUG_SPARSE > blocking_notifier_chain_register > notifier_chain_register # 0 is always returned > >> -device_initcall(prevent_bootmem_remove_init); >> +early_initcall(prevent_bootmem_remove_init); >> #endif >> > > Cheers, > Gavin > >

