Re: [RFC v3 1/3] fadump: Refactor and prepare fadump_cma_init for late init
From: Madhavan Srinivasan <maddy@linux.ibm.com>
Date: 2024-10-14 10:24:41
Also in:
linux-mm, lkml
On 10/11/24 8:30 PM, Ritesh Harjani (IBM) wrote:
We anyway don't use any return values from fadump_cma_init(). Since fadump_reserve_mem() from where fadump_cma_init() gets called today, already has the required checks. This patch makes this function return type as void. Let's also handle extra cases like return if fadump_supported is false or dump_active, so that in later patches we can call fadump_cma_init() separately from setup_arch().
Usually patches to this file are posted with title format of powerpc/fadump:<>
quoted hunk ↗ jump to hunk
Acked-by: Hari Bathini <hbathini@linux.ibm.com> Signed-off-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com> --- v2 -> v3: Separated the series into 2 as discussed in v2. [v2]: https://lore.kernel.org/linuxppc-dev/cover.1728585512.git.ritesh.list@gmail.com/ (local) arch/powerpc/kernel/fadump.c | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-)diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c index a612e7513a4f..162327d66982 100644 --- a/arch/powerpc/kernel/fadump.c +++ b/arch/powerpc/kernel/fadump.c@@ -78,27 +78,23 @@ static struct cma *fadump_cma; * But for some reason even if it fails we still have the memory reservation * with us and we can still continue doing fadump. */ -static int __init fadump_cma_init(void) +static void __init fadump_cma_init(void) { unsigned long long base, size; int rc; - if (!fw_dump.fadump_enabled) - return 0; - + if (!fw_dump.fadump_supported || !fw_dump.fadump_enabled || + fw_dump.dump_active) + return;
Is these checks even needed here? fadump_reserve_mem() checked for all these already, also dont see any other caller for fadump_cma_init().
/* * Do not use CMA if user has provided fadump=nocma kernel parameter. - * Return 1 to continue with fadump old behaviour. */ - if (fw_dump.nocma) - return 1; + if (fw_dump.nocma || !fw_dump.boot_memory_size) + return; base = fw_dump.reserve_dump_area_start; size = fw_dump.boot_memory_size; - if (!size) - return 0;
So this is the only place where we return 0, which in turn will make the "ret" in fadump_reserve_mem() as zero forcing to call reserve_crashkernel() in early_init_devtree(). we are removing it, becos we know "size" here will never be zero?
quoted hunk ↗ jump to hunk
- rc = cma_init_reserved_mem(base, size, 0, "fadump_cma", &fadump_cma); if (rc) { pr_err("Failed to init cma area for firmware-assisted dump,%d\n", rc);@@ -108,7 +104,7 @@ static int __init fadump_cma_init(void) * blocked from production system usage. Hence return 1, * so that we can continue with fadump. */ - return 1; + return; } /*@@ -125,10 +121,9 @@ static int __init fadump_cma_init(void) cma_get_size(fadump_cma), (unsigned long)cma_get_base(fadump_cma) >> 20, fw_dump.reserve_dump_area_size); - return 1; } #else -static int __init fadump_cma_init(void) { return 1; } +static void __init fadump_cma_init(void) { } #endif /* CONFIG_CMA */ /*@@ -638,7 +633,7 @@ int __init fadump_reserve_mem(void) pr_info("Reserved %lldMB of memory at %#016llx (System RAM: %lldMB)\n", (size >> 20), base, (memblock_phys_mem_size() >> 20)); - ret = fadump_cma_init(); + fadump_cma_init(); } return ret; --2.46.0