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@xxxxxxxxxx>
>>>>>
>>>>> 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@xxxxxxxxxx>
>>>>> ---
>>>>> 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.
