| Message ID | 20201031074437.168008-2-chenzhou10@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | support reserving crashkernel above 4G on arm64 kdump | expand |
On 10/31/20 at 03:44pm, Chen Zhou wrote: > Move CRASH_ALIGN to header asm/kexec.h and replace the hard-coded > alignment with macro CRASH_ALIGN in function reserve_crashkernel(). Seems you tell what you have done in this patch, but don't like adding several more words to tell why it's done like that. Please see below inline comments. In other patches, I can also see this similar problem. > > Suggested-by: Dave Young <dyoung@redhat.com> > Signed-off-by: Chen Zhou <chenzhou10@huawei.com> > Tested-by: John Donnelly <John.p.donnelly@oracle.com> > --- > arch/x86/include/asm/kexec.h | 3 +++ > arch/x86/kernel/setup.c | 5 +---- > 2 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/arch/x86/include/asm/kexec.h b/arch/x86/include/asm/kexec.h > index 6802c59e8252..8cf9d3fd31c7 100644 > --- a/arch/x86/include/asm/kexec.h > +++ b/arch/x86/include/asm/kexec.h > @@ -18,6 +18,9 @@ > > # define KEXEC_CONTROL_CODE_MAX_SIZE 2048 > > +/* 2M alignment for crash kernel regions */ > +#define CRASH_ALIGN SZ_16M > + > #ifndef __ASSEMBLY__ > > #include <linux/string.h> > diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c > index 84f581c91db4..bf373422dc8a 100644 > --- a/arch/x86/kernel/setup.c > +++ b/arch/x86/kernel/setup.c > @@ -395,9 +395,6 @@ static void __init memblock_x86_reserve_range_setup_data(void) > > #ifdef CONFIG_KEXEC_CORE > > -/* 16M alignment for crash kernel regions */ > -#define CRASH_ALIGN SZ_16M > - > /* > * Keep the crash kernel below this limit. > * > @@ -515,7 +512,7 @@ static void __init reserve_crashkernel(void) > } else { > unsigned long long start; > > - start = memblock_phys_alloc_range(crash_size, SZ_1M, crash_base, > + start = memblock_phys_alloc_range(crash_size, CRASH_ALIGN, crash_base, > crash_base + crash_size); Here, SZ_1M is replaced with CRASH_ALIGN which is 16M. I remember I ever commented that this had better be told in patch log. > if (start != crash_base) { > pr_info("crashkernel reservation failed - memory is in use.\n"); > -- > 2.20.1 > > > _______________________________________________ > kexec mailing list > kexec@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/kexec >
Hi Baoquan, On 2020/11/11 9:38, Baoquan He wrote: > On 10/31/20 at 03:44pm, Chen Zhou wrote: >> Move CRASH_ALIGN to header asm/kexec.h and replace the hard-coded >> alignment with macro CRASH_ALIGN in function reserve_crashkernel(). > Seems you tell what you have done in this patch, but don't like adding > several more words to tell why it's done like that. Please see below > inline comments. > > In other patches, I can also see this similar problem. Thanks for your review. I will update relevant commit messages. > >> Suggested-by: Dave Young <dyoung@redhat.com> >> Signed-off-by: Chen Zhou <chenzhou10@huawei.com> >> Tested-by: John Donnelly <John.p.donnelly@oracle.com> >> --- >> arch/x86/include/asm/kexec.h | 3 +++ >> arch/x86/kernel/setup.c | 5 +---- >> 2 files changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/arch/x86/include/asm/kexec.h b/arch/x86/include/asm/kexec.h >> index 6802c59e8252..8cf9d3fd31c7 100644 >> --- a/arch/x86/include/asm/kexec.h >> +++ b/arch/x86/include/asm/kexec.h >> @@ -18,6 +18,9 @@ >> >> # define KEXEC_CONTROL_CODE_MAX_SIZE 2048 >> >> +/* 2M alignment for crash kernel regions */ >> +#define CRASH_ALIGN SZ_16M >> + >> #ifndef __ASSEMBLY__ >> >> #include <linux/string.h> >> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c >> index 84f581c91db4..bf373422dc8a 100644 >> --- a/arch/x86/kernel/setup.c >> +++ b/arch/x86/kernel/setup.c >> @@ -395,9 +395,6 @@ static void __init memblock_x86_reserve_range_setup_data(void) >> >> #ifdef CONFIG_KEXEC_CORE >> >> -/* 16M alignment for crash kernel regions */ >> -#define CRASH_ALIGN SZ_16M >> - >> /* >> * Keep the crash kernel below this limit. >> * >> @@ -515,7 +512,7 @@ static void __init reserve_crashkernel(void) >> } else { >> unsigned long long start; >> >> - start = memblock_phys_alloc_range(crash_size, SZ_1M, crash_base, >> + start = memblock_phys_alloc_range(crash_size, CRASH_ALIGN, crash_base, >> crash_base + crash_size); > Here, SZ_1M is replaced with CRASH_ALIGN which is 16M. I remember I ever > commented that this had better be told in patch log. got it. Thanks, Chen Zhou > >> if (start != crash_base) { >> pr_info("crashkernel reservation failed - memory is in use.\n"); >> -- >> 2.20.1 >> >> >> _______________________________________________ >> kexec mailing list >> kexec@lists.infradead.org >> http://lists.infradead.org/mailman/listinfo/kexec >> > . >
Hi, On Sat, Oct 31, 2020 at 03:44:30PM +0800, Chen Zhou wrote: > Move CRASH_ALIGN to header asm/kexec.h and replace the hard-coded > alignment with macro CRASH_ALIGN in function reserve_crashkernel(). > > Suggested-by: Dave Young <dyoung@redhat.com> > Signed-off-by: Chen Zhou <chenzhou10@huawei.com> > Tested-by: John Donnelly <John.p.donnelly@oracle.com> > --- > arch/x86/include/asm/kexec.h | 3 +++ > arch/x86/kernel/setup.c | 5 +---- > 2 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/arch/x86/include/asm/kexec.h b/arch/x86/include/asm/kexec.h > index 6802c59e8252..8cf9d3fd31c7 100644 > --- a/arch/x86/include/asm/kexec.h > +++ b/arch/x86/include/asm/kexec.h > @@ -18,6 +18,9 @@ > > # define KEXEC_CONTROL_CODE_MAX_SIZE 2048 > > +/* 2M alignment for crash kernel regions */ > +#define CRASH_ALIGN SZ_16M Please update the comment to match the code. > + > #ifndef __ASSEMBLY__ > > #include <linux/string.h> > diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c > index 84f581c91db4..bf373422dc8a 100644 > --- a/arch/x86/kernel/setup.c > +++ b/arch/x86/kernel/setup.c > @@ -395,9 +395,6 @@ static void __init memblock_x86_reserve_range_setup_data(void) > > #ifdef CONFIG_KEXEC_CORE > > -/* 16M alignment for crash kernel regions */ > -#define CRASH_ALIGN SZ_16M > - > /* > * Keep the crash kernel below this limit. > * > @@ -515,7 +512,7 @@ static void __init reserve_crashkernel(void) > } else { > unsigned long long start; > > - start = memblock_phys_alloc_range(crash_size, SZ_1M, crash_base, > + start = memblock_phys_alloc_range(crash_size, CRASH_ALIGN, crash_base, > crash_base + crash_size); > if (start != crash_base) { > pr_info("crashkernel reservation failed - memory is in use.\n"); > -- > 2.20.1 >
On 2020/11/12 15:58, Mike Rapoport wrote: > Hi, > > On Sat, Oct 31, 2020 at 03:44:30PM +0800, Chen Zhou wrote: >> Move CRASH_ALIGN to header asm/kexec.h and replace the hard-coded >> alignment with macro CRASH_ALIGN in function reserve_crashkernel(). >> >> Suggested-by: Dave Young <dyoung@redhat.com> >> Signed-off-by: Chen Zhou <chenzhou10@huawei.com> >> Tested-by: John Donnelly <John.p.donnelly@oracle.com> >> --- >> arch/x86/include/asm/kexec.h | 3 +++ >> arch/x86/kernel/setup.c | 5 +---- >> 2 files changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/arch/x86/include/asm/kexec.h b/arch/x86/include/asm/kexec.h >> index 6802c59e8252..8cf9d3fd31c7 100644 >> --- a/arch/x86/include/asm/kexec.h >> +++ b/arch/x86/include/asm/kexec.h >> @@ -18,6 +18,9 @@ >> >> # define KEXEC_CONTROL_CODE_MAX_SIZE 2048 >> >> +/* 2M alignment for crash kernel regions */ >> +#define CRASH_ALIGN SZ_16M > Please update the comment to match the code. Ok, thanks for pointing this mistake. Thanks, Chen Zhou > >> + >> #ifndef __ASSEMBLY__ >> >> #include <linux/string.h> >> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c >> index 84f581c91db4..bf373422dc8a 100644 >> --- a/arch/x86/kernel/setup.c >> +++ b/arch/x86/kernel/setup.c >> @@ -395,9 +395,6 @@ static void __init memblock_x86_reserve_range_setup_data(void) >> >> #ifdef CONFIG_KEXEC_CORE >> >> -/* 16M alignment for crash kernel regions */ >> -#define CRASH_ALIGN SZ_16M >> - >> /* >> * Keep the crash kernel below this limit. >> * >> @@ -515,7 +512,7 @@ static void __init reserve_crashkernel(void) >> } else { >> unsigned long long start; >> >> - start = memblock_phys_alloc_range(crash_size, SZ_1M, crash_base, >> + start = memblock_phys_alloc_range(crash_size, CRASH_ALIGN, crash_base, >> crash_base + crash_size); >> if (start != crash_base) { >> pr_info("crashkernel reservation failed - memory is in use.\n"); >> -- >> 2.20.1 >>
diff --git a/arch/x86/include/asm/kexec.h b/arch/x86/include/asm/kexec.h index 6802c59e8252..8cf9d3fd31c7 100644 --- a/arch/x86/include/asm/kexec.h +++ b/arch/x86/include/asm/kexec.h @@ -18,6 +18,9 @@ # define KEXEC_CONTROL_CODE_MAX_SIZE 2048 +/* 2M alignment for crash kernel regions */ +#define CRASH_ALIGN SZ_16M + #ifndef __ASSEMBLY__ #include <linux/string.h> diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c index 84f581c91db4..bf373422dc8a 100644 --- a/arch/x86/kernel/setup.c +++ b/arch/x86/kernel/setup.c @@ -395,9 +395,6 @@ static void __init memblock_x86_reserve_range_setup_data(void) #ifdef CONFIG_KEXEC_CORE -/* 16M alignment for crash kernel regions */ -#define CRASH_ALIGN SZ_16M - /* * Keep the crash kernel below this limit. * @@ -515,7 +512,7 @@ static void __init reserve_crashkernel(void) } else { unsigned long long start; - start = memblock_phys_alloc_range(crash_size, SZ_1M, crash_base, + start = memblock_phys_alloc_range(crash_size, CRASH_ALIGN, crash_base, crash_base + crash_size); if (start != crash_base) { pr_info("crashkernel reservation failed - memory is in use.\n");