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.
Although there could be just not enough mirrored memory even EFI reports, it may
be better to avoid this kind of oom if there is something with with EFI mirror or
this system is boot with legacy BIOS without the support of 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.
Yes, you are right.
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
>>>>
>>>
>
