On 04/17/14 at 02:32pm, Dave Young wrote: > On 04/17/14 at 02:17pm, WANG Chao wrote: > > On 04/17/14 at 01:35pm, Dave Young wrote: > > > On 04/17/14 at 01:29pm, Dave Young wrote: > > > > On 04/14/14 at 10:55pm, WANG Chao wrote: > > > > > command line size is restricted by kernel, sometimes memmap=exactmap has > > > > > too many memory ranges to pass to cmdline. And also memmap=exactmap and > > > > > kASLR doesn't work together. > > > > > > > > > > A better approach, to pass the memory ranges for crash kernel to boot > > > > > into, is filling the memory ranges into E820. > > > > > > > > > > boot_params only got 128 slots for E820 map to fit in, when the number of > > > > > memory map exceeds 128, use setup_data to pass the rest as extended E820 > > > > > memory map. > > > > > > > > > > kexec boot could also benefit from setup_data in case E820 memory map > > > > > exceeds 128. > > > > > > > > > > Now this new approach becomes default instead of memmap=exactmap. > > > > > saved_max_pfn users can specify --pass-memmap-cmdline to use the > > > > > exactmap approach. > > > > > > > > > > Signed-off-by: WANG Chao <chaowang at redhat.com> > > > > > Tested-by: Linn Crosetto <linn at hp.com> > > > > > Reviewed-by: Linn Crosetto <linn at hp.com> > > > > > --- > > > > > kexec/arch/i386/crashdump-x86.c | 6 +- > > > > > kexec/arch/i386/x86-linux-setup.c | 149 +++++++++++++++++++++++++------------- > > > > > kexec/arch/i386/x86-linux-setup.h | 1 + > > > > > 3 files changed, 103 insertions(+), 53 deletions(-) > > > > > > > > > > diff --git a/kexec/arch/i386/crashdump-x86.c b/kexec/arch/i386/crashdump-x86.c > > > > > index 7b618a6..4a1491b 100644 > > > > > --- a/kexec/arch/i386/crashdump-x86.c > > > > > +++ b/kexec/arch/i386/crashdump-x86.c > > > > > @@ -979,7 +979,8 @@ int load_crashdump_segments(struct kexec_info *info, char* mod_cmdline, > > > > > dbgprintf("Created elf header segment at 0x%lx\n", elfcorehdr); > > > > > if (delete_memmap(memmap_p, &nr_memmap, elfcorehdr, memsz) < 0) > > > > > return -1; > > > > > - cmdline_add_memmap(mod_cmdline, memmap_p); > > > > > + if (arch_options.pass_memmap_cmdline) > > > > > + cmdline_add_memmap(mod_cmdline, memmap_p); > > > > > if (!bzImage_support_efi_boot) > > > > > cmdline_add_efi(mod_cmdline); > > > > > cmdline_add_elfcorehdr(mod_cmdline, elfcorehdr); > > > > > @@ -995,7 +996,8 @@ int load_crashdump_segments(struct kexec_info *info, char* mod_cmdline, > > > > > type = mem_range[i].type; > > > > > size = end - start + 1; > > > > > add_memmap(memmap_p, &nr_memmap, start, size, type); > > > > > - cmdline_add_memmap_acpi(mod_cmdline, start, end); > > > > > + if (arch_options.pass_memmap_cmdline) > > > > > + cmdline_add_memmap_acpi(mod_cmdline, start, end); > > > > > > > > Seems memmap_p contains the acpi ranges as well, so cmdline_add_memmap_acpi is > > > > not necessary anymore, just improve cmdline_add_memmap to add both RAM and ACPI > > > > ranges is enough. > > > > > > CRASH_MAX_MEMMAP_NR is used in cmdline_add_memmap, it does not include the acpi > > > ranges, it looks strange, but anyway there's checking about cmdline overflow, tos > > > I think maybe this CRASH_MAX_MEMMAP_NR can be dropped. > > > > First, naming cmdline_add_memmap is not accurate regarding what the > > function does (adding RAM only). But it's historical naming issue, > > nothing to do with this patch :( > > > > I'm sure that could be improved later. > > > > Second, about dropping CRASH_MAX_MEMMAP_NR, I'm not sure if it's a good > > idea. > > > > CRASH_MAX_MEMMAP_NR is used to allocate memmap_p memory: > > > > load_crash_segments(){ > > [..] > > sz = (sizeof(struct memory_range) * CRASH_MAX_MEMMAP_NR); > > memmap_p = xmalloc(sz); > > memset(memmap_p, 0, sz); > > [..] > > } > > > > And so that every time when we walk through memmap_p, > > CRASH_MAX_MEMMAP_NR can be used as a upper boundary. > > > > I think maintain CRASH_MAX_MEMMAP_NR is reasonable and necessary based > > on the current code base. > > Hmm, if it's used for allocate mem range array then how about directly use > CRASH_MAX_MEMORY_RANGES which is same in your 5/9. I really would like to see a cleaner code, but since this issue is trival I will not object it now. For the calgary issue I'm curious if there are people who report a bug. At least if that happens we will know that there's actually someone who are using the calgary code. Thanks Dave