| Message ID | 20220614092156.1972846-6-mawupeng1@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | introduce mirrored memory support for arm64 | expand |
On 06/14/22 at 05:21pm, Wupeng Ma wrote: > From: Ma Wupeng <mawupeng1@huawei.com> > > If system have some mirrored memory and mirrored feature is not specified > in boot parameter, the basic mirrored feature will be enabled and this will > lead to the following situations: > > - memblock memory allocation prefers mirrored region. This may have some > unexpected influence on numa affinity. > > - contiguous memory will be split into several parts if parts of them > is mirrored memory via memblock_mark_mirror(). > > To fix this, variable mirrored_kernelcore will be checked in > memblock_mark_mirror(). Mark mirrored memory with flag MEMBLOCK_MIRROR iff > kernelcore=mirror is added in the kernel parameters. > > Signed-off-by: Ma Wupeng <mawupeng1@huawei.com> > Acked-by: Ard Biesheuvel <ardb@kernel.org> > --- > mm/internal.h | 2 ++ > mm/memblock.c | 3 +++ > mm/page_alloc.c | 2 +- > 3 files changed, 6 insertions(+), 1 deletion(-) > > diff --git a/mm/internal.h b/mm/internal.h > index c0f8fbe0445b..ddd2d6a46f1b 100644 > --- a/mm/internal.h > +++ b/mm/internal.h > @@ -861,4 +861,6 @@ struct folio *try_grab_folio(struct page *page, int refs, unsigned int flags); > > DECLARE_PER_CPU(struct per_cpu_nodestat, boot_nodestats); > > +extern bool mirrored_kernelcore; > + > #endif /* __MM_INTERNAL_H */ > diff --git a/mm/memblock.c b/mm/memblock.c > index b1d2a0009733..a9f18b988b7f 100644 > --- a/mm/memblock.c > +++ b/mm/memblock.c > @@ -924,6 +924,9 @@ int __init_memblock memblock_clear_hotplug(phys_addr_t base, phys_addr_t size) > */ > int __init_memblock memblock_mark_mirror(phys_addr_t base, phys_addr_t size) > { > + if (!mirrored_kernelcore) > + return 0; memblock_mark_mirror() is just a wrapper, maybe we should check this in efi_find_mirror(). Otherwise, how do we explain the message printed out at below in boot log if we don't mark mirror memory at all? void __init efi_find_mirror(void) { ...... if (mirror_size) pr_info("Memory: %lldM/%lldM mirrored memory\n", mirror_size>>20, total_size>>20); } > + > system_has_some_mirror = true; > > return memblock_setclr_flag(base, size, 1, MEMBLOCK_MIRROR); > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index e008a3df0485..10dc35ec7479 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -356,7 +356,7 @@ static unsigned long required_kernelcore_percent __initdata; > static unsigned long required_movablecore __initdata; > static unsigned long required_movablecore_percent __initdata; > static unsigned long zone_movable_pfn[MAX_NUMNODES] __initdata; > -static bool mirrored_kernelcore __meminitdata; > +bool mirrored_kernelcore __initdata_memblock; > > /* movable_zone is the "real" zone pages in ZONE_MOVABLE are taken from */ > int movable_zone; > -- > 2.25.1 > >
On Tue, 14 Jun 2022 at 12:20, Baoquan He <bhe@redhat.com> wrote: > > On 06/14/22 at 05:21pm, Wupeng Ma wrote: > > From: Ma Wupeng <mawupeng1@huawei.com> > > > > If system have some mirrored memory and mirrored feature is not specified > > in boot parameter, the basic mirrored feature will be enabled and this will > > lead to the following situations: > > > > - memblock memory allocation prefers mirrored region. This may have some > > unexpected influence on numa affinity. > > > > - contiguous memory will be split into several parts if parts of them > > is mirrored memory via memblock_mark_mirror(). > > > > To fix this, variable mirrored_kernelcore will be checked in > > memblock_mark_mirror(). Mark mirrored memory with flag MEMBLOCK_MIRROR iff > > kernelcore=mirror is added in the kernel parameters. > > > > Signed-off-by: Ma Wupeng <mawupeng1@huawei.com> > > Acked-by: Ard Biesheuvel <ardb@kernel.org> > > --- > > mm/internal.h | 2 ++ > > mm/memblock.c | 3 +++ > > mm/page_alloc.c | 2 +- > > 3 files changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/mm/internal.h b/mm/internal.h > > index c0f8fbe0445b..ddd2d6a46f1b 100644 > > --- a/mm/internal.h > > +++ b/mm/internal.h > > @@ -861,4 +861,6 @@ struct folio *try_grab_folio(struct page *page, int refs, unsigned int flags); > > > > DECLARE_PER_CPU(struct per_cpu_nodestat, boot_nodestats); > > > > +extern bool mirrored_kernelcore; > > + > > #endif /* __MM_INTERNAL_H */ > > diff --git a/mm/memblock.c b/mm/memblock.c > > index b1d2a0009733..a9f18b988b7f 100644 > > --- a/mm/memblock.c > > +++ b/mm/memblock.c > > @@ -924,6 +924,9 @@ int __init_memblock memblock_clear_hotplug(phys_addr_t base, phys_addr_t size) > > */ > > int __init_memblock memblock_mark_mirror(phys_addr_t base, phys_addr_t size) > > { > > + if (!mirrored_kernelcore) > > + return 0; > > memblock_mark_mirror() is just a wrapper, maybe we should check this in > efi_find_mirror(). Otherwise, how do we explain the message printed out > at below in boot log if we don't mark mirror memory at all? > > void __init efi_find_mirror(void) > { > ...... > if (mirror_size) > pr_info("Memory: %lldM/%lldM mirrored memory\n", > mirror_size>>20, total_size>>20); > } > EFI does not care about *how* mirrored memory is being used or not, it just reports what the firmware provided. So EFI is not the appropriate level to take kernelcore=mirror into account. I already mentioned that memblock_mark_mirror() is also the wrong place IMO, but Kefeng explained that doing it elsewhere is problematic.
On 06/14/22 at 12:27pm, Ard Biesheuvel wrote: > On Tue, 14 Jun 2022 at 12:20, Baoquan He <bhe@redhat.com> wrote: > > > > On 06/14/22 at 05:21pm, Wupeng Ma wrote: > > > From: Ma Wupeng <mawupeng1@huawei.com> > > > > > > If system have some mirrored memory and mirrored feature is not specified > > > in boot parameter, the basic mirrored feature will be enabled and this will > > > lead to the following situations: > > > > > > - memblock memory allocation prefers mirrored region. This may have some > > > unexpected influence on numa affinity. > > > > > > - contiguous memory will be split into several parts if parts of them > > > is mirrored memory via memblock_mark_mirror(). > > > > > > To fix this, variable mirrored_kernelcore will be checked in > > > memblock_mark_mirror(). Mark mirrored memory with flag MEMBLOCK_MIRROR iff > > > kernelcore=mirror is added in the kernel parameters. > > > > > > Signed-off-by: Ma Wupeng <mawupeng1@huawei.com> > > > Acked-by: Ard Biesheuvel <ardb@kernel.org> > > > --- > > > mm/internal.h | 2 ++ > > > mm/memblock.c | 3 +++ > > > mm/page_alloc.c | 2 +- > > > 3 files changed, 6 insertions(+), 1 deletion(-) > > > > > > diff --git a/mm/internal.h b/mm/internal.h > > > index c0f8fbe0445b..ddd2d6a46f1b 100644 > > > --- a/mm/internal.h > > > +++ b/mm/internal.h > > > @@ -861,4 +861,6 @@ struct folio *try_grab_folio(struct page *page, int refs, unsigned int flags); > > > > > > DECLARE_PER_CPU(struct per_cpu_nodestat, boot_nodestats); > > > > > > +extern bool mirrored_kernelcore; > > > + > > > #endif /* __MM_INTERNAL_H */ > > > diff --git a/mm/memblock.c b/mm/memblock.c > > > index b1d2a0009733..a9f18b988b7f 100644 > > > --- a/mm/memblock.c > > > +++ b/mm/memblock.c > > > @@ -924,6 +924,9 @@ int __init_memblock memblock_clear_hotplug(phys_addr_t base, phys_addr_t size) > > > */ > > > int __init_memblock memblock_mark_mirror(phys_addr_t base, phys_addr_t size) > > > { > > > + if (!mirrored_kernelcore) > > > + return 0; > > > > memblock_mark_mirror() is just a wrapper, maybe we should check this in > > efi_find_mirror(). Otherwise, how do we explain the message printed out > > at below in boot log if we don't mark mirror memory at all? > > > > void __init efi_find_mirror(void) > > { > > ...... > > if (mirror_size) > > pr_info("Memory: %lldM/%lldM mirrored memory\n", > > mirror_size>>20, total_size>>20); > > } > > > > EFI does not care about *how* mirrored memory is being used or not, it > just reports what the firmware provided. So EFI is not the appropriate > level to take kernelcore=mirror into account. > > I already mentioned that memblock_mark_mirror() is also the wrong > place IMO, but Kefeng explained that doing it elsewhere is > problematic. OK, seems we have no better choice other than these two. >
diff --git a/mm/internal.h b/mm/internal.h index c0f8fbe0445b..ddd2d6a46f1b 100644 --- a/mm/internal.h +++ b/mm/internal.h @@ -861,4 +861,6 @@ struct folio *try_grab_folio(struct page *page, int refs, unsigned int flags); DECLARE_PER_CPU(struct per_cpu_nodestat, boot_nodestats); +extern bool mirrored_kernelcore; + #endif /* __MM_INTERNAL_H */ diff --git a/mm/memblock.c b/mm/memblock.c index b1d2a0009733..a9f18b988b7f 100644 --- a/mm/memblock.c +++ b/mm/memblock.c @@ -924,6 +924,9 @@ int __init_memblock memblock_clear_hotplug(phys_addr_t base, phys_addr_t size) */ int __init_memblock memblock_mark_mirror(phys_addr_t base, phys_addr_t size) { + if (!mirrored_kernelcore) + return 0; + system_has_some_mirror = true; return memblock_setclr_flag(base, size, 1, MEMBLOCK_MIRROR); diff --git a/mm/page_alloc.c b/mm/page_alloc.c index e008a3df0485..10dc35ec7479 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -356,7 +356,7 @@ static unsigned long required_kernelcore_percent __initdata; static unsigned long required_movablecore __initdata; static unsigned long required_movablecore_percent __initdata; static unsigned long zone_movable_pfn[MAX_NUMNODES] __initdata; -static bool mirrored_kernelcore __meminitdata; +bool mirrored_kernelcore __initdata_memblock; /* movable_zone is the "real" zone pages in ZONE_MOVABLE are taken from */ int movable_zone;