Re: [PATCH] powerpc/fadump: reset dump area size variable if memblock reserve fails
On 03/07/23 16:59, Michael Ellerman wrote: Sourabh Jain writes: Hello Michael, Do you have any feedback or comments regarding this patch? Thanks, Sourabh On 08/06/23 14:52, Sourabh Jain wrote: If the memory reservation process (memblock_reserve) fails to reserve the memory, the reserve dump variable retains the dump area size. Consequently, the size of the dump area calculated for reservation is displayed in /sys/kernel/fadump/mem_reserved. To resolve this issue, the reserve dump area size variable is set to 0 if the memblock_reserve fails to reserve memory. Fixes: 8255da95e545 ("powerpc/fadump: release all the memory above boot memory size") Signed-off-by: Sourabh Jain Acked-by: Mahesh Salgaonkar --- arch/powerpc/kernel/fadump.c | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c index ea0a073abd96..a8f2c3b2fa1e 100644 --- a/arch/powerpc/kernel/fadump.c +++ b/arch/powerpc/kernel/fadump.c @@ -641,6 +641,7 @@ int __init fadump_reserve_mem(void) goto error_out; if (memblock_reserve(base, size)) { + fw_dump.reserve_dump_area_size = 0; pr_err("Failed to reserve memory!\n"); goto error_out; } Shouldn't reserve_dump_area_size be set to zero at error_out, which already clears fadump_enabled? return ret; error_out: fw_dump.fadump_enabled = 0; return 0; } Otherwise the code immediately above will suffer from the same issue won't it? if (fw_dump.ops->fadump_setup_metadata && (fw_dump.ops->fadump_setup_metadata(_dump) < 0)) goto error_out; Agree, resetting fw_dump.reserve_dump_area_size below error_out label is better. Thanks for the review. I will send v2. Sourabh Jain
Re: [PATCH] powerpc/fadump: reset dump area size variable if memblock reserve fails
Sourabh Jain writes: > Hello Michael, > > Do you have any feedback or comments regarding this patch? > > Thanks, > Sourabh > > On 08/06/23 14:52, Sourabh Jain wrote: >> If the memory reservation process (memblock_reserve) fails to reserve >> the memory, the reserve dump variable retains the dump area size. >> Consequently, the size of the dump area calculated for reservation >> is displayed in /sys/kernel/fadump/mem_reserved. >> >> To resolve this issue, the reserve dump area size variable is set to 0 >> if the memblock_reserve fails to reserve memory. >> >> Fixes: 8255da95e545 ("powerpc/fadump: release all the memory above boot >> memory size") >> Signed-off-by: Sourabh Jain >> Acked-by: Mahesh Salgaonkar >> --- >> arch/powerpc/kernel/fadump.c | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c >> index ea0a073abd96..a8f2c3b2fa1e 100644 >> --- a/arch/powerpc/kernel/fadump.c >> +++ b/arch/powerpc/kernel/fadump.c >> @@ -641,6 +641,7 @@ int __init fadump_reserve_mem(void) >> goto error_out; >> >> if (memblock_reserve(base, size)) { >> +fw_dump.reserve_dump_area_size = 0; >> pr_err("Failed to reserve memory!\n"); >> goto error_out; >> } Shouldn't reserve_dump_area_size be set to zero at error_out, which already clears fadump_enabled? return ret; error_out: fw_dump.fadump_enabled = 0; return 0; } Otherwise the code immediately above will suffer from the same issue won't it? if (fw_dump.ops->fadump_setup_metadata && (fw_dump.ops->fadump_setup_metadata(_dump) < 0)) goto error_out; cheers
Re: [PATCH] powerpc/fadump: reset dump area size variable if memblock reserve fails
Hello Michael, Do you have any feedback or comments regarding this patch? Thanks, Sourabh On 08/06/23 14:52, Sourabh Jain wrote: If the memory reservation process (memblock_reserve) fails to reserve the memory, the reserve dump variable retains the dump area size. Consequently, the size of the dump area calculated for reservation is displayed in /sys/kernel/fadump/mem_reserved. To resolve this issue, the reserve dump area size variable is set to 0 if the memblock_reserve fails to reserve memory. Fixes: 8255da95e545 ("powerpc/fadump: release all the memory above boot memory size") Signed-off-by: Sourabh Jain Acked-by: Mahesh Salgaonkar --- arch/powerpc/kernel/fadump.c | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c index ea0a073abd96..a8f2c3b2fa1e 100644 --- a/arch/powerpc/kernel/fadump.c +++ b/arch/powerpc/kernel/fadump.c @@ -641,6 +641,7 @@ int __init fadump_reserve_mem(void) goto error_out; if (memblock_reserve(base, size)) { + fw_dump.reserve_dump_area_size = 0; pr_err("Failed to reserve memory!\n"); goto error_out; }
[PATCH] powerpc/fadump: reset dump area size variable if memblock reserve fails
If the memory reservation process (memblock_reserve) fails to reserve the memory, the reserve dump variable retains the dump area size. Consequently, the size of the dump area calculated for reservation is displayed in /sys/kernel/fadump/mem_reserved. To resolve this issue, the reserve dump area size variable is set to 0 if the memblock_reserve fails to reserve memory. Fixes: 8255da95e545 ("powerpc/fadump: release all the memory above boot memory size") Signed-off-by: Sourabh Jain Acked-by: Mahesh Salgaonkar --- arch/powerpc/kernel/fadump.c | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/powerpc/kernel/fadump.c b/arch/powerpc/kernel/fadump.c index ea0a073abd96..a8f2c3b2fa1e 100644 --- a/arch/powerpc/kernel/fadump.c +++ b/arch/powerpc/kernel/fadump.c @@ -641,6 +641,7 @@ int __init fadump_reserve_mem(void) goto error_out; if (memblock_reserve(base, size)) { + fw_dump.reserve_dump_area_size = 0; pr_err("Failed to reserve memory!\n"); goto error_out; } -- 2.40.1