| Message ID | 1646108941-27919-2-git-send-email-yangtiezhu@loongson.cn (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | MIPS: Modify mem= and memmap= parameter | expand |
On Tue, Mar 01, 2022 at 12:28:58PM +0800, Tiezhu Yang wrote: > According to Documentation/admin-guide/kernel-parameters.txt, > the kernel command-line parameter mem= means "Force usage of > a specific amount of memory", but when add "mem=3G" to the > command-line, kernel boot hangs in sparse_init(). > > This commit is similar with the implementation of the other > archs such as arm64, powerpc and riscv, refactor the function > early_parse_mem() and then use memblock_enforce_memory_limit() > to limit the memory size. > > With this patch, when add "mem=3G" to the command-line, the > kernel boots successfully, we can see the following messages: unfortunately this patch would break platforms without memory detection, which simply use mem=32M for memory configuration. Not sure how many rely on this mechanism. If we can make sure nobody uses it, I'm fine with your patch. Thomas.
On Fri, Mar 04, 2022 at 04:10:52PM +0100, Thomas Bogendoerfer wrote: > On Tue, Mar 01, 2022 at 12:28:58PM +0800, Tiezhu Yang wrote: > > According to Documentation/admin-guide/kernel-parameters.txt, > > the kernel command-line parameter mem= means "Force usage of > > a specific amount of memory", but when add "mem=3G" to the > > command-line, kernel boot hangs in sparse_init(). > > > > This commit is similar with the implementation of the other > > archs such as arm64, powerpc and riscv, refactor the function > > early_parse_mem() and then use memblock_enforce_memory_limit() > > to limit the memory size. > > > > With this patch, when add "mem=3G" to the command-line, the > > kernel boots successfully, we can see the following messages: > > unfortunately this patch would break platforms without memory detection, > which simply use mem=32M for memory configuration. Not sure how many > rely on this mechanism. If we can make sure nobody uses it, I'm fine > with your patch. maybe we could add a CONFIG option, which will be selected by platforms, which don't need/want this usermem thing. Thomas.
On Fri, 4 Mar 2022, Thomas Bogendoerfer wrote: > > > With this patch, when add "mem=3G" to the command-line, the > > > kernel boots successfully, we can see the following messages: > > > > unfortunately this patch would break platforms without memory detection, > > which simply use mem=32M for memory configuration. Not sure how many > > rely on this mechanism. If we can make sure nobody uses it, I'm fine > > with your patch. > > maybe we could add a CONFIG option, which will be selected by > platforms, which don't need/want this usermem thing. FWIW I don't understand what the issue is here beyond that we have a bug that causes a system to hang when "mem=3G" is passed on the kernel command line. That is assuming that system does have contiguous RAM available for the kernel to use from address 0 up to 3GiB; otherwise it's a user error to tell the kernel it has that memory available (I did get bitten by that myself too): garbage in, garbage out. I think having a CONFIG option automatically selected to disable the ability to give a memory map override would handicap people in debugging their systems or working around firmware bugs, so I would rather be against it. Maciej
On Fri, Mar 04, 2022 at 05:11:44PM +0000, Maciej W. Rozycki wrote: > On Fri, 4 Mar 2022, Thomas Bogendoerfer wrote: > > > > > With this patch, when add "mem=3G" to the command-line, the > > > > kernel boots successfully, we can see the following messages: > > > > > > unfortunately this patch would break platforms without memory detection, > > > which simply use mem=32M for memory configuration. Not sure how many > > > rely on this mechanism. If we can make sure nobody uses it, I'm fine > > > with your patch. > > > > maybe we could add a CONFIG option, which will be selected by > > platforms, which don't need/want this usermem thing. > > FWIW I don't understand what the issue is here beyond that we have a bug > that causes a system to hang when "mem=3G" is passed on the kernel command > line. That is assuming that system does have contiguous RAM available for > the kernel to use from address 0 up to 3GiB; otherwise it's a user error > to tell the kernel it has that memory available (I did get bitten by that > myself too): garbage in, garbage out. This is exactly the case here because the system does not have contiguous RAM: [ 0.000000] Early memory node ranges [ 0.000000] node 0: [mem 0x0000000004000000-0x000000000effffff] [ 0.000000] node 0: [mem 0x0000000090200000-0x00000000ffffffff] [ 0.000000] node 0: [mem 0x0000000120000000-0x00000001653fffff] (from patch 3/4 in this series) I don't see what "bug" this patch is trying to fix. Is indeed possible to make MIPS' mem= behave like it does not arm64 and ppc, but that would break systems that use current semantics and I recall seeing some of OpenWRT machines using mem= to override memory map supplied by firmware. > Maciej
On Fri, Mar 04, 2022 at 05:11:44PM +0000, Maciej W. Rozycki wrote: > On Fri, 4 Mar 2022, Thomas Bogendoerfer wrote: > > > > > With this patch, when add "mem=3G" to the command-line, the > > > > kernel boots successfully, we can see the following messages: > > > > > > unfortunately this patch would break platforms without memory detection, > > > which simply use mem=32M for memory configuration. Not sure how many > > > rely on this mechanism. If we can make sure nobody uses it, I'm fine > > > with your patch. > > > > maybe we could add a CONFIG option, which will be selected by > > platforms, which don't need/want this usermem thing. > > FWIW I don't understand what the issue is here beyond that we have a bug > that causes a system to hang when "mem=3G" is passed on the kernel command > line. That is assuming that system does have contiguous RAM available for > the kernel to use from address 0 up to 3GiB; otherwise it's a user error > to tell the kernel it has that memory available (I did get bitten by that > myself too): garbage in, garbage out. I did a quick test with an IP30: >> bootp(): ip=dhcp root=/dev/nfs console=ttyS0 mem=384M Setting $netaddr to 192.168.8.208 (from server ) Obtaining from server 9012640+181664 entry: 0xa800000020664a60 Linux version 5.17.0-rc3+ (tbogendoerfer@adalid) (mips64-linux-gnu-gcc (GCC) 6.1.1 20160621 (Red Hat Cross 6.1.1-2), GNU ld version 2.27-3.fc24) #155 SMP Mon Mar 7 13:12:01 CET 2022 ARCH: SGI-IP30 PROMLIB: ARC firmware Version 64 Revision 0 printk: bootconsole [early0] enabled CPU0 revision is: 00000934 (R10000) FPU revision is: 00000900 Detected 512MB of physical memory. User-defined physical RAM map overwrite Kernel sections are not in the memory maps IP30: Slot: 0, PrID: 00000934, PhyID: 0, VirtID: 0 IP30: Slot: 1, PrID: 00000934, PhyID: 1, VirtID: 1 IP30: Detected 2 CPU(s) present. Primary instruction cache 32kB, VIPT, 2-way, linesize 64 bytes. Primary data cache 32kB, 2-way, VIPT, no aliases, linesize 32 bytes Unified secondary cache 1024kB 2-way, linesize 128 bytes. Zone ranges: DMA32 [mem 0x0000000000000000-0x00000000ffffffff] Normal empty Movable zone start for each node Early memory node ranges node 0: [mem 0x0000000000000000-0x0000000017ffffff] node 0: [mem 0x0000000020004000-0x00000000208c7fff] Initmem setup node 0 [mem 0x0000000000000000-0x00000000208c7fff] after that it's dead (it doesn't have memory starting at 0x0). Most SGI systems will act broken with mem= in one way or another. And I already had the need to limit the amount of memory. > I think having a CONFIG option automatically selected to disable the > ability to give a memory map override would handicap people in debugging > their systems or working around firmware bugs, so I would rather be > against it. I'm thinking about a CONFIG option, which isn't user selectable, but selected via Kconfig only. But that would give to differents semantics for mem= So can I just limit amount of memory without interfering with normal memory detection ? Thomas.
On Mon, Mar 07, 2022 at 05:29:09PM +0100, Thomas Bogendoerfer wrote: > On Fri, Mar 04, 2022 at 05:11:44PM +0000, Maciej W. Rozycki wrote: > > On Fri, 4 Mar 2022, Thomas Bogendoerfer wrote: > > > > > > > With this patch, when add "mem=3G" to the command-line, the > > > > > kernel boots successfully, we can see the following messages: > > > > > > > > unfortunately this patch would break platforms without memory detection, > > > > which simply use mem=32M for memory configuration. Not sure how many > > > > rely on this mechanism. If we can make sure nobody uses it, I'm fine > > > > with your patch. > > > > > > maybe we could add a CONFIG option, which will be selected by > > > platforms, which don't need/want this usermem thing. > > > > FWIW I don't understand what the issue is here beyond that we have a bug > > that causes a system to hang when "mem=3G" is passed on the kernel command > > line. That is assuming that system does have contiguous RAM available for > > the kernel to use from address 0 up to 3GiB; otherwise it's a user error > > to tell the kernel it has that memory available (I did get bitten by that > > myself too): garbage in, garbage out. > > I did a quick test with an IP30: > > >> bootp(): ip=dhcp root=/dev/nfs console=ttyS0 mem=384M > Setting $netaddr to 192.168.8.208 (from server ) > Obtaining from server > 9012640+181664 entry: 0xa800000020664a60 > Linux version 5.17.0-rc3+ (tbogendoerfer@adalid) (mips64-linux-gnu-gcc (GCC) 6.1.1 20160621 (Red Hat Cross 6.1.1-2), GNU ld version 2.27-3.fc24) #155 SMP Mon Mar 7 13:12:01 CET 2022 > ARCH: SGI-IP30 > PROMLIB: ARC firmware Version 64 Revision 0 > printk: bootconsole [early0] enabled > CPU0 revision is: 00000934 (R10000) > FPU revision is: 00000900 > Detected 512MB of physical memory. > User-defined physical RAM map overwrite > Kernel sections are not in the memory maps > IP30: Slot: 0, PrID: 00000934, PhyID: 0, VirtID: 0 > IP30: Slot: 1, PrID: 00000934, PhyID: 1, VirtID: 1 > IP30: Detected 2 CPU(s) present. > Primary instruction cache 32kB, VIPT, 2-way, linesize 64 bytes. > Primary data cache 32kB, 2-way, VIPT, no aliases, linesize 32 bytes > Unified secondary cache 1024kB 2-way, linesize 128 bytes. > Zone ranges: > DMA32 [mem 0x0000000000000000-0x00000000ffffffff] > Normal empty > Movable zone start for each node > Early memory node ranges > node 0: [mem 0x0000000000000000-0x0000000017ffffff] > node 0: [mem 0x0000000020004000-0x00000000208c7fff] > Initmem setup node 0 [mem 0x0000000000000000-0x00000000208c7fff] > > after that it's dead (it doesn't have memory starting at 0x0). > Most SGI systems will act broken with mem= in one way or another. > And I already had the need to limit the amount of memory. > > > I think having a CONFIG option automatically selected to disable the > > ability to give a memory map override would handicap people in debugging > > their systems or working around firmware bugs, so I would rather be > > against it. > > I'm thinking about a CONFIG option, which isn't user selectable, but > selected via Kconfig only. But that would give to differents semantics > for mem= > > So can I just limit amount of memory without interfering with normal > memory detection ? Maybe it's better to add a new encoding to mem= that will have the semantics of limiting amount of memory? E.g. mem=384M@ would mean "only use 384M of memory that firmware reported" while mem=384M would mean "set memory to 0 - 384M" as it does now. I think it's fine to have this MIPS specific because there is anyway no consistency among architectures in mem= handling. > Thomas. > > -- > Crap can work. Given enough thrust pigs will fly, but it's not necessarily a > good idea. [ RFC1925, 2.3 ]
On Tue, 8 Mar 2022, Mike Rapoport wrote: > > So can I just limit amount of memory without interfering with normal > > memory detection ? > > Maybe it's better to add a new encoding to mem= that will have the semantics > of limiting amount of memory? > > E.g. > > mem=384M@ > > would mean "only use 384M of memory that firmware reported" while > > mem=384M would mean "set memory to 0 - 384M" as it does now. I think you're going in the right direction, we'd just need to sort out the most reasonable syntax for the new semantics; `mem=384M@' just seems too analogous to me to `mem=384M@0'. Maybe `mem=384M-'? NB that would have to work with the existing overrides, for e.g.: `mem=192M@0 mem=192M@256M mem=384M-' to produce the following memory ranges available for use: Normal [mem 0x0000000000000000-0x000000000bffffff] Normal [mem 0x0000000010000000-0x0000000017ffffff] (so that you can paste the final cap at some command prompt and still have earlier parameters respected that may have been passed by the firmware or bootloader, or built in). Maciej
diff --git a/arch/mips/kernel/setup.c b/arch/mips/kernel/setup.c index f979adf..50396ba 100644 --- a/arch/mips/kernel/setup.c +++ b/arch/mips/kernel/setup.c @@ -339,27 +339,15 @@ static void __init bootmem_init(void) #endif /* CONFIG_SGI_IP27 */ static int usermem __initdata; +static phys_addr_t memory_limit; static int __init early_parse_mem(char *p) { - phys_addr_t start, size; - - /* - * If a user specifies memory size, we - * blow away any automatically generated - * size. - */ - if (usermem == 0) { - usermem = 1; - memblock_remove(memblock_start_of_DRAM(), - memblock_end_of_DRAM() - memblock_start_of_DRAM()); - } - start = 0; - size = memparse(p, &p); - if (*p == '@') - start = memparse(p + 1, &p); + if (!p) + return 1; - memblock_add(start, size); + memory_limit = memparse(p, &p) & PAGE_MASK; + pr_notice("Memory limited to %lluMB\n", (u64)memory_limit >> 20); return 0; } @@ -678,6 +666,10 @@ static void __init arch_mem_init(char **cmdline_p) memblock_reserve(__pa_symbol(&__nosave_begin), __pa_symbol(&__nosave_end) - __pa_symbol(&__nosave_begin)); + /* Limit the memory. */ + memblock_enforce_memory_limit(memory_limit); + memblock_allow_resize(); + early_memtest(PFN_PHYS(ARCH_PFN_OFFSET), PFN_PHYS(max_low_pfn)); }
According to Documentation/admin-guide/kernel-parameters.txt, the kernel command-line parameter mem= means "Force usage of a specific amount of memory", but when add "mem=3G" to the command-line, kernel boot hangs in sparse_init(). This commit is similar with the implementation of the other archs such as arm64, powerpc and riscv, refactor the function early_parse_mem() and then use memblock_enforce_memory_limit() to limit the memory size. With this patch, when add "mem=3G" to the command-line, the kernel boots successfully, we can see the following messages: [ 0.000000] Memory limited to 3072MB ... [ 0.000000] Early memory node ranges [ 0.000000] node 0: [mem 0x0000000000200000-0x000000000effffff] [ 0.000000] node 0: [mem 0x0000000090200000-0x00000000ffffffff] [ 0.000000] node 0: [mem 0x0000000120000000-0x00000001613fffff] ... [ 0.000000] Memory: 3005280K/3145728K available (...) After login, the output of free command is consistent with the above log. Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn> --- arch/mips/kernel/setup.c | 26 +++++++++----------------- 1 file changed, 9 insertions(+), 17 deletions(-)