2022-06-13 08:12:22

by mawupeng

[permalink] [raw]
Subject: [PATCH v4 0/6] introduce mirrored memory support for arm64

From: Ma Wupeng <[email protected]>

Commit b05b9f5f9dcf ("x86, mirror: x86 enabling - find mirrored memory ranges")
introduced mirrored memory support for x86. This support rely on UEFI to
report mirrored memory address ranges. See UEFI 2.5 spec pages 157-158:

http://www.uefi.org/sites/default/files/resources/UEFI%202_5.pdf

Memory mirroring is a technique used to separate memory into two separate
channels, usually on a memory device, like a server. In memory mirroring,
one channel is copied to another to create redundancy. This method makes
input/output (I/O) registers and memory appear with more than one address
range because the same physical byte is accessible at more than one
address. Using memory mirroring, higher memory reliability and a higher
level of memory consolidation are possible.

These EFI memory regions have various attributes, and the "mirrored"
attribute is one of them. The physical memory region whose descriptors
in EFI memory map has EFI_MEMORY_MORE_RELIABLE attribute (bit: 16) are
mirrored. The address range mirroring feature of the kernel arranges such
mirrored regions into normal zones and other regions into movable zones.

Arm64 can support this too. So mirrored memory support is added to support
arm64.

The main purpose of this patch set is to introduce mirrored support for
arm64 and we have already fixed the problems we had which is shown in
patch #5 to patch #8 and try to bring total isolation in patch #9 which
will disable mirror feature if kernelcore is not specified.

In order to test this support in arm64:
- patch this patch set
- add kernelcore=mirror in kernel parameter
- start you kernel

Patch #1-#2 introduce mirrored memory support form arm64.
Patch #3-#5 fix some bugs for arm64 if memory reliable is enabled.
Patch #6 disable mirror feature if kernelcore is not specified.

Thanks to Ard Biesheuvel's hard work [1], now kernel will perfer mirrored
memory if kaslr is enabled.

[1] https://lore.kernel.org/linux-arm-kernel/CAMj1kXEPVEzMgOM4+Yj6PxHA-jFuDOAUdDJSiSxy_XaP4P7LSw@mail.gmail.com/T/

Changelog since v3:
- limit warning message in vmemmap_verify via pr_warn_once()
- only clear memblock_nomap flags rather than bring the mirrored flag back
- disable mirrored feature in memblock_mark_mirror()

Changelog since v2:
- remove efi_fake_mem support
- remove Commit ("remove some redundant code in ia64 efi_init") since
efi_print_memmap() is not public
- add mirror flag back on initrd memory

Changelog since v1:
- update changelog in cover letter
- use PHYS_PFN in patch #7

Ma Wupeng (6):
efi: Make efi_find_mirror() public
arm64/mirror: arm64 enabling - find mirrored memory ranges
mm: Ratelimited mirrored memory related warning messages
mm: Limit warning message in vmemmap_verify() to once
mm: Only remove nomap flag for initrd
memblock: Disable mirror feature if kernelcore is not specified

.../admin-guide/kernel-parameters.txt | 2 +-
arch/arm64/kernel/setup.c | 1 +
arch/arm64/mm/init.c | 2 +-
arch/x86/include/asm/efi.h | 4 ----
arch/x86/platform/efi/efi.c | 23 -------------------
drivers/firmware/efi/efi.c | 23 +++++++++++++++++++
include/linux/efi.h | 3 +++
mm/internal.h | 2 ++
mm/memblock.c | 7 ++++--
mm/page_alloc.c | 2 +-
mm/sparse-vmemmap.c | 2 +-
11 files changed, 38 insertions(+), 33 deletions(-)

--
2.25.1


2022-06-13 08:24:47

by mawupeng

[permalink] [raw]
Subject: [PATCH v4 3/6] mm: Ratelimited mirrored memory related warning messages

From: Ma Wupeng <[email protected]>

If system has mirrored memory, memblock will try to allocate mirrored
memory firstly and fallback to non-mirrored memory when fails, but if with
limited mirrored memory or some numa node without mirrored memory, lots of
warning message about memblock allocation will occur.

This patch ratelimit the warning message to avoid a very long print during
bootup.

