| Message ID | 20211210065533.2023-3-thunder.leizhen@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | support reserving crashkernel above 4G on arm64 kdump | expand |
On 12/10/21 at 02:55pm, Zhen Lei wrote: > From: Chen Zhou <chenzhou10@huawei.com> > > The lower bounds of crash kernel reservation and crash kernel low > reservation are different, use the consistent value CRASH_ALIGN. > > Suggested-by: Dave Young <dyoung@redhat.com> > Signed-off-by: Chen Zhou <chenzhou10@huawei.com> > Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> You may need add Co-developed-by to clarify who is author, and who is co-author. Please check section "When to use Acked-by:, Cc:, and Co-developed-by:" of Documentation/process/submitting-patches.rst. Otherwise, Acked-by: Baoquan He <bhe@redhat.com> > Tested-by: John Donnelly <John.p.donnelly@oracle.com> > Tested-by: Dave Kleikamp <dave.kleikamp@oracle.com> > --- > arch/x86/kernel/setup.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c > index 5cc60996eac56d6..6424ee4f23da2cf 100644 > --- a/arch/x86/kernel/setup.c > +++ b/arch/x86/kernel/setup.c > @@ -441,7 +441,8 @@ static int __init reserve_crashkernel_low(void) > return 0; > } > > - low_base = memblock_phys_alloc_range(low_size, CRASH_ALIGN, 0, CRASH_ADDR_LOW_MAX); > + low_base = memblock_phys_alloc_range(low_size, CRASH_ALIGN, CRASH_ALIGN, > + CRASH_ADDR_LOW_MAX); > if (!low_base) { > pr_err("Cannot reserve %ldMB crashkernel low memory, please try smaller size.\n", > (unsigned long)(low_size >> 20)); > -- > 2.25.1 >
On 12/10/21 12:55 AM, Zhen Lei wrote: > From: Chen Zhou <chenzhou10@huawei.com> > > The lower bounds of crash kernel reservation and crash kernel low > reservation are different, use the consistent value CRASH_ALIGN. > > Suggested-by: Dave Young <dyoung@redhat.com> > Signed-off-by: Chen Zhou <chenzhou10@huawei.com> > Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> > Tested-by: John Donnelly <John.p.donnelly@oracle.com> > Tested-by: Dave Kleikamp <dave.kleikamp@oracle.com> Acked-by: John Donnelly <john.p.donnelly@oracle.com> > --- > arch/x86/kernel/setup.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c > index 5cc60996eac56d6..6424ee4f23da2cf 100644 > --- a/arch/x86/kernel/setup.c > +++ b/arch/x86/kernel/setup.c > @@ -441,7 +441,8 @@ static int __init reserve_crashkernel_low(void) > return 0; > } > > - low_base = memblock_phys_alloc_range(low_size, CRASH_ALIGN, 0, CRASH_ADDR_LOW_MAX); > + low_base = memblock_phys_alloc_range(low_size, CRASH_ALIGN, CRASH_ALIGN, > + CRASH_ADDR_LOW_MAX); > if (!low_base) { > pr_err("Cannot reserve %ldMB crashkernel low memory, please try smaller size.\n", > (unsigned long)(low_size >> 20));
On 2021/12/13 21:37, Baoquan He wrote: > On 12/10/21 at 02:55pm, Zhen Lei wrote: >> From: Chen Zhou <chenzhou10@huawei.com> >> >> The lower bounds of crash kernel reservation and crash kernel low >> reservation are different, use the consistent value CRASH_ALIGN. >> >> Suggested-by: Dave Young <dyoung@redhat.com> >> Signed-off-by: Chen Zhou <chenzhou10@huawei.com> >> Signed-off-by: Zhen Lei <thunder.leizhen@huawei.com> > > You may need add Co-developed-by to clarify who is author, and who is > co-author. Please check section "When to use Acked-by:, Cc:, and Co-developed-by:" > of Documentation/process/submitting-patches.rst. Otherwise, Okay, thanks for the heads-up. I will modify it. > > Acked-by: Baoquan He <bhe@redhat.com> > >> Tested-by: John Donnelly <John.p.donnelly@oracle.com> >> Tested-by: Dave Kleikamp <dave.kleikamp@oracle.com> >> --- >> arch/x86/kernel/setup.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c >> index 5cc60996eac56d6..6424ee4f23da2cf 100644 >> --- a/arch/x86/kernel/setup.c >> +++ b/arch/x86/kernel/setup.c >> @@ -441,7 +441,8 @@ static int __init reserve_crashkernel_low(void) >> return 0; >> } >> >> - low_base = memblock_phys_alloc_range(low_size, CRASH_ALIGN, 0, CRASH_ADDR_LOW_MAX); >> + low_base = memblock_phys_alloc_range(low_size, CRASH_ALIGN, CRASH_ALIGN, >> + CRASH_ADDR_LOW_MAX); >> if (!low_base) { >> pr_err("Cannot reserve %ldMB crashkernel low memory, please try smaller size.\n", >> (unsigned long)(low_size >> 20)); >> -- >> 2.25.1 >> > > . >
On Fri, Dec 10, 2021 at 02:55:25PM +0800, Zhen Lei wrote: > From: Chen Zhou <chenzhou10@huawei.com> > > The lower bounds of crash kernel reservation and crash kernel low > reservation are different, use the consistent value CRASH_ALIGN. A big WHY is missing here to explain why the lower bound of the allocation range needs to be 16M and why was 0 wrong? > diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c > index 5cc60996eac56d6..6424ee4f23da2cf 100644 > --- a/arch/x86/kernel/setup.c > +++ b/arch/x86/kernel/setup.c > @@ -441,7 +441,8 @@ static int __init reserve_crashkernel_low(void) > return 0; > } > > - low_base = memblock_phys_alloc_range(low_size, CRASH_ALIGN, 0, CRASH_ADDR_LOW_MAX); > + low_base = memblock_phys_alloc_range(low_size, CRASH_ALIGN, CRASH_ALIGN, > + CRASH_ADDR_LOW_MAX); You don't have to break this line. Thx.
On Tue, Dec 14, 2021 at 08:07:58PM +0100, Borislav Petkov wrote: > On Fri, Dec 10, 2021 at 02:55:25PM +0800, Zhen Lei wrote: > > From: Chen Zhou <chenzhou10@huawei.com> > > > > The lower bounds of crash kernel reservation and crash kernel low > > reservation are different, use the consistent value CRASH_ALIGN. > > A big WHY is missing here to explain why the lower bound of the > allocation range needs to be 16M and why was 0 wrong? I asked the same here: https://lore.kernel.org/r/20210224143547.GB28965@arm.com IIRC Baoquan said that there is a 1MB reserved for x86 anyway in the lower part, so that's equivalent in practice to starting from CRASH_ALIGN. Anyway, I agree the commit log should describe this.
On 2021/12/15 3:24, Catalin Marinas wrote: > On Tue, Dec 14, 2021 at 08:07:58PM +0100, Borislav Petkov wrote: >> On Fri, Dec 10, 2021 at 02:55:25PM +0800, Zhen Lei wrote: >>> From: Chen Zhou <chenzhou10@huawei.com> >>> >>> The lower bounds of crash kernel reservation and crash kernel low >>> reservation are different, use the consistent value CRASH_ALIGN. >> >> A big WHY is missing here to explain why the lower bound of the >> allocation range needs to be 16M and why was 0 wrong? > > I asked the same here: > > https://lore.kernel.org/r/20210224143547.GB28965@arm.com > > IIRC Baoquan said that there is a 1MB reserved for x86 anyway in the > lower part, so that's equivalent in practice to starting from > CRASH_ALIGN. > > Anyway, I agree the commit log should describe this. OK, I will add the description. >
On 12/14/21 at 07:24pm, Catalin Marinas wrote: > On Tue, Dec 14, 2021 at 08:07:58PM +0100, Borislav Petkov wrote: > > On Fri, Dec 10, 2021 at 02:55:25PM +0800, Zhen Lei wrote: > > > From: Chen Zhou <chenzhou10@huawei.com> > > > > > > The lower bounds of crash kernel reservation and crash kernel low > > > reservation are different, use the consistent value CRASH_ALIGN. > > > > A big WHY is missing here to explain why the lower bound of the > > allocation range needs to be 16M and why was 0 wrong? > > I asked the same here: > > https://lore.kernel.org/r/20210224143547.GB28965@arm.com > > IIRC Baoquan said that there is a 1MB reserved for x86 anyway in the > lower part, so that's equivalent in practice to starting from > CRASH_ALIGN. Yeah, even for i386, there's area reserved by BIOS inside low 1M. Considering the existing alignment CRASH_ALIGN which is 16M, we definitely have no chance to get memory starting from 0. So starting from 16M can skip the useless memblock searching, and make the crashkernel low reservation consisten with crashkernel reservation on allocation code. > > Anyway, I agree the commit log should describe this. Yes, totally.
On Wed, Dec 15, 2021 at 11:42:19AM +0800, Baoquan He wrote: > On 12/14/21 at 07:24pm, Catalin Marinas wrote: > > On Tue, Dec 14, 2021 at 08:07:58PM +0100, Borislav Petkov wrote: > > > On Fri, Dec 10, 2021 at 02:55:25PM +0800, Zhen Lei wrote: > > > > From: Chen Zhou <chenzhou10@huawei.com> > > > > > > > > The lower bounds of crash kernel reservation and crash kernel low > > > > reservation are different, use the consistent value CRASH_ALIGN. > > > > > > A big WHY is missing here to explain why the lower bound of the > > > allocation range needs to be 16M and why was 0 wrong? > > > > I asked the same here: > > > > https://lore.kernel.org/r/20210224143547.GB28965@arm.com > > > > IIRC Baoquan said that there is a 1MB reserved for x86 anyway in the > > lower part, so that's equivalent in practice to starting from > > CRASH_ALIGN. > > Yeah, even for i386, there's area reserved by BIOS inside low 1M. > Considering the existing alignment CRASH_ALIGN which is 16M, we > definitely have no chance to get memory starting from 0. So starting > from 16M can skip the useless memblock searching, and make the > crashkernel low reservation consisten with crashkernel reservation on > allocation code. That's the x86 assumption. Is it valid for other architectures once the code has been made generic in patch 6? It should be ok for arm64, RAM tends to start from higher up but other architectures may start using this common code. If you want to keep the same semantics as before, just leave it as 0. It's not that the additional lower bound makes the search slower.
On 12/15/21 at 11:01am, Catalin Marinas wrote: > On Wed, Dec 15, 2021 at 11:42:19AM +0800, Baoquan He wrote: > > On 12/14/21 at 07:24pm, Catalin Marinas wrote: > > > On Tue, Dec 14, 2021 at 08:07:58PM +0100, Borislav Petkov wrote: > > > > On Fri, Dec 10, 2021 at 02:55:25PM +0800, Zhen Lei wrote: > > > > > From: Chen Zhou <chenzhou10@huawei.com> > > > > > > > > > > The lower bounds of crash kernel reservation and crash kernel low > > > > > reservation are different, use the consistent value CRASH_ALIGN. > > > > > > > > A big WHY is missing here to explain why the lower bound of the > > > > allocation range needs to be 16M and why was 0 wrong? > > > > > > I asked the same here: > > > > > > https://lore.kernel.org/r/20210224143547.GB28965@arm.com > > > > > > IIRC Baoquan said that there is a 1MB reserved for x86 anyway in the > > > lower part, so that's equivalent in practice to starting from > > > CRASH_ALIGN. > > > > Yeah, even for i386, there's area reserved by BIOS inside low 1M. > > Considering the existing alignment CRASH_ALIGN which is 16M, we > > definitely have no chance to get memory starting from 0. So starting > > from 16M can skip the useless memblock searching, and make the > > crashkernel low reservation consisten with crashkernel reservation on > > allocation code. > > That's the x86 assumption. Is it valid for other architectures once the > code has been made generic in patch 6? It should be ok for arm64, RAM > tends to start from higher up but other architectures may start using > this common code. Good point. I didn't think of this from generic code side, then let's keep it as 0. > > If you want to keep the same semantics as before, just leave it as 0. > It's not that the additional lower bound makes the search slower. Agree.
On 2021/12/15 19:16, Baoquan He wrote: > On 12/15/21 at 11:01am, Catalin Marinas wrote: >> On Wed, Dec 15, 2021 at 11:42:19AM +0800, Baoquan He wrote: >>> On 12/14/21 at 07:24pm, Catalin Marinas wrote: >>>> On Tue, Dec 14, 2021 at 08:07:58PM +0100, Borislav Petkov wrote: >>>>> On Fri, Dec 10, 2021 at 02:55:25PM +0800, Zhen Lei wrote: >>>>>> From: Chen Zhou <chenzhou10@huawei.com> >>>>>> >>>>>> The lower bounds of crash kernel reservation and crash kernel low >>>>>> reservation are different, use the consistent value CRASH_ALIGN. >>>>> >>>>> A big WHY is missing here to explain why the lower bound of the >>>>> allocation range needs to be 16M and why was 0 wrong? >>>> >>>> I asked the same here: >>>> >>>> https://lore.kernel.org/r/20210224143547.GB28965@arm.com >>>> >>>> IIRC Baoquan said that there is a 1MB reserved for x86 anyway in the >>>> lower part, so that's equivalent in practice to starting from >>>> CRASH_ALIGN. >>> >>> Yeah, even for i386, there's area reserved by BIOS inside low 1M. >>> Considering the existing alignment CRASH_ALIGN which is 16M, we >>> definitely have no chance to get memory starting from 0. So starting >>> from 16M can skip the useless memblock searching, and make the >>> crashkernel low reservation consisten with crashkernel reservation on >>> allocation code. >> >> That's the x86 assumption. Is it valid for other architectures once the >> code has been made generic in patch 6? It should be ok for arm64, RAM >> tends to start from higher up but other architectures may start using >> this common code. > > Good point. I didn't think of this from generic code side, then let's > keep it as 0. > >> >> If you want to keep the same semantics as before, just leave it as 0. >> It's not that the additional lower bound makes the search slower. > > Agree. OK, I will drop this patch. > > . >
diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c index 5cc60996eac56d6..6424ee4f23da2cf 100644 --- a/arch/x86/kernel/setup.c +++ b/arch/x86/kernel/setup.c @@ -441,7 +441,8 @@ static int __init reserve_crashkernel_low(void) return 0; } - low_base = memblock_phys_alloc_range(low_size, CRASH_ALIGN, 0, CRASH_ADDR_LOW_MAX); + low_base = memblock_phys_alloc_range(low_size, CRASH_ALIGN, CRASH_ALIGN, + CRASH_ADDR_LOW_MAX); if (!low_base) { pr_err("Cannot reserve %ldMB crashkernel low memory, please try smaller size.\n", (unsigned long)(low_size >> 20));