Re: [PATCH] arm64: crash_core: Export MODULES, VMALLOC, and VMEMMAP ranges
On Thu, Feb 17, 2022 at 10:02:03AM +, Huang Shijie wrote: > > Thanks for digging up all of the kernel memory map changes and taking the > > time to explain the macros. However, all I'm really after is something in > > the commit message of the patch which explains what is broken without this > This kernel patch does not break anything. I mean this kernel patch is not a bugfix, it is just make the ARM64 ecosystem (including the Crash) a little better.. The Crash can just parse out the MODULES/VMALLOC/VMALLOC ranges from the vmcore file, no need to calculate them in a very _hard_ way. Thanks Huang Shijie ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH] arm64: crash_core: Export MODULES, VMALLOC, and VMEMMAP ranges
Hi Will, CC Kazu and Lianbo. On Wed, Feb 16, 2022 at 12:40:27PM +, Will Deacon wrote: > On Wed, Feb 16, 2022 at 09:28:49AM +, Huang Shijie wrote: > > Hi Will, > > On Tue, Feb 15, 2022 at 04:44:23PM +, Will Deacon wrote: > > > On Wed, Feb 09, 2022 at 09:26:42AM +, Huang Shijie wrote: > > > > The following interrelated ranges are needed by the kdump crash tool: > > > > MODULES_VADDR ~ MODULES_END, > > > > VMALLOC_START ~ VMALLOC_END, > > > > VMEMMAP_START ~ VMEMMAP_END > > > > > > > > Since these values change from time to time, it is preferable to export > > > > them via vmcoreinfo than to change the crash's code frequently. > > > > > > Please can you explain _why_ they are needed? > > > > The current Crash code is still based at kernel v4.9. > >The virtual memory layout looks like this: > >++ > >|KASAN | MODULE | VMALLOC | | VMEMMAP | > >++ > > > > The Crash uses MODULES range to set the VMALLOC ranges. > > If the ranges are wrong, Crash will _NOT_ works well for some latest kernel > > ,such as v5.11 later. (Please correct me if I am wrong). > > It seems the VMEMMAP range is less important. > > [...] > > > 5.) In the kernel v5.16, after the patch > > "b89ddf4cca43 arm64/bpf: Remove 128MB limit for BPF JIT programs" > > the virtual memory layout looks like this: > > > >++ > >| MODULE | VMALLOC | | VMEMMAP| > >++ > > > > The macros are: > > #define MODULES_VADDR (_PAGE_END(VA_BITS_MIN)) > > #define MODULES_END (MODULES_VADDR + MODULES_VSIZE) > > > > #define VMALLOC_START (MODULES_END) > > #define VMALLOC_END (VMEMMAP_START - SZ_256M) > > > > #define VMEMMAP_START (-(UL(1) << (VA_BITS - VMEMMAP_SHIFT))) > > #define VMEMMAP_END (VMEMMAP_START + VMEMMAP_SIZE) > > > > > > BTW:I am currently coding a patch for the Crash to update all the ranges to > > the latest kernel version v5.17-rc4. > > Thanks for digging up all of the kernel memory map changes and taking the > time to explain the macros. However, all I'm really after is something in > the commit message of the patch which explains what is broken without this This kernel patch does not break anything. It just makes the Crash easy to maintain. > patch. What does crash use this information for, and what doesn't work at > the moment? I know two cases now: 1.) The Crash uses MODULES/VMALLOC/VMEMMAP ranges in arm64_IS_VMALLOC_ADDR(): https://github.com/crash-utility/crash/blob/master/arm64.c#L4104 If arm64_IS_VMALLOC_ADDR() does not work correctly, the internal code may get wrong results. 2.) The "help -m" gets wrong output about MODULES/VMALLOC/VMEMMAP ranges. The guy who use the Use the Crash to debug a vmcore, will get the wrong information of the kernel panic. Thanks Huang Shijie ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v20 3/5] arm64: kdump: reimplement crashkernel=X
On 2022/2/16 18:20, Baoquan He wrote:
> On 02/16/22 at 10:58am, Leizhen (ThunderTown) wrote:
>>
>>
>> On 2022/2/14 15:53, Leizhen (ThunderTown) wrote:
>>>
>>>
>>> On 2022/2/14 11:52, Baoquan He wrote:
On 01/24/22 at 04:47pm, Zhen Lei wrote:
..
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index 6c653a2c7cff052..a5d43feac0d7d96 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -71,6 +71,30 @@ phys_addr_t arm64_dma_phys_limit __ro_after_init;
> #define CRASH_ADDR_LOW_MAX arm64_dma_phys_limit
> #define CRASH_ADDR_HIGH_MAX MEMBLOCK_ALLOC_ACCESSIBLE
>
> +static int __init reserve_crashkernel_low(unsigned long long low_size)
> +{
> + unsigned long long low_base;
> +
> + /* passed with crashkernel=0,low ? */
> + if (!low_size)
> + return 0;
> +
> + low_base = memblock_phys_alloc_range(low_size, CRASH_ALIGN, 0,
> CRASH_ADDR_LOW_MAX);
> + if (!low_base) {
> + pr_err("cannot allocate crashkernel low memory
> (size:0x%llx).\n", low_size);
> + return -ENOMEM;
> + }
> +
> + pr_info("crashkernel low memory reserved: 0x%llx - 0x%llx (%lld MB)\n",
> + low_base, low_base + low_size, low_size >> 20);
> +
> + crashk_low_res.start = low_base;
> + crashk_low_res.end = low_base + low_size - 1;
> + insert_resource(&iomem_resource, &crashk_low_res);
> +
> + return 0;
> +}
> +
> /*
> * reserve_crashkernel() - reserves memory for crash kernel
My another concern is the crashkernel=,low handling. In this patch, the
code related to low memory is obscure. Wondering if we should make them
explicit with a little redundant but very clear code flows. Saying this
because the code must be very clear to you and reviewers, it may be
harder for later code reader or anyone interested to understand.
1) crashkernel=X,high
2) crashkernel=X,high crashkernel=Y,low
3) crashkernel=X,high crashkernel=0,low
4) crashkernel=X,high crashkernel='messy code',low
5) crashkernel=X //fall back to high memory, low memory is required then.
It could be me thinking about it too much. I made changes to your patch
with a tuning, not sure if it's OK to you. Otherwise, this patchset
>>>
>>> I think it's good.
>>>
works very well for all above test cases, it's ripe to be merged for
wider testing.
>>>
>>> I will test it tomorrow. I've prepared a little more use cases than yours.
>>
>> After the following modifications, I have tested it and it works well. Passed
>> all the test cases I prepared.
>
> That's great.
>
> You might need to add 'crashkernel=xM, crashkernel=0,low',
> 'crashkernel=xM, crashkernel='messy code',low' to your test cases.
Oh, right, I will add them.
>
>>
>>>
>>> 1) crashkernel=4G //high=4G,
>>> low=256M
>>> 2) crashkernel=4G crashkernel=512M,high crashkernel=512M,low
>>> //high=4G, low=256M, high and low are ignored
>>> 3) crashkernel=4G crashkernel=512M,high
>>> //high=4G, low=256M, high is ignored
>>> 4) crashkernel=4G crashkernel=512M,low
>>> //high=4G, low=256M, low is ignored
>>> 5) crashkernel=4G@0xe000
>>> //high=0G, low=0M, cannot allocate, failed
>>> 6) crashkernel=512M //high=0G,
>>> low=512M
>>> 7) crashkernel=128M //high=0G,
>>> low=128M
>>> 8) crashkernel=512M@0xde00 //512M@3552M
>>> //high=0G, low=512M
>>> 9) crashkernel=4G,high
>>> //high=4G, low=256M
>>> a) crashkernel=4G,high crashkernel=512M,low //high=4G,
>>> low=512M
>>> b) crashkernel=512M,high crashkernel=128M,low
>>> //high=512M, low=128M
>>> c) crashkernel=512M,low
>>> //high=0G, low=0M, invalid
>>>
>>>
diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
index a5d43feac0d7..671862c56d7d 100644
--- a/arch/arm64/mm/init.c
+++ b/arch/arm64/mm/init.c
@@ -94,7 +94,8 @@ static int __init reserve_crashkernel_low(unsigned long
long low_size)
return 0;
}
-
+/*Words explaining why it's 256M*/
+#define DEFAULT_CRASH_KERNEL_LOW_SIZE SZ_256M
>>
>> It's an empirical value.
>>
>> 94fb9334182284e8e7e4bcb9125c25dc33af19d4 x86/crash: Allocate enough low
>> memory when crashkernel=high
>>
>> When the crash kernel is loaded above 4GiB in memory, the
>> first kernel allocates only 72MiB of low-memory for the DMA
>> requirements of the second kernel. On systems with many
>> devices this is not enough and causes device driver
>> initialization errors and failed crash dumps. Testing b
Re: [PATCH] arm64: crash_core: Export MODULES, VMALLOC, and VMEMMAP ranges
On Wed, Feb 16, 2022 at 09:28:49AM +, Huang Shijie wrote: > Hi Will, > On Tue, Feb 15, 2022 at 04:44:23PM +, Will Deacon wrote: > > On Wed, Feb 09, 2022 at 09:26:42AM +, Huang Shijie wrote: > > > The following interrelated ranges are needed by the kdump crash tool: > > > MODULES_VADDR ~ MODULES_END, > > > VMALLOC_START ~ VMALLOC_END, > > > VMEMMAP_START ~ VMEMMAP_END > > > > > > Since these values change from time to time, it is preferable to export > > > them via vmcoreinfo than to change the crash's code frequently. > > > > Please can you explain _why_ they are needed? > > The current Crash code is still based at kernel v4.9. >The virtual memory layout looks like this: >++ >|KASAN | MODULE | VMALLOC | | VMEMMAP | >++ > > The Crash uses MODULES range to set the VMALLOC ranges. > If the ranges are wrong, Crash will _NOT_ works well for some latest kernel > ,such as v5.11 later. (Please correct me if I am wrong). > It seems the VMEMMAP range is less important. [...] > 5.) In the kernel v5.16, after the patch > "b89ddf4cca43 arm64/bpf: Remove 128MB limit for BPF JIT programs" > the virtual memory layout looks like this: > >++ >| MODULE | VMALLOC | | VMEMMAP| >++ > > The macros are: > #define MODULES_VADDR (_PAGE_END(VA_BITS_MIN)) > #define MODULES_END (MODULES_VADDR + MODULES_VSIZE) > > #define VMALLOC_START (MODULES_END) > #define VMALLOC_END (VMEMMAP_START - SZ_256M) > > #define VMEMMAP_START (-(UL(1) << (VA_BITS - VMEMMAP_SHIFT))) > #define VMEMMAP_END (VMEMMAP_START + VMEMMAP_SIZE) > > > BTW:I am currently coding a patch for the Crash to update all the ranges to > the latest kernel version v5.17-rc4. Thanks for digging up all of the kernel memory map changes and taking the time to explain the macros. However, all I'm really after is something in the commit message of the patch which explains what is broken without this patch. What does crash use this information for, and what doesn't work at the moment? Cheers, Will ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v20 3/5] arm64: kdump: reimplement crashkernel=X
On 02/16/22 at 10:58am, Leizhen (ThunderTown) wrote:
>
>
> On 2022/2/14 15:53, Leizhen (ThunderTown) wrote:
> >
> >
> > On 2022/2/14 11:52, Baoquan He wrote:
> >> On 01/24/22 at 04:47pm, Zhen Lei wrote:
> >> ..
> >>> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> >>> index 6c653a2c7cff052..a5d43feac0d7d96 100644
> >>> --- a/arch/arm64/mm/init.c
> >>> +++ b/arch/arm64/mm/init.c
> >>> @@ -71,6 +71,30 @@ phys_addr_t arm64_dma_phys_limit __ro_after_init;
> >>> #define CRASH_ADDR_LOW_MAX arm64_dma_phys_limit
> >>> #define CRASH_ADDR_HIGH_MAX MEMBLOCK_ALLOC_ACCESSIBLE
> >>>
> >>> +static int __init reserve_crashkernel_low(unsigned long long low_size)
> >>> +{
> >>> + unsigned long long low_base;
> >>> +
> >>> + /* passed with crashkernel=0,low ? */
> >>> + if (!low_size)
> >>> + return 0;
> >>> +
> >>> + low_base = memblock_phys_alloc_range(low_size, CRASH_ALIGN, 0,
> >>> CRASH_ADDR_LOW_MAX);
> >>> + if (!low_base) {
> >>> + pr_err("cannot allocate crashkernel low memory
> >>> (size:0x%llx).\n", low_size);
> >>> + return -ENOMEM;
> >>> + }
> >>> +
> >>> + pr_info("crashkernel low memory reserved: 0x%llx - 0x%llx (%lld MB)\n",
> >>> + low_base, low_base + low_size, low_size >> 20);
> >>> +
> >>> + crashk_low_res.start = low_base;
> >>> + crashk_low_res.end = low_base + low_size - 1;
> >>> + insert_resource(&iomem_resource, &crashk_low_res);
> >>> +
> >>> + return 0;
> >>> +}
> >>> +
> >>> /*
> >>> * reserve_crashkernel() - reserves memory for crash kernel
> >>
> >> My another concern is the crashkernel=,low handling. In this patch, the
> >> code related to low memory is obscure. Wondering if we should make them
> >> explicit with a little redundant but very clear code flows. Saying this
> >> because the code must be very clear to you and reviewers, it may be
> >> harder for later code reader or anyone interested to understand.
> >>
> >> 1) crashkernel=X,high
> >> 2) crashkernel=X,high crashkernel=Y,low
> >> 3) crashkernel=X,high crashkernel=0,low
> >> 4) crashkernel=X,high crashkernel='messy code',low
> >> 5) crashkernel=X //fall back to high memory, low memory is required then.
> >>
> >> It could be me thinking about it too much. I made changes to your patch
> >> with a tuning, not sure if it's OK to you. Otherwise, this patchset
> >
> > I think it's good.
> >
> >> works very well for all above test cases, it's ripe to be merged for
> >> wider testing.
> >
> > I will test it tomorrow. I've prepared a little more use cases than yours.
>
> After the following modifications, I have tested it and it works well. Passed
> all the test cases I prepared.
That's great.
You might need to add 'crashkernel=xM, crashkernel=0,low',
'crashkernel=xM, crashkernel='messy code',low' to your test cases.
>
> >
> > 1) crashkernel=4G //high=4G,
> > low=256M
> > 2) crashkernel=4G crashkernel=512M,high crashkernel=512M,low
> > //high=4G, low=256M, high and low are ignored
> > 3) crashkernel=4G crashkernel=512M,high
> > //high=4G, low=256M, high is ignored
> > 4) crashkernel=4G crashkernel=512M,low
> > //high=4G, low=256M, low is ignored
> > 5) crashkernel=4G@0xe000
> > //high=0G, low=0M, cannot allocate, failed
> > 6) crashkernel=512M //high=0G,
> > low=512M
> > 7) crashkernel=128M //high=0G,
> > low=128M
> > 8) crashkernel=512M@0xde00 //512M@3552M
> > //high=0G, low=512M
> > 9) crashkernel=4G,high
> > //high=4G, low=256M
> > a) crashkernel=4G,high crashkernel=512M,low //high=4G,
> > low=512M
> > b) crashkernel=512M,high crashkernel=128M,low
> > //high=512M, low=128M
> > c) crashkernel=512M,low
> > //high=0G, low=0M, invalid
> >
> >
> >>
> >> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> >> index a5d43feac0d7..671862c56d7d 100644
> >> --- a/arch/arm64/mm/init.c
> >> +++ b/arch/arm64/mm/init.c
> >> @@ -94,7 +94,8 @@ static int __init reserve_crashkernel_low(unsigned long
> >> long low_size)
> >>
> >>return 0;
> >> }
> >> -
> >> +/*Words explaining why it's 256M*/
> >> +#define DEFAULT_CRASH_KERNEL_LOW_SIZE SZ_256M
>
> It's an empirical value.
>
> 94fb9334182284e8e7e4bcb9125c25dc33af19d4 x86/crash: Allocate enough low
> memory when crashkernel=high
>
> When the crash kernel is loaded above 4GiB in memory, the
> first kernel allocates only 72MiB of low-memory for the DMA
> requirements of the second kernel. On systems with many
> devices this is not enough and causes device driver
> initialization errors and failed crash dumps. Testing by
> SUSE and Redhat has shown that 256MiB is a good default
> value for now and
Re: [PATCH V5] panic: Move panic_print before kmsg dumpers
On 02/14/22 at 11:17am, Guilherme G. Piccoli wrote: > On 14/02/2022 06:00, Baoquan He wrote: > > [...] > >> This patch changes that in 2 ways: > >~~~ steps? > > > > Otherwise, the whole looks straightforward, clear, thanks. > > > > Acked-by: Baoquan He > > > > Thanks a lot Baoquan, just sent a V6 including your suggestion and your > Acked-by - I'm feeling we are close now...heheh Glad to hear that, and thanks for the effort and patience. ___ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec
