On 03/02/23 at 11:32am, Leizhen (ThunderTown) wrote:
>
>
> On 2023/2/23 20:45, Baoquan He wrote:
......
> There are two minor review comments, see below. Otherwise:
Makes sense, all accepted. Will update and repost.
Thanks for careful reviewing.
>
> Reviewed-by: Zhen Lei <thunder.leizhen@xxxxxxxxxx>
>
> >
> > Signed-off-by: Baoquan He <bhe@xxxxxxxxxx>
> > ---
> > v2->v3:
> > - Rephrase patch log to clarify the current crashkernel high
> > reservation could cross the high and low memory boundary, but not
> > 4G boundary only, because RPi4 of arm64 has high and low memory
> > boudary as 1G. The v3 patch log could mislead people that the RPi4
> > also use 4G as high,low memory boundary.
> > v1->v2:
> > - Fold patch 2 of v1 into patch 1 for better reviewing.
> > - Update patch log to add more details.
> > arch/arm64/mm/init.c | 43 +++++++++++++++++++++++++++++++++----------
> > 1 file changed, 33 insertions(+), 10 deletions(-)
> >
> > diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> > index 58a0bb2c17f1..b8cb780df0cb 100644
> > --- a/arch/arm64/mm/init.c
> > +++ b/arch/arm64/mm/init.c
> > @@ -127,12 +127,13 @@ static int __init reserve_crashkernel_low(unsigned long long low_size)
> > */
> > static void __init reserve_crashkernel(void)
> > {
> > - unsigned long long crash_base, crash_size;
> > - unsigned long long crash_low_size = 0;
> > + unsigned long long crash_base, crash_size, search_base;
> > unsigned long long crash_max = CRASH_ADDR_LOW_MAX;
> > + unsigned long long crash_low_size = 0;
> > char *cmdline = boot_command_line;
> > - int ret;
> > bool fixed_base = false;
> > + bool high = false;
> > + int ret;
> >
> > if (!IS_ENABLED(CONFIG_KEXEC_CORE))
> > return;
> > @@ -155,7 +156,9 @@ static void __init reserve_crashkernel(void)
> > else if (ret)
> > return;
> >
> > + search_base = CRASH_ADDR_LOW_MAX;
> > crash_max = CRASH_ADDR_HIGH_MAX;
> > + high = true;
> > } else if (ret || !crash_size) {
> > /* The specified value is invalid */
> > return;
> > @@ -166,31 +169,51 @@ static void __init reserve_crashkernel(void)
> > /* User specifies base address explicitly. */
> > if (crash_base) {
> > fixed_base = true;
> > + search_base = crash_base;
> > crash_max = crash_base + crash_size;
> > }
> >
> > retry:
> > crash_base = memblock_phys_alloc_range(crash_size, CRASH_ALIGN,
> > - crash_base, crash_max);
> > + search_base, crash_max);
> > if (!crash_base) {
> > /*
> > - * If the first attempt was for low memory, fall back to
> > - * high memory, the minimum required low memory will be
> > - * reserved later.
> > + * For crashkernel=size[KMG]@offset[KMG], print out failure
> > + * message if can't reserve the specified region.
> > */
> > - if (!fixed_base && (crash_max == CRASH_ADDR_LOW_MAX)) {
> > + if (fixed_base) {
> > + pr_info("crashkernel reservation failed - memory is in use.\n");
>
> How about changing pr_info to pr_warn?
>
> > + return;
> > + }
> > +
> > + /*
> > + * For crashkernel=size[KMG], if the first attempt was for
> > + * low memory, fall back to high memory, the minimum required
> > + * low memory will be reserved later.
> > + */
> > + if (!high && crash_max == CRASH_ADDR_LOW_MAX) {
> > crash_max = CRASH_ADDR_HIGH_MAX;
> > + search_base = CRASH_ADDR_LOW_MAX;
> > crash_low_size = DEFAULT_CRASH_KERNEL_LOW_SIZE;
> > goto retry;
> > }
> >
> > + /*
> > + * For crashkernel=size[KMG],high, if the first attempt was
> > + * for high memory, fall back to low memory.
> > + */
> > + if (high && crash_max == CRASH_ADDR_HIGH_MAX) {
>
> Adding unlikely to indicate that it is rare would be better.
>
> if (unlikely(high && crash_max == CRASH_ADDR_HIGH_MAX))
>
> > + crash_max = CRASH_ADDR_LOW_MAX;
> > + search_base = 0;
> > + goto retry;
> > + }
> > pr_warn("cannot allocate crashkernel (size:0x%llx)\n",
> > crash_size);
> > return;
> > }
> >
> > - if ((crash_base > CRASH_ADDR_LOW_MAX - crash_low_size) &&
> > - crash_low_size && reserve_crashkernel_low(crash_low_size)) {
> > + if ((crash_base >= CRASH_ADDR_LOW_MAX) && crash_low_size &&
> > + reserve_crashkernel_low(crash_low_size)) {
> > memblock_phys_free(crash_base, crash_size);
> > return;
> > }
> >
>
> --
> Regards,
> Zhen Lei
>
_______________________________________________
kexec mailing list
kexec@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/kexec
