On 5/27/22 14:26, Zhouguanghui (OS Kernel) wrote: > Hi Anshuman, > > 在 2022/5/18 12:40, Anshuman Khandual 写道: >> Hi Zhou, >> >> A small nit. >> >> This changes generic memblock to accommodate arm64 specific scenario. >> Keeping the subject line as 'mm/memblock: ...' might be better. >> > > I will add memblock to the subject line. > >> On 5/17/22 17:13, Zhou Guanghui wrote: >>> In a system using HBM, a multi-bit ECC error occurs, and the BIOS >>> saves the corresponding area (for example, 2 MB). When the system >>> restarts next time, these areas are isolated and not reported or >>> reported as EFI_UNUSABLE_MEMORY. Both of them lead to an increase >> >> Which cases dont get reported rather than as EFI_UNUSABLE_MEMORY ? Is >> this supported on arm64 platform via mainline kernel ? >> > > The BIOS determines how to report the memory area that cannot be used to > the kernel. Do not report the memory area to the kernel or inform the > kernel that the memory area is unusable. Right, but just curious whether there are real systems in the field with this feature running mainline kernel ? OR this is just being future proof. > >>> in the number of memblocks, whereas EFI_UNUSABLE_MEMORY leads to >>> a larger number of memblocks. >>> >>> For example, if the EFI_UNUSABLE_MEMORY type is reported: >>> ... >>> memory[0x92] [0x0000200834a00000-0x0000200835bfffff], 0x0000000001200000 bytes on node 7 flags: 0x0 >>> memory[0x93] [0x0000200835c00000-0x0000200835dfffff], 0x0000000000200000 bytes on node 7 flags: 0x4 >>> memory[0x94] [0x0000200835e00000-0x00002008367fffff], 0x0000000000a00000 bytes on node 7 flags: 0x0 >>> memory[0x95] [0x0000200836800000-0x00002008369fffff], 0x0000000000200000 bytes on node 7 flags: 0x4 >>> memory[0x96] [0x0000200836a00000-0x0000200837bfffff], 0x0000000001200000 bytes on node 7 flags: 0x0 >>> memory[0x97] [0x0000200837c00000-0x0000200837dfffff], 0x0000000000200000 bytes on node 7 flags: 0x4 >>> memory[0x98] [0x0000200837e00000-0x000020087fffffff], 0x0000000048200000 bytes on node 7 flags: 0x0 >>> memory[0x99] [0x0000200880000000-0x0000200bcfffffff], 0x0000000350000000 bytes on node 6 flags: 0x0 >>> memory[0x9a] [0x0000200bd0000000-0x0000200bd01fffff], 0x0000000000200000 bytes on node 6 flags: 0x4 >>> memory[0x9b] [0x0000200bd0200000-0x0000200bd07fffff], 0x0000000000600000 bytes on node 6 flags: 0x0 >>> memory[0x9c] [0x0000200bd0800000-0x0000200bd09fffff], 0x0000000000200000 bytes on node 6 flags: 0x4 >>> memory[0x9d] [0x0000200bd0a00000-0x0000200fcfffffff], 0x00000003ff600000 bytes on node 6 flags: 0x0 >>> memory[0x9e] [0x0000200fd0000000-0x0000200fd01fffff], 0x0000000000200000 bytes on node 6 flags: 0x4 >>> memory[0x9f] [0x0000200fd0200000-0x0000200fffffffff], 0x000000002fe00000 bytes on node 6 flags: 0x0 >> >> Got it. >> >>> ... >>> >>> If the size of the init memblock regions is exceeded before the >>> array size can be resized, the excess memory will be lost. >> >> Could you please elaborate more on why additional memblock regions can >> not be accommodated via memblock array resizing ? >> > > As described in the memblock_double_array function: We don't allow > resizing until we know about the reserved regions of memory that aren' > not suitable for allocation. > >>> >>> Signed-off-by: Zhou Guanghui <zhouguanghui1@xxxxxxxxxx> >>> --- >>> arch/arm64/include/asm/memory.h | 9 +++++++++ >>> mm/memblock.c | 14 +++++++++----- >>> 2 files changed, 18 insertions(+), 5 deletions(-) >>> >>> diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h >>> index 0af70d9abede..eda61c0389c4 100644 >>> --- a/arch/arm64/include/asm/memory.h >>> +++ b/arch/arm64/include/asm/memory.h >>> @@ -364,6 +364,15 @@ void dump_mem_limit(void); >>> # define INIT_MEMBLOCK_RESERVED_REGIONS (INIT_MEMBLOCK_REGIONS + NR_CPUS + 1) >>> #endif >>> >>> +/* >>> + * memory regions which marked with flag MEMBLOCK_NOMAP may divide a continuous >>> + * memory block into multiple parts. As a result, the number of memory regions >>> + * is large. >>> + */ >>> +#ifdef CONFIG_EFI >> >> Could not memblock regions tagged with MEMBLOCK_NOMAP flag not present >> on non-EFI systems ? Just wondering, are there not some other scenarios >> which will also require expanded static memblock array. > > Systems using devicetree can also have "no-map" memory. However, in this > case, the expanded static memblock array is required only when a large > number of such no-map reserved memories are manually added. I don't know > if any users will do that. > > Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml > > As to whether other scenarios also require expanded static memblock > arrays, I really don't know. In that case could this comment here be more specific about this increased static array size, being applicable only for MEMBLOCK_NOMAP regions on EFI system with EFI_UNUSABLE_MEMORY tagging support. Is there an way to narrow this down further wrt EFI_UNUSABLE_MEMORY, rather than blanket EFI ? +/* + * memory regions which marked with flag MEMBLOCK_NOMAP may divide a continuous + * memory block into multiple parts. As a result, the number of memory regions + * is large. + */ +#ifdef CONFIG_EFI +#define INIT_MEMBLOCK_MEMORY_REGIONS 1024 +#endif + > >> >>> +#define INIT_MEMBLOCK_MEMORY_REGIONS 1024 >>> +#endif >>> + >>> #include <asm-generic/memory_model.h> >>> >>> #endif /* __ASM_MEMORY_H */ >>> diff --git a/mm/memblock.c b/mm/memblock.c >>> index e4f03a6e8e56..7c63571a69d7 100644 >>> --- a/mm/memblock.c >>> +++ b/mm/memblock.c >>> @@ -29,6 +29,10 @@ >>> # define INIT_MEMBLOCK_RESERVED_REGIONS INIT_MEMBLOCK_REGIONS >>> #endif >>> >>> +#ifndef INIT_MEMBLOCK_MEMORY_REGIONS >>> +#define INIT_MEMBLOCK_MEMORY_REGIONS INIT_MEMBLOCK_REGIONS >>> +#endif >> >> Why create an additional macro INIT_MEMBLOCK_MEMORY_REGIONS ? Why cannot >> INIT_MEMBLOCK_REGIONS be defined in the platform directly like the other >> macro INIT_MEMBLOCK_RESERVED_REGIONS ? >> > > The number of reserved memblocks does not need to be increased. Got it.