| Message ID | 20230608092246.343761-1-sourabhjain@linux.ibm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | powerpc/fadump: reset dump area size variable if memblock reserve fails | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/github-powerpc_selftests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_ppctests | success | Successfully ran 8 jobs. |
| snowpatch_ozlabs/github-powerpc_sparse | success | Successfully ran 4 jobs. |
| snowpatch_ozlabs/github-powerpc_clang | success | Successfully ran 6 jobs. |
| snowpatch_ozlabs/github-powerpc_kernel_qemu | success | Successfully ran 24 jobs. |
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 <sourabhjain@linux.ibm.com> > Acked-by: Mahesh Salgaonkar <mahesh@linux.ibm.com> > --- > 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; > }
Sourabh Jain <sourabhjain@linux.ibm.com> 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 <sourabhjain@linux.ibm.com> >> Acked-by: Mahesh Salgaonkar <mahesh@linux.ibm.com> >> --- >> 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(&fw_dump) < 0)) goto error_out; cheers
On 03/07/23 16:59, Michael Ellerman wrote: > Sourabh Jain <sourabhjain@linux.ibm.com> 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 <sourabhjain@linux.ibm.com> >>> Acked-by: Mahesh Salgaonkar <mahesh@linux.ibm.com> >>> --- >>> 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(&fw_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
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; }
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 <sourabhjain@linux.ibm.com> Acked-by: Mahesh Salgaonkar <mahesh@linux.ibm.com> --- arch/powerpc/kernel/fadump.c | 1 + 1 file changed, 1 insertion(+)