Re: [PATCH] mm: disable kernelcore=mirror when no mirror memory

[Mike Rapoport <rppt@kernel.org>]

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
> > > 
> > 

-- 
Sincerely yours,
Mike.