Signed-off-by: Ma Wupeng <[email protected]>
Reviewed-by: David Hildenbrand <[email protected]>
Acked-by: Mike Rapoport <[email protected]>
Reviewed-by: Anshuman Khandual <[email protected]>
Reviewed-by: Kefeng Wang <[email protected]>
---
mm/memblock.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mm/memblock.c b/mm/memblock.c
index e4f03a6e8e56..b1d2a0009733 100644
--- a/mm/memblock.c
+++ b/mm/memblock.c
@@ -327,7 +327,7 @@ static phys_addr_t __init_memblock memblock_find_in_range(phys_addr_t start,
NUMA_NO_NODE, flags);

if (!ret && (flags & MEMBLOCK_MIRROR)) {
- pr_warn("Could not allocate %pap bytes of mirrored memory\n",
+ pr_warn_ratelimited("Could not allocate %pap bytes of mirrored memory\n",
&size);
flags &= ~MEMBLOCK_MIRROR;
goto again;
@@ -1384,7 +1384,7 @@ phys_addr_t __init memblock_alloc_range_nid(phys_addr_t size,

if (flags & MEMBLOCK_MIRROR) {
flags &= ~MEMBLOCK_MIRROR;
- pr_warn("Could not allocate %pap bytes of mirrored memory\n",
+ pr_warn_ratelimited("Could not allocate %pap bytes of mirrored memory\n",
&size);
goto again;
}
--
2.25.1

2022-06-13 08:24:58

by mawupeng

[permalink] [raw]
Subject: [PATCH v4 2/6] arm64/mirror: arm64 enabling - find mirrored memory ranges

From: Ma Wupeng <[email protected]>

Commit b05b9f5f9dcf ("x86, mirror: x86 enabling - find mirrored memory ranges")
introduced mirrored memory support for x86 and this could be used on arm64.

Since we only support this feature on arm64, efi_find_mirror() won't be placed
into efi_init(), which is used by riscv/arm/arm64, it is added in setup_arch()
to scan the memory map and mark mirrored memory in memblock.

Signed-off-by: Ma Wupeng <[email protected]>
Reviewed-by: Kefeng Wang <[email protected]>
---
Documentation/admin-guide/kernel-parameters.txt | 2 +-
arch/arm64/kernel/setup.c | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 8090130b544b..e3537646b6f7 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -2301,7 +2301,7 @@

keepinitrd [HW,ARM]

- kernelcore= [KNL,X86,IA-64,PPC]
+ kernelcore= [KNL,X86,IA-64,PPC,ARM64]
Format: nn[KMGTPE] | nn% | "mirror"
This parameter specifies the amount of memory usable by
the kernel for non-movable allocations. The requested
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index cf3a759f10d4..6e9acd7ecf0f 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -328,6 +328,7 @@ void __init __no_sanitize_address setup_arch(char **cmdline_p)

xen_early_init();
efi_init();
+ efi_find_mirror();

if (!efi_enabled(EFI_BOOT) && ((u64)_text % MIN_KIMG_ALIGN) != 0)
pr_warn(FW_BUG "Kernel image misaligned at boot, please fix your bootloader!");
--
2.25.1

2022-06-13 08:25:42

by mawupeng

[permalink] [raw]
Subject: [PATCH v4 6/6] memblock: Disable mirror feature if kernelcore is not specified

From: Ma Wupeng <[email protected]>

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 <[email protected]>
---
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;

/* movable_zone is the "real" zone pages in ZONE_MOVABLE are taken from */
int movable_zone;
--
2.25.1

2022-06-13 08:26:35

by mawupeng

[permalink] [raw]
Subject: [PATCH v4 1/6] efi: Make efi_find_mirror() public

From: Ma Wupeng <[email protected]>

Commit b05b9f5f9dcf ("x86, mirror: x86 enabling - find mirrored memory
ranges") introduce the efi_find_mirror function on x86. In order to reuse
the API we make it public in preparation for arm64 to support mirrord
memory.

Signed-off-by: Ma Wupeng <[email protected]>
Reviewed-by: Kefeng Wang <[email protected]>
---
arch/x86/include/asm/efi.h | 4 ----
arch/x86/platform/efi/efi.c | 23 -----------------------
drivers/firmware/efi/efi.c | 23 +++++++++++++++++++++++
include/linux/efi.h | 3 +++
4 files changed, 26 insertions(+), 27 deletions(-)

diff --git a/arch/x86/include/asm/efi.h b/arch/x86/include/asm/efi.h
index 71943dce691e..eb90206eae80 100644
--- a/arch/x86/include/asm/efi.h
+++ b/arch/x86/include/asm/efi.h
@@ -383,7 +383,6 @@ static inline bool efi_is_64bit(void)
extern bool efi_reboot_required(void);
extern bool efi_is_table_address(unsigned long phys_addr);

-extern void efi_find_mirror(void);
extern void efi_reserve_boot_services(void);
#else
static inline void parse_efi_setup(u64 phys_addr, u32 data_len) {}
@@ -395,9 +394,6 @@ static inline bool efi_is_table_address(unsigned long phys_addr)
{
return false;
}
-static inline void efi_find_mirror(void)
-{
-}
static inline void efi_reserve_boot_services(void)
{
}
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c
index 1591d67e0bcd..6e598bd78eef 100644
--- a/arch/x86/platform/efi/efi.c
+++ b/arch/x86/platform/efi/efi.c
@@ -108,29 +108,6 @@ static int __init setup_add_efi_memmap(char *arg)
}
early_param("add_efi_memmap", setup_add_efi_memmap);

-void __init efi_find_mirror(void)
-{
- efi_memory_desc_t *md;
- u64 mirror_size = 0, total_size = 0;
-
- if (!efi_enabled(EFI_MEMMAP))
- return;
-
- for_each_efi_memory_desc(md) {
- unsigned long long start = md->phys_addr;
- unsigned long long size = md->num_pages << EFI_PAGE_SHIFT;
-
- total_size += size;
- if (md->attribute & EFI_MEMORY_MORE_RELIABLE) {
- memblock_mark_mirror(start, size);
- mirror_size += size;
- }
- }
- if (mirror_size)
- pr_info("Memory: %lldM/%lldM mirrored memory\n",
- mirror_size>>20, total_size>>20);
-}
-
/*
* Tell the kernel about the EFI memory map. This might include
* more than the max 128 entries that can fit in the passed in e820
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c
index 860534bcfdac..79c232e07de7 100644
--- a/drivers/firmware/efi/efi.c
+++ b/drivers/firmware/efi/efi.c
@@ -446,6 +446,29 @@ static int __init efisubsys_init(void)

subsys_initcall(efisubsys_init);

+void __init efi_find_mirror(void)
+{
+ efi_memory_desc_t *md;
+ u64 mirror_size = 0, total_size = 0;
+
+ if (!efi_enabled(EFI_MEMMAP))
+ return;
+
+ for_each_efi_memory_desc(md) {
+ unsigned long long start = md->phys_addr;
+ unsigned long long size = md->num_pages << EFI_PAGE_SHIFT;
+
+ total_size += size;
+ if (md->attribute & EFI_MEMORY_MORE_RELIABLE) {
+ memblock_mark_mirror(start, size);
+ mirror_size += size;
+ }
+ }
+ if (mirror_size)
+ pr_info("Memory: %lldM/%lldM mirrored memory\n",
+ mirror_size>>20, total_size>>20);
+}
+
/*
* Find the efi memory descriptor for a given physical address. Given a
* physical address, determine if it exists within an EFI Memory Map entry,
diff --git a/include/linux/efi.h b/include/linux/efi.h
index 7d9b0bb47eb3..53f64c14a525 100644
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -872,6 +872,7 @@ static inline bool efi_rt_services_supported(unsigned int mask)
{
return (efi.runtime_supported_mask & mask) == mask;
}
+extern void efi_find_mirror(void);
#else
static inline bool efi_enabled(int feature)
{
@@ -889,6 +890,8 @@ static inline bool efi_rt_services_supported(unsigned int mask)
{
return false;
}
+
+static inline void efi_find_mirror(void) {}
#endif

extern int efi_status_to_err(efi_status_t status);
--
2.25.1

2022-06-13 09:42:49

by mawupeng

[permalink] [raw]
Subject: Re: [PATCH v4 6/6] memblock: Disable mirror feature if kernelcore is not specified



On 6/13/2022 4:21 PM, Wupeng Ma wrote:
> From: Ma Wupeng <[email protected]>
>
> 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 <[email protected]>
> ---
> 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;

2022-06-13 09:49:21

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH v4 6/6] memblock: Disable mirror feature if kernelcore is not specified

On Mon, 13 Jun 2022 at 11:13, mawupeng <[email protected]> wrote:
>
>
>
> On 6/13/2022 4:21 PM, Wupeng Ma wrote:
> > From: Ma Wupeng <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> > 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 <[email protected]>

> >
> > /* movable_zone is the "real" zone pages in ZONE_MOVABLE are taken from */
> > int movable_zone;

2022-06-13 10:03:31

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH v4 2/6] arm64/mirror: arm64 enabling - find mirrored memory ranges

On Mon, 13 Jun 2022 at 10:00, Wupeng Ma <[email protected]> wrote:
>
> From: Ma Wupeng <[email protected]>
>
> Commit b05b9f5f9dcf ("x86, mirror: x86 enabling - find mirrored memory ranges")
> introduced mirrored memory support for x86 and this could be used on arm64.
>
> Since we only support this feature on arm64, efi_find_mirror() won't be placed
> into efi_init(), which is used by riscv/arm/arm64, it is added in setup_arch()
> to scan the memory map and mark mirrored memory in memblock.
>
> Signed-off-by: Ma Wupeng <[email protected]>
> Reviewed-by: Kefeng Wang <[email protected]>
> ---
> Documentation/admin-guide/kernel-parameters.txt | 2 +-
> arch/arm64/kernel/setup.c | 1 +
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 8090130b544b..e3537646b6f7 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -2301,7 +2301,7 @@
>
> keepinitrd [HW,ARM]
>
> - kernelcore= [KNL,X86,IA-64,PPC]
> + kernelcore= [KNL,X86,IA-64,PPC,ARM64]
> Format: nn[KMGTPE] | nn% | "mirror"
> This parameter specifies the amount of memory usable by
> the kernel for non-movable allocations. The requested
> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
> index cf3a759f10d4..6e9acd7ecf0f 100644
> --- a/arch/arm64/kernel/setup.c
> +++ b/arch/arm64/kernel/setup.c
> @@ -328,6 +328,7 @@ void __init __no_sanitize_address setup_arch(char **cmdline_p)
>
> xen_early_init();
> efi_init();
> + efi_find_mirror();
>
> if (!efi_enabled(EFI_BOOT) && ((u64)_text % MIN_KIMG_ALIGN) != 0)
> pr_warn(FW_BUG "Kernel image misaligned at boot, please fix your bootloader!");

As suggested by Kefeng Wang, I think this call needs to be moved into
efi_init() [the generic version]

Please drop this hunk, and add this call to efi_init() in patch #1.

2022-06-13 12:31:37

by Mike Rapoport

[permalink] [raw]
Subject: Re: [PATCH v4 6/6] memblock: Disable mirror feature if kernelcore is not specified

On Mon, Jun 13, 2022 at 04:21:47PM +0800, Wupeng Ma wrote:
> From: Ma Wupeng <[email protected]>
>
> 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 <[email protected]>
> ---
> 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
>

--
Sincerely yours,
Mike.

2022-06-13 15:50:05

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH v4 6/6] memblock: Disable mirror feature if kernelcore is not specified

On Mon, 13 Jun 2022 at 13:05, Mike Rapoport <[email protected]> wrote:
>
> On Mon, Jun 13, 2022 at 04:21:47PM +0800, Wupeng Ma wrote:
> > From: Ma Wupeng <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> > 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.

2022-06-13 15:55:31

by Kefeng Wang

[permalink] [raw]
Subject: Re: [PATCH v4 6/6] memblock: Disable mirror feature if kernelcore is not specified


On 2022/6/13 19:25, Ard Biesheuvel wrote:
> On Mon, 13 Jun 2022 at 13:05, Mike Rapoport <[email protected]> wrote:
>> On Mon, Jun 13, 2022 at 04:21:47PM +0800, Wupeng Ma wrote:
>>> From: Ma Wupeng <[email protected]>
>>>
>>> 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().