Messages in this thread | | | From | Anshuman Khandual <> | Subject | Re: [PATCH V3 1/3] arm64/mm/hotplug: Register boot memory hot remove notifier earlier | Date | Thu, 24 Sep 2020 08:53:47 +0530 |
| |
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 <catalin.marinas@arm.com> >> Cc: Will Deacon <will@kernel.com> >> Cc: Mark Rutland <mark.rutland@arm.com> >> Cc: Marc Zyngier <maz@kernel.org> >> Cc: Steve Capper <steve.capper@arm.com> >> Cc: Mark Brown <broonie@kernel.org> >> Cc: linux-arm-kernel@lists.infradead.org >> Cc: linux-kernel@vger.kernel.org >> Signed-off-by: Anshuman Khandual <anshuman.khandual@arm.com> >> --- >> arch/arm64/mm/mmu.c | 14 ++++++++++++-- >> 1 file changed, 12 insertions(+), 2 deletions(-) >> > > With the following nit-picky comments resolved: > > Reviewed-by: Gavin Shan <gshan@redhat.com> > >> 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 > >
| |