Hi, On Thu, 21 Mar 2024 at 05:56, Huang, Kai <kai.hu...@intel.com> wrote: > > Hi Dave, > > Some nitpicking in changelog:
Will fix the grammar issues. Thanks for your review! > > On 5/03/2024 2:32 pm, Dave Young wrote: > > crashkernel reservation failed on a Thinkpad t440s laptop recently, > > ',' -> '.' to make it as a standalone sentence. > > > Actually the memblock reservation succeeded, but later insert_resource() > > failed. > > > > Test step: > > kexec load -> > > kexec reboot -> > > check the crashkernel memory > > dmesg|grep "crashkernel reserved"; saw reserved suceeeded: > > "suceeeded" -> "succeeded". > > > 0x00000000d0000000 - 0x00000000da000000 > > grep Crash /proc/iomem: got nothing > > And somehow I found it's not easy to read. :-) > > > > > The background story is like below: > > Better to have an blank line to make text more breathable. > > > Currently E820 code reserves setup_data regions for both the current kernel > > and the kexec kernel, and it will also insert them into resources list. > > "will insert" -> "inserts". > > > Before the kexec kernel reboot nobody passes the old setup_data, kexec only > > ^ "reboots" ^ and > > > passes SETUP_EFI and SETUP_IMA if needed. Thus the old setup data memory > > are not used at all. But due to old kernel updated the kexec e820 table > > ^ is ^ updates > > > as well so kexec kernel see them as E820_TYPE_RESERVED_KERN regions, later > > "so kexec kernel" -> ", the kexec kernel" > > "see" -> "sees" > > ", later" -> ", and later" > > > the old setup_data regions will be inserted into resources list in kexec > > "will be" -> "are" > > > kernel by e820__reserve_resources(). > > > > Note, due to no setup_data passed in for those old regions they are not > > ^ is > > > early reserved (by function early_reserve_memory), crashkernel memblock > > ^ and the > > > reservation will just regard them as usable memory and it could reserve > > "regard" -> "treat" > > > reserve crashkernel region overlaps with the old setup_data regions. > > double "reserve". > > "crashkernel region" -> "the crashkernel region" > > "overlaps" -> "which overlaps" > > > > > Just like the bug I noticed here, kdump insert_resource failed because > > e820__reserve_resources added the overlapped chunks in /proc/iomem already. > > ^ has added > > > > Finally, looking at the code, the old setup_data regions are not used > > at all as no setup_data passed in by the kexec boot loader. Although > > ^ is passed > > > something like SETUP_PCI etc could be needed, kexec should pass > > the info as setup_data so that kexec kernel can take care of them. > > This should be taken care of in other separate patches if needed. > > > > Thus drop the useless buggy code here. > > > > Signed-off-by: Dave Young <dyo...@redhat.com> > > --- > > arch/x86/kernel/e820.c | 16 +--------------- > > 1 file changed, 1 insertion(+), 15 deletions(-) > > > > Index: linux/arch/x86/kernel/e820.c > > =================================================================== > > --- linux.orig/arch/x86/kernel/e820.c > > +++ linux/arch/x86/kernel/e820.c > > @@ -1015,16 +1015,6 @@ void __init e820__reserve_setup_data(voi > > pa_next = data->next; > > > > e820__range_update(pa_data, sizeof(*data)+data->len, > > E820_TYPE_RAM, E820_TYPE_RESERVED_KERN); > > - > > - /* > > - * SETUP_EFI and SETUP_IMA are supplied by kexec and do not > > need > > - * to be reserved. > > - */ > > - if (data->type != SETUP_EFI && data->type != SETUP_IMA) > > - e820__range_update_kexec(pa_data, > > - sizeof(*data) + data->len, > > - E820_TYPE_RAM, > > E820_TYPE_RESERVED_KERN); > > - > > if (data->type == SETUP_INDIRECT) { > > len += data->len; > > early_memunmap(data, sizeof(*data)); > > @@ -1036,12 +1026,9 @@ void __init e820__reserve_setup_data(voi > > > > indirect = (struct setup_indirect *)data->data; > > > > - if (indirect->type != SETUP_INDIRECT) { > > + if (indirect->type != SETUP_INDIRECT) > > e820__range_update(indirect->addr, > > indirect->len, > > E820_TYPE_RAM, > > E820_TYPE_RESERVED_KERN); > > - e820__range_update_kexec(indirect->addr, > > indirect->len, > > - E820_TYPE_RAM, > > E820_TYPE_RESERVED_KERN); > > - } > > } > > > > pa_data = pa_next; > > @@ -1049,7 +1036,6 @@ void __init e820__reserve_setup_data(voi > > } > > > > e820__update_table(e820_table); > > - e820__update_table(e820_table_kexec); > > > > pr_info("extended physical RAM map:\n"); > > e820__print_table("reserve setup_data"); > > > > > _______________________________________________ kexec mailing list kexec@lists.infradead.org http://lists.infradead.org/mailman/listinfo/kexec