| Message ID | 20230728040124.4093229-1-mawupeng1@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm: disable kernelcore=mirror when no mirror memory | expand |
On Fri, Jul 28, 2023 at 12:01:24PM +0800, Wupeng Ma wrote: > From: Ma Wupeng <mawupeng1@huawei.com> > > For system with kernelcore=mirror enabled while no mirrored memory is > reported by efi. This could lead to kernel OOM during startup since > all memory beside zone DMA are in the movable zone and this prevents > the kernel to use it. > > Zone DMA/DMA32 initialization is independent of mirrored memory and > their max pfn is set in zone_sizes_init(). Since kernel can fallback > to zone DMA/DMA32 if there is no memory in zone Normal, these zones > are seen as mirrored memory no mather their memory attributes are. Using kernelcore= and movablecore= always come with the risk there will be to little memory for the kernel to use. Even if EFI reports mirrored memory it's possible to have OOM with kernelcore=mirror because there could be just not enough mirrored memory. > To solve this problem, disable kernelcore=mirror when there is no real > mirrored memory exists. > > Signed-off-by: Ma Wupeng <mawupeng1@huawei.com> > --- > mm/internal.h | 2 ++ > mm/memblock.c | 2 +- > mm/mm_init.c | 6 +++++- > 3 files changed, 8 insertions(+), 2 deletions(-) > > diff --git a/mm/internal.h b/mm/internal.h > index a7d9e980429a..98a03ac74ca7 100644 > --- a/mm/internal.h > +++ b/mm/internal.h > @@ -374,6 +374,8 @@ static inline void clear_zone_contiguous(struct zone *zone) > zone->contiguous = false; > } > > +extern bool system_has_some_mirror; > + > extern int __isolate_free_page(struct page *page, unsigned int order); > extern void __putback_isolated_page(struct page *page, unsigned int order, > int mt); > diff --git a/mm/memblock.c b/mm/memblock.c > index f9e61e565a53..e7a7a65415fb 100644 > --- a/mm/memblock.c > +++ b/mm/memblock.c > @@ -156,10 +156,10 @@ static __refdata struct memblock_type *memblock_memory = &memblock.memory; > } while (0) > > static int memblock_debug __initdata_memblock; > -static bool system_has_some_mirror __initdata_memblock; > static int memblock_can_resize __initdata_memblock; > static int memblock_memory_in_slab __initdata_memblock; > static int memblock_reserved_in_slab __initdata_memblock; > +bool system_has_some_mirror __initdata_memblock; > > static enum memblock_flags __init_memblock choose_memblock_flags(void) > { > diff --git a/mm/mm_init.c b/mm/mm_init.c > index a1963c3322af..6267b9f75927 100644 > --- a/mm/mm_init.c > +++ b/mm/mm_init.c > @@ -269,7 +269,11 @@ static int __init cmdline_parse_kernelcore(char *p) > { > /* parse kernelcore=mirror */ > if (parse_option_str(p, "mirror")) { > - mirrored_kernelcore = true; > + if (system_has_some_mirror) > + mirrored_kernelcore = true; On many architectures early parameters are parsed before memblock is setup, so system_has_some_mirror will always be true. > + else > + pr_warn("The system has no mirror memory, disable kernelcore=mirror.\n"); > + > return 0; > } > > -- > 2.25.1 >
On 2023/7/29 16:12, Mike Rapoport wrote: > On Fri, Jul 28, 2023 at 12:01:24PM +0800, Wupeng Ma wrote: >> From: Ma Wupeng <mawupeng1@huawei.com> >> >> For system with kernelcore=mirror enabled while no mirrored memory is >> reported by efi. This could lead to kernel OOM during startup since >> all memory beside zone DMA are in the movable zone and this prevents >> the kernel to use it. >> >> Zone DMA/DMA32 initialization is independent of mirrored memory and >> their max pfn is set in zone_sizes_init(). Since kernel can fallback >> to zone DMA/DMA32 if there is no memory in zone Normal, these zones >> are seen as mirrored memory no mather their memory attributes are. > > Using kernelcore= and movablecore= always come with the risk there will be > to little memory for the kernel to use. Even if EFI reports mirrored memory > it's possible to have OOM with kernelcore=mirror because there could be > just not enough mirrored memory. Yes, this is a big problem, could we add an option to move some ZONE_MOVABLE pages into ZONE_NORMAL(MIGRATE_MOVABLE) when low free memory?> >> To solve this problem, disable kernelcore=mirror when there is no real >> mirrored memory exists. >> >> Signed-off-by: Ma Wupeng <mawupeng1@huawei.com> >> --- >> mm/internal.h | 2 ++ >> mm/memblock.c | 2 +- >> mm/mm_init.c | 6 +++++- >> 3 files changed, 8 insertions(+), 2 deletions(-) >> >> diff --git a/mm/internal.h b/mm/internal.h >> index a7d9e980429a..98a03ac74ca7 100644 >> --- a/mm/internal.h >> +++ b/mm/internal.h >> @@ -374,6 +374,8 @@ static inline void clear_zone_contiguous(struct zone *zone) >> zone->contiguous = false; >> } >> >> +extern bool system_has_some_mirror; >> + >> extern int __isolate_free_page(struct page *page, unsigned int order); >> extern void __putback_isolated_page(struct page *page, unsigned int order, >> int mt); >> diff --git a/mm/memblock.c b/mm/memblock.c >> index f9e61e565a53..e7a7a65415fb 100644 >> --- a/mm/memblock.c >> +++ b/mm/memblock.c >> @@ -156,10 +156,10 @@ static __refdata struct memblock_type *memblock_memory = &memblock.memory; >> } while (0) >> >> static int memblock_debug __initdata_memblock; >> -static bool system_has_some_mirror __initdata_memblock; >> static int memblock_can_resize __initdata_memblock; >> static int memblock_memory_in_slab __initdata_memblock; >> static int memblock_reserved_in_slab __initdata_memblock; >> +bool system_has_some_mirror __initdata_memblock; >> >> static enum memblock_flags __init_memblock choose_memblock_flags(void) >> { >> diff --git a/mm/mm_init.c b/mm/mm_init.c >> index a1963c3322af..6267b9f75927 100644 >> --- a/mm/mm_init.c >> +++ b/mm/mm_init.c >> @@ -269,7 +269,11 @@ static int __init cmdline_parse_kernelcore(char *p) >> { >> /* parse kernelcore=mirror */ >> if (parse_option_str(p, "mirror")) { >> - mirrored_kernelcore = true; >> + if (system_has_some_mirror) >> + mirrored_kernelcore = true; > > On many architectures early parameters are parsed before memblock is setup, > so system_has_some_mirror will always be true. Only x86/arm64 support kernelcore=mirror, system_has_some_mirror is false by default, so it should no issue for now, but it is better to move this check into find_zone_movable_pfns_for_nodes(). > >> + else >> + pr_warn("The system has no mirror memory, disable kernelcore=mirror.\n"); >> + >> return 0; >> } >> >> -- >> 2.25.1 >> >
On Sat, Jul 29, 2023 at 04:57:17PM +0800, Kefeng Wang wrote: > > > On 2023/7/29 16:12, Mike Rapoport wrote: > > On Fri, Jul 28, 2023 at 12:01:24PM +0800, Wupeng Ma wrote: > > > From: Ma Wupeng <mawupeng1@huawei.com> > > > > > > For system with kernelcore=mirror enabled while no mirrored memory is > > > reported by efi. This could lead to kernel OOM during startup since > > > all memory beside zone DMA are in the movable zone and this prevents > > > the kernel to use it. > > > > > > Zone DMA/DMA32 initialization is independent of mirrored memory and > > > their max pfn is set in zone_sizes_init(). Since kernel can fallback > > > to zone DMA/DMA32 if there is no memory in zone Normal, these zones > > > are seen as mirrored memory no mather their memory attributes are. > > > > Using kernelcore= and movablecore= always come with the risk there will be > > to little memory for the kernel to use. Even if EFI reports mirrored memory > > it's possible to have OOM with kernelcore=mirror because there could be > > just not enough mirrored memory. > > Yes, this is a big problem, could we add an option to move some > ZONE_MOVABLE pages into ZONE_NORMAL(MIGRATE_MOVABLE) when low free > memory?> > > > To solve this problem, disable kernelcore=mirror when there is no real > > > mirrored memory exists. > > > > > > Signed-off-by: Ma Wupeng <mawupeng1@huawei.com> > > > --- > > > mm/internal.h | 2 ++ > > > mm/memblock.c | 2 +- > > > mm/mm_init.c | 6 +++++- > > > 3 files changed, 8 insertions(+), 2 deletions(-) > > > > > > diff --git a/mm/internal.h b/mm/internal.h > > > index a7d9e980429a..98a03ac74ca7 100644 > > > --- a/mm/internal.h > > > +++ b/mm/internal.h > > > @@ -374,6 +374,8 @@ static inline void clear_zone_contiguous(struct zone *zone) > > > zone->contiguous = false; > > > } > > > +extern bool system_has_some_mirror; > > > + > > > extern int __isolate_free_page(struct page *page, unsigned int order); > > > extern void __putback_isolated_page(struct page *page, unsigned int order, > > > int mt); > > > diff --git a/mm/memblock.c b/mm/memblock.c > > > index f9e61e565a53..e7a7a65415fb 100644 > > > --- a/mm/memblock.c > > > +++ b/mm/memblock.c > > > @@ -156,10 +156,10 @@ static __refdata struct memblock_type *memblock_memory = &memblock.memory; > > > } while (0) > > > static int memblock_debug __initdata_memblock; > > > -static bool system_has_some_mirror __initdata_memblock; > > > static int memblock_can_resize __initdata_memblock; > > > static int memblock_memory_in_slab __initdata_memblock; > > > static int memblock_reserved_in_slab __initdata_memblock; > > > +bool system_has_some_mirror __initdata_memblock; > > > static enum memblock_flags __init_memblock choose_memblock_flags(void) > > > { > > > diff --git a/mm/mm_init.c b/mm/mm_init.c > > > index a1963c3322af..6267b9f75927 100644 > > > --- a/mm/mm_init.c > > > +++ b/mm/mm_init.c > > > @@ -269,7 +269,11 @@ static int __init cmdline_parse_kernelcore(char *p) > > > { > > > /* parse kernelcore=mirror */ > > > if (parse_option_str(p, "mirror")) { > > > - mirrored_kernelcore = true; > > > + if (system_has_some_mirror) > > > + mirrored_kernelcore = true; > > > > On many architectures early parameters are parsed before memblock is setup, > > so system_has_some_mirror will always be true. > > Only x86/arm64 support kernelcore=mirror, system_has_some_mirror is > false by default, so it should no issue for now, but it is better to > move this check into find_zone_movable_pfns_for_nodes(). Sorry, I meant that system_has_some_mirror is false by default, and both x86/arm64 parse early parameters before they set up memblock, so system_has_some_mirror will be always false at this point. > > > + else > > > + pr_warn("The system has no mirror memory, disable kernelcore=mirror.\n"); > > > + > > > return 0; > > > } > > > -- > > > 2.25.1 > > > > >
On 2023/7/30 14:53, Mike Rapoport wrote: > On Sat, Jul 29, 2023 at 04:57:17PM +0800, Kefeng Wang wrote: >> >> >> On 2023/7/29 16:12, Mike Rapoport wrote: >>> On Fri, Jul 28, 2023 at 12:01:24PM +0800, Wupeng Ma wrote: >>>> From: Ma Wupeng <mawupeng1@huawei.com> >>>> >>>> For system with kernelcore=mirror enabled while no mirrored memory is >>>> reported by efi. This could lead to kernel OOM during startup since >>>> all memory beside zone DMA are in the movable zone and this prevents >>>> the kernel to use it. >>>> >>>> Zone DMA/DMA32 initialization is independent of mirrored memory and >>>> their max pfn is set in zone_sizes_init(). Since kernel can fallback >>>> to zone DMA/DMA32 if there is no memory in zone Normal, these zones >>>> are seen as mirrored memory no mather their memory attributes are. >>> >>> Using kernelcore= and movablecore= always come with the risk there will be >>> to little memory for the kernel to use. Even if EFI reports mirrored memory >>> it's possible to have OOM with kernelcore=mirror because there could be >>> just not enough mirrored memory. >> >> Yes, this is a big problem, could we add an option to move some >> ZONE_MOVABLE pages into ZONE_NORMAL(MIGRATE_MOVABLE) when low free >> memory?> >>>> To solve this problem, disable kernelcore=mirror when there is no real >>>> mirrored memory exists. >>>> >>>> Signed-off-by: Ma Wupeng <mawupeng1@huawei.com> >>>> --- >>>> mm/internal.h | 2 ++ >>>> mm/memblock.c | 2 +- >>>> mm/mm_init.c | 6 +++++- >>>> 3 files changed, 8 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/mm/internal.h b/mm/internal.h >>>> index a7d9e980429a..98a03ac74ca7 100644 >>>> --- a/mm/internal.h >>>> +++ b/mm/internal.h >>>> @@ -374,6 +374,8 @@ static inline void clear_zone_contiguous(struct zone *zone) >>>> zone->contiguous = false; >>>> } >>>> +extern bool system_has_some_mirror; >>>> + >>>> extern int __isolate_free_page(struct page *page, unsigned int order); >>>> extern void __putback_isolated_page(struct page *page, unsigned int order, >>>> int mt); >>>> diff --git a/mm/memblock.c b/mm/memblock.c >>>> index f9e61e565a53..e7a7a65415fb 100644 >>>> --- a/mm/memblock.c >>>> +++ b/mm/memblock.c >>>> @@ -156,10 +156,10 @@ static __refdata struct memblock_type *memblock_memory = &memblock.memory; >>>> } while (0) >>>> static int memblock_debug __initdata_memblock; >>>> -static bool system_has_some_mirror __initdata_memblock; >>>> static int memblock_can_resize __initdata_memblock; >>>> static int memblock_memory_in_slab __initdata_memblock; >>>> static int memblock_reserved_in_slab __initdata_memblock; >>>> +bool system_has_some_mirror __initdata_memblock; >>>> static enum memblock_flags __init_memblock choose_memblock_flags(void) >>>> { >>>> diff --git a/mm/mm_init.c b/mm/mm_init.c >>>> index a1963c3322af..6267b9f75927 100644 >>>> --- a/mm/mm_init.c >>>> +++ b/mm/mm_init.c >>>> @@ -269,7 +269,11 @@ static int __init cmdline_parse_kernelcore(char *p) >>>> { >>>> /* parse kernelcore=mirror */ >>>> if (parse_option_str(p, "mirror")) { >>>> - mirrored_kernelcore = true; >>>> + if (system_has_some_mirror) >>>> + mirrored_kernelcore = true; >>> >>> On many architectures early parameters are parsed before memblock is setup, >>> so system_has_some_mirror will always be true. >> >> Only x86/arm64 support kernelcore=mirror, system_has_some_mirror is >> false by default, so it should no issue for now, but it is better to >> move this check into find_zone_movable_pfns_for_nodes(). > > Sorry, I meant that system_has_some_mirror is false by default, and both > x86/arm64 parse early parameters before they set up memblock, so > system_has_some_mirror will be always false at this point. Clear, so let's move check into find_zone_movable_pfns_for_nodes()(no test), is this ok? diff --git a/mm/internal.h b/mm/internal.h index a7d9e980429a..1599becc9079 100644 --- a/mm/internal.h +++ b/mm/internal.h @@ -1005,6 +1005,7 @@ static inline bool gup_must_unshare(struct vm_area_struct *vma, } extern bool mirrored_kernelcore; +extern bool system_has_some_mirror; static inline bool vma_soft_dirty_enabled(struct vm_area_struct *vma) { diff --git a/mm/memblock.c b/mm/memblock.c index f9e61e565a53..e7a7a65415fb 100644 --- a/mm/memblock.c +++ b/mm/memblock.c @@ -156,10 +156,10 @@ static __refdata struct memblock_type *memblock_memory = &memblock.memory; } while (0) static int memblock_debug __initdata_memblock; -static bool system_has_some_mirror __initdata_memblock; static int memblock_can_resize __initdata_memblock; static int memblock_memory_in_slab __initdata_memblock; static int memblock_reserved_in_slab __initdata_memblock; +bool system_has_some_mirror __initdata_memblock; static enum memblock_flags __init_memblock choose_memblock_flags(void) { diff --git a/mm/mm_init.c b/mm/mm_init.c index a1963c3322af..c444da6065a6 100644 --- a/mm/mm_init.c +++ b/mm/mm_init.c @@ -377,6 +377,11 @@ static void __init find_zone_movable_pfns_for_nodes(void) if (mirrored_kernelcore) { bool mem_below_4gb_not_mirrored = false; + if (!system_has_some_mirror) { + pr_warn("The system has no mirror memory, ignore kernelcore=mirror.\n"); + goto out; + } + for_each_mem_region(r) { if (memblock_is_mirror(r)) continue;
On 2023/7/30 14:53, Mike Rapoport wrote: > On Sat, Jul 29, 2023 at 04:57:17PM +0800, Kefeng Wang wrote: >> >> >> On 2023/7/29 16:12, Mike Rapoport wrote: >>> On Fri, Jul 28, 2023 at 12:01:24PM +0800, Wupeng Ma wrote: >>>> From: Ma Wupeng <mawupeng1@huawei.com> >>>> >>>> For system with kernelcore=mirror enabled while no mirrored memory is >>>> reported by efi. This could lead to kernel OOM during startup since >>>> all memory beside zone DMA are in the movable zone and this prevents >>>> the kernel to use it. >>>> >>>> Zone DMA/DMA32 initialization is independent of mirrored memory and >>>> their max pfn is set in zone_sizes_init(). Since kernel can fallback >>>> to zone DMA/DMA32 if there is no memory in zone Normal, these zones >>>> are seen as mirrored memory no mather their memory attributes are. >>> >>> Using kernelcore= and movablecore= always come with the risk there will be >>> to little memory for the kernel to use. Even if EFI reports mirrored memory >>> it's possible to have OOM with kernelcore=mirror because there could be >>> just not enough mirrored memory. Although there could be just not enough mirrored memory even EFI reports, it may be better to avoid this kind of oom if there is something with with EFI mirror or this system is boot with legacy BIOS without the support of mirrored memory? >> >> Yes, this is a big problem, could we add an option to move some >> ZONE_MOVABLE pages into ZONE_NORMAL(MIGRATE_MOVABLE) when low free >> memory?> >>>> To solve this problem, disable kernelcore=mirror when there is no real >>>> mirrored memory exists. >>>> >>>> Signed-off-by: Ma Wupeng <mawupeng1@huawei.com> >>>> --- >>>> mm/internal.h | 2 ++ >>>> mm/memblock.c | 2 +- >>>> mm/mm_init.c | 6 +++++- >>>> 3 files changed, 8 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/mm/internal.h b/mm/internal.h >>>> index a7d9e980429a..98a03ac74ca7 100644 >>>> --- a/mm/internal.h >>>> +++ b/mm/internal.h >>>> @@ -374,6 +374,8 @@ static inline void clear_zone_contiguous(struct zone *zone) >>>> zone->contiguous = false; >>>> } >>>> +extern bool system_has_some_mirror; >>>> + >>>> extern int __isolate_free_page(struct page *page, unsigned int order); >>>> extern void __putback_isolated_page(struct page *page, unsigned int order, >>>> int mt); >>>> diff --git a/mm/memblock.c b/mm/memblock.c >>>> index f9e61e565a53..e7a7a65415fb 100644 >>>> --- a/mm/memblock.c >>>> +++ b/mm/memblock.c >>>> @@ -156,10 +156,10 @@ static __refdata struct memblock_type *memblock_memory = &memblock.memory; >>>> } while (0) >>>> static int memblock_debug __initdata_memblock; >>>> -static bool system_has_some_mirror __initdata_memblock; >>>> static int memblock_can_resize __initdata_memblock; >>>> static int memblock_memory_in_slab __initdata_memblock; >>>> static int memblock_reserved_in_slab __initdata_memblock; >>>> +bool system_has_some_mirror __initdata_memblock; >>>> static enum memblock_flags __init_memblock choose_memblock_flags(void) >>>> { >>>> diff --git a/mm/mm_init.c b/mm/mm_init.c >>>> index a1963c3322af..6267b9f75927 100644 >>>> --- a/mm/mm_init.c >>>> +++ b/mm/mm_init.c >>>> @@ -269,7 +269,11 @@ static int __init cmdline_parse_kernelcore(char *p) >>>> { >>>> /* parse kernelcore=mirror */ >>>> if (parse_option_str(p, "mirror")) { >>>> - mirrored_kernelcore = true; >>>> + if (system_has_some_mirror) >>>> + mirrored_kernelcore = true; >>> >>> On many architectures early parameters are parsed before memblock is setup, >>> so system_has_some_mirror will always be true. >> >> Only x86/arm64 support kernelcore=mirror, system_has_some_mirror is >> false by default, so it should no issue for now, but it is better to >> move this check into find_zone_movable_pfns_for_nodes(). > > Sorry, I meant that system_has_some_mirror is false by default, and both > x86/arm64 parse early parameters before they set up memblock, so > system_has_some_mirror will be always false at this point. Yes, you are right. system_has_some_mirror will be always false at this point. > >>>> + else >>>> + pr_warn("The system has no mirror memory, disable kernelcore=mirror.\n"); >>>> + >>>> return 0; >>>> } >>>> -- >>>> 2.25.1 >>>> >>> >
On 2023/7/31 9:39, Kefeng Wang wrote: > > > On 2023/7/30 14:53, Mike Rapoport wrote: >> On Sat, Jul 29, 2023 at 04:57:17PM +0800, Kefeng Wang wrote: >>> >>> >>> On 2023/7/29 16:12, Mike Rapoport wrote: >>>> On Fri, Jul 28, 2023 at 12:01:24PM +0800, Wupeng Ma wrote: >>>>> From: Ma Wupeng <mawupeng1@huawei.com> >>>>> >>>>> For system with kernelcore=mirror enabled while no mirrored memory is >>>>> reported by efi. This could lead to kernel OOM during startup since >>>>> all memory beside zone DMA are in the movable zone and this prevents >>>>> the kernel to use it. >>>>> >>>>> Zone DMA/DMA32 initialization is independent of mirrored memory and >>>>> their max pfn is set in zone_sizes_init(). Since kernel can fallback >>>>> to zone DMA/DMA32 if there is no memory in zone Normal, these zones >>>>> are seen as mirrored memory no mather their memory attributes are. >>>> >>>> Using kernelcore= and movablecore= always come with the risk there will be >>>> to little memory for the kernel to use. Even if EFI reports mirrored memory >>>> it's possible to have OOM with kernelcore=mirror because there could be >>>> just not enough mirrored memory. >>> >>> Yes, this is a big problem, could we add an option to move some >>> ZONE_MOVABLE pages into ZONE_NORMAL(MIGRATE_MOVABLE) when low free >>> memory?> >>>>> To solve this problem, disable kernelcore=mirror when there is no real >>>>> mirrored memory exists. >>>>> >>>>> Signed-off-by: Ma Wupeng <mawupeng1@huawei.com> >>>>> --- >>>>> mm/internal.h | 2 ++ >>>>> mm/memblock.c | 2 +- >>>>> mm/mm_init.c | 6 +++++- >>>>> 3 files changed, 8 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/mm/internal.h b/mm/internal.h >>>>> index a7d9e980429a..98a03ac74ca7 100644 >>>>> --- a/mm/internal.h >>>>> +++ b/mm/internal.h >>>>> @@ -374,6 +374,8 @@ static inline void clear_zone_contiguous(struct zone *zone) >>>>> zone->contiguous = false; >>>>> } >>>>> +extern bool system_has_some_mirror; >>>>> + >>>>> extern int __isolate_free_page(struct page *page, unsigned int order); >>>>> extern void __putback_isolated_page(struct page *page, unsigned int order, >>>>> int mt); >>>>> diff --git a/mm/memblock.c b/mm/memblock.c >>>>> index f9e61e565a53..e7a7a65415fb 100644 >>>>> --- a/mm/memblock.c >>>>> +++ b/mm/memblock.c >>>>> @@ -156,10 +156,10 @@ static __refdata struct memblock_type *memblock_memory = &memblock.memory; >>>>> } while (0) >>>>> static int memblock_debug __initdata_memblock; >>>>> -static bool system_has_some_mirror __initdata_memblock; >>>>> static int memblock_can_resize __initdata_memblock; >>>>> static int memblock_memory_in_slab __initdata_memblock; >>>>> static int memblock_reserved_in_slab __initdata_memblock; >>>>> +bool system_has_some_mirror __initdata_memblock; >>>>> static enum memblock_flags __init_memblock choose_memblock_flags(void) >>>>> { >>>>> diff --git a/mm/mm_init.c b/mm/mm_init.c >>>>> index a1963c3322af..6267b9f75927 100644 >>>>> --- a/mm/mm_init.c >>>>> +++ b/mm/mm_init.c >>>>> @@ -269,7 +269,11 @@ static int __init cmdline_parse_kernelcore(char *p) >>>>> { >>>>> /* parse kernelcore=mirror */ >>>>> if (parse_option_str(p, "mirror")) { >>>>> - mirrored_kernelcore = true; >>>>> + if (system_has_some_mirror) >>>>> + mirrored_kernelcore = true; >>>> >>>> On many architectures early parameters are parsed before memblock is setup, >>>> so system_has_some_mirror will always be true. >>> >>> Only x86/arm64 support kernelcore=mirror, system_has_some_mirror is >>> false by default, so it should no issue for now, but it is better to >>> move this check into find_zone_movable_pfns_for_nodes(). >> Sorry, I meant that system_has_some_mirror is false by default, and both >> x86/arm64 parse early parameters before they set up memblock, so >> system_has_some_mirror will be always false at this point. > > Clear, so let's move check into find_zone_movable_pfns_for_nodes()(no test), is this ok? > > diff --git a/mm/internal.h b/mm/internal.h > index a7d9e980429a..1599becc9079 100644 > --- a/mm/internal.h > +++ b/mm/internal.h > @@ -1005,6 +1005,7 @@ static inline bool gup_must_unshare(struct vm_area_struct *vma, > } > > extern bool mirrored_kernelcore; > +extern bool system_has_some_mirror; > > static inline bool vma_soft_dirty_enabled(struct vm_area_struct *vma) > { > diff --git a/mm/memblock.c b/mm/memblock.c > index f9e61e565a53..e7a7a65415fb 100644 > --- a/mm/memblock.c > +++ b/mm/memblock.c > @@ -156,10 +156,10 @@ static __refdata struct memblock_type *memblock_memory = &memblock.memory; > } while (0) > > static int memblock_debug __initdata_memblock; > -static bool system_has_some_mirror __initdata_memblock; > static int memblock_can_resize __initdata_memblock; > static int memblock_memory_in_slab __initdata_memblock; > static int memblock_reserved_in_slab __initdata_memblock; > +bool system_has_some_mirror __initdata_memblock; > > static enum memblock_flags __init_memblock choose_memblock_flags(void) > { > diff --git a/mm/mm_init.c b/mm/mm_init.c > index a1963c3322af..c444da6065a6 100644 > --- a/mm/mm_init.c > +++ b/mm/mm_init.c > @@ -377,6 +377,11 @@ static void __init find_zone_movable_pfns_for_nodes(void) > if (mirrored_kernelcore) { > bool mem_below_4gb_not_mirrored = false; > > + if (!system_has_some_mirror) { > + pr_warn("The system has no mirror memory, ignore kernelcore=mirror.\n"); > + goto out; > + } > + > for_each_mem_region(r) { > if (memblock_is_mirror(r)) > continue; Test on 6.5.0-rc3-next-20230728, everything works fine.
On Mon, Jul 31, 2023 at 09:39:37AM +0800, Kefeng Wang wrote: > > On 2023/7/30 14:53, Mike Rapoport wrote: > > On Sat, Jul 29, 2023 at 04:57:17PM +0800, Kefeng Wang wrote: > > > > > > > > > On 2023/7/29 16:12, Mike Rapoport wrote: > > > > On Fri, Jul 28, 2023 at 12:01:24PM +0800, Wupeng Ma wrote: > > > > > From: Ma Wupeng <mawupeng1@huawei.com> > > > > > > > > > > For system with kernelcore=mirror enabled while no mirrored memory is > > > > > reported by efi. This could lead to kernel OOM during startup since > > > > > all memory beside zone DMA are in the movable zone and this prevents > > > > > the kernel to use it. > > > > > > > > > > Zone DMA/DMA32 initialization is independent of mirrored memory and > > > > > their max pfn is set in zone_sizes_init(). Since kernel can fallback > > > > > to zone DMA/DMA32 if there is no memory in zone Normal, these zones > > > > > are seen as mirrored memory no mather their memory attributes are. > > > > > > > > Using kernelcore= and movablecore= always come with the risk there will be > > > > to little memory for the kernel to use. Even if EFI reports mirrored memory > > > > it's possible to have OOM with kernelcore=mirror because there could be > > > > just not enough mirrored memory. > > > > > > > > > Only x86/arm64 support kernelcore=mirror, system_has_some_mirror is > > > false by default, so it should no issue for now, but it is better to > > > move this check into find_zone_movable_pfns_for_nodes(). > > > > Sorry, I meant that system_has_some_mirror is false by default, and both > > x86/arm64 parse early parameters before they set up memblock, so > > system_has_some_mirror will be always false at this point. > > Clear, so let's move check into find_zone_movable_pfns_for_nodes()(no test), > is this ok? This looks fine to me, just one nit below. > diff --git a/mm/internal.h b/mm/internal.h > index a7d9e980429a..1599becc9079 100644 > --- a/mm/internal.h > +++ b/mm/internal.h > @@ -1005,6 +1005,7 @@ static inline bool gup_must_unshare(struct > vm_area_struct *vma, > } > > extern bool mirrored_kernelcore; > +extern bool system_has_some_mirror; I prefer keeping system_has_some_mirror static and adding, say, memblock_has_mirror(). > static inline bool vma_soft_dirty_enabled(struct vm_area_struct *vma) > { > diff --git a/mm/memblock.c b/mm/memblock.c > index f9e61e565a53..e7a7a65415fb 100644 > --- a/mm/memblock.c > +++ b/mm/memblock.c > @@ -156,10 +156,10 @@ static __refdata struct memblock_type *memblock_memory > = &memblock.memory; > } while (0) > > static int memblock_debug __initdata_memblock; > -static bool system_has_some_mirror __initdata_memblock; > static int memblock_can_resize __initdata_memblock; > static int memblock_memory_in_slab __initdata_memblock; > static int memblock_reserved_in_slab __initdata_memblock; > +bool system_has_some_mirror __initdata_memblock; > > static enum memblock_flags __init_memblock choose_memblock_flags(void) > { > diff --git a/mm/mm_init.c b/mm/mm_init.c > index a1963c3322af..c444da6065a6 100644 > --- a/mm/mm_init.c > +++ b/mm/mm_init.c > @@ -377,6 +377,11 @@ static void __init > find_zone_movable_pfns_for_nodes(void) > if (mirrored_kernelcore) { > bool mem_below_4gb_not_mirrored = false; > > + if (!system_has_some_mirror) { > + pr_warn("The system has no mirror memory, ignore kernelcore=mirror.\n"); > + goto out; > + } > + > for_each_mem_region(r) { > if (memblock_is_mirror(r)) > continue; >
diff --git a/mm/internal.h b/mm/internal.h index a7d9e980429a..98a03ac74ca7 100644 --- a/mm/internal.h +++ b/mm/internal.h @@ -374,6 +374,8 @@ static inline void clear_zone_contiguous(struct zone *zone) zone->contiguous = false; } +extern bool system_has_some_mirror; + extern int __isolate_free_page(struct page *page, unsigned int order); extern void __putback_isolated_page(struct page *page, unsigned int order, int mt); diff --git a/mm/memblock.c b/mm/memblock.c index f9e61e565a53..e7a7a65415fb 100644 --- a/mm/memblock.c +++ b/mm/memblock.c @@ -156,10 +156,10 @@ static __refdata struct memblock_type *memblock_memory = &memblock.memory; } while (0) static int memblock_debug __initdata_memblock; -static bool system_has_some_mirror __initdata_memblock; static int memblock_can_resize __initdata_memblock; static int memblock_memory_in_slab __initdata_memblock; static int memblock_reserved_in_slab __initdata_memblock; +bool system_has_some_mirror __initdata_memblock; static enum memblock_flags __init_memblock choose_memblock_flags(void) { diff --git a/mm/mm_init.c b/mm/mm_init.c index a1963c3322af..6267b9f75927 100644 --- a/mm/mm_init.c +++ b/mm/mm_init.c @@ -269,7 +269,11 @@ static int __init cmdline_parse_kernelcore(char *p) { /* parse kernelcore=mirror */ if (parse_option_str(p, "mirror")) { - mirrored_kernelcore = true; + if (system_has_some_mirror) + mirrored_kernelcore = true; + else + pr_warn("The system has no mirror memory, disable kernelcore=mirror.\n"); + return 0; }