| Message ID | 20220613082147.183145-7-mawupeng1@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | introduce mirrored memory support for arm64 | expand |
On 6/13/2022 4:21 PM, 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> > --- > 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; > + > 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..9b030aeb4983 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; __initdata here is not suitable and will lead to compile warnings. In my test, __initdata_memblock and ro_after_init are both fine, but I am not sure which one to choose? Do you have any idea on this? > > /* movable_zone is the "real" zone pages in ZONE_MOVABLE are taken from */ > int movable_zone;
On Mon, 13 Jun 2022 at 11:13, mawupeng <mawupeng1@huawei.com> wrote: > > > > On 6/13/2022 4:21 PM, 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> > > --- > > 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; > > + > > 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..9b030aeb4983 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; > > __initdata here is not suitable and will lead to compile warnings. > > In my test, __initdata_memblock and ro_after_init are both fine, but I am not > sure which one to choose? Do you have any idea on this? > __initdata_memblock is fine if it works. This looks to me like the right place to implement this policy, so Acked-by: Ard Biesheuvel <ardb@kernel.org> > > > > /* movable_zone is the "real" zone pages in ZONE_MOVABLE are taken from */ > > int movable_zone;
On Mon, Jun 13, 2022 at 04:21:47PM +0800, 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> > --- > 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; > + Hmm, this changes the way x86 uses mirrored memory. This change makes sense for x86 as well, but we should get an Ack from x86 folks. > 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..9b030aeb4983 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; > > /* movable_zone is the "real" zone pages in ZONE_MOVABLE are taken from */ > int movable_zone; > -- > 2.25.1 >
On Mon, 13 Jun 2022 at 13:05, Mike Rapoport <rppt@kernel.org> wrote: > > On Mon, Jun 13, 2022 at 04:21:47PM +0800, 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> > > --- > > 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; > > + > > Hmm, this changes the way x86 uses mirrored memory. > This change makes sense for x86 as well, but we should get an Ack from x86 folks. > Also, on second thought, I don't think marking as mirror is what should be affected by the policy. Instead, choose_memblock_flags() should take this into account, in a way that we could refine later if needed.
On 2022/6/13 19:25, Ard Biesheuvel wrote: > On Mon, 13 Jun 2022 at 13:05, Mike Rapoport <rppt@kernel.org> wrote: >> On Mon, Jun 13, 2022 at 04:21:47PM +0800, 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(). ... > Also, on second thought, I don't think marking as mirror is what > should be affected by the policy. Instead, choose_memblock_flags() > should take this into account, in a way that we could refine later if > needed. > . The choose_memblock_flags() only solve the issue of memblock allocation, but the memblock could be splitted and fragmentized, the kernel won't treat the mirror memory as special if no mirrored_kernelcore for now, so I think we'd better to add the check into memblock_mark_mirror().
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..9b030aeb4983 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; /* movable_zone is the "real" zone pages in ZONE_MOVABLE are taken from */ int movable_zone;