[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <Z31CF9f//ZD+VH59@li-008a6a4c-3549-11b2-a85c-c5cc2836eea2.ibm.com>
Date: Tue, 7 Jan 2025 16:02:47 +0100
From: Alexander Gordeev <agordeev@...ux.ibm.com>
To: Guo Weikang <guoweikang.kernel@...il.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>, Mike Rapoport <rppt@...nel.org>,
Geert Uytterhoeven <geert@...ux-m68k.org>,
Dennis Zhou <dennis@...nel.org>, Tejun Heo <tj@...nel.org>,
Christoph Lameter <cl@...ux.com>,
Thomas Bogendoerfer <tsbogend@...ha.franken.de>,
Sam Creasey <sammy@...my.net>, Huacai Chen <chenhuacai@...nel.org>,
Will Deacon <will@...nel.org>,
Catalin Marinas <catalin.marinas@....com>,
Oreoluwa Babatunde <quic_obabatun@...cinc.com>,
rafael.j.wysocki@...el.com, Palmer Dabbelt <palmer@...osinc.com>,
Hanjun Guo <guohanjun@...wei.com>,
Easwar Hariharan <eahariha@...ux.microsoft.com>,
Johannes Berg <johannes.berg@...el.com>,
Ingo Molnar <mingo@...nel.org>, Dave Hansen <dave.hansen@...el.com>,
Christian Brauner <brauner@...nel.org>, KP Singh <kpsingh@...nel.org>,
Richard Henderson <richard.henderson@...aro.org>,
Matt Turner <mattst88@...il.com>, Russell King <linux@...linux.org.uk>,
WANG Xuerui <kernel@...0n.name>, Michael Ellerman <mpe@...erman.id.au>,
Stefan Kristiansson <stefan.kristiansson@...nalahti.fi>,
Stafford Horne <shorne@...il.com>, Helge Deller <deller@....de>,
Nicholas Piggin <npiggin@...il.com>,
Christophe Leroy <christophe.leroy@...roup.eu>,
Naveen N Rao <naveen@...nel.org>,
Madhavan Srinivasan <maddy@...ux.ibm.com>,
Geoff Levand <geoff@...radead.org>,
Paul Walmsley <paul.walmsley@...ive.com>,
Palmer Dabbelt <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>,
Andrey Ryabinin <ryabinin.a.a@...il.com>,
Alexander Potapenko <glider@...gle.com>,
Andrey Konovalov <andreyknvl@...il.com>,
Dmitry Vyukov <dvyukov@...gle.com>,
Vincenzo Frascino <vincenzo.frascino@....com>,
Heiko Carstens <hca@...ux.ibm.com>, Vasily Gorbik <gor@...ux.ibm.com>,
Christian Borntraeger <borntraeger@...ux.ibm.com>,
Sven Schnelle <svens@...ux.ibm.com>,
Yoshinori Sato <ysato@...rs.sourceforge.jp>,
Rich Felker <dalias@...c.org>,
John Paul Adrian Glaubitz <glaubitz@...sik.fu-berlin.de>,
Andreas Larsson <andreas@...sler.com>,
Richard Weinberger <richard@....at>,
Anton Ivanov <anton.ivanov@...bridgegreys.com>,
Johannes Berg <johannes@...solutions.net>,
Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
Borislav Petkov <bp@...en8.de>,
Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
linux-alpha@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, loongarch@...ts.linux.dev,
linux-m68k@...ts.linux-m68k.org, linux-mips@...r.kernel.org,
linux-openrisc@...r.kernel.org, linux-parisc@...r.kernel.org,
linuxppc-dev@...ts.ozlabs.org, linux-riscv@...ts.infradead.org,
kasan-dev@...glegroups.com, linux-s390@...r.kernel.org,
linux-sh@...r.kernel.org, sparclinux@...r.kernel.org,
linux-um@...ts.infradead.org, linux-acpi@...r.kernel.org,
xen-devel@...ts.xenproject.org, linux-omap@...r.kernel.org,
linux-clk@...r.kernel.org, devicetree@...r.kernel.org,
linux-mm@...ck.org, linux-pm@...r.kernel.org,
Xi Ruoyao <xry111@...111.site>
Subject: Re: [PATCH v7] mm/memblock: Add memblock_alloc_or_panic interface
On Sun, Dec 22, 2024 at 07:15:37PM +0800, Guo Weikang wrote:
Hi Guo,
> Before SLUB initialization, various subsystems used memblock_alloc to
> allocate memory. In most cases, when memory allocation fails, an immediate
> panic is required. To simplify this behavior and reduce repetitive checks,
> introduce `memblock_alloc_or_panic`. This function ensures that memory
> allocation failures result in a panic automatically, improving code
> readability and consistency across subsystems that require this behavior.
I believe, you also want to make similar function against memblock_alloc_low().
Please, find s390 comments below.
...
> diff --git a/arch/s390/kernel/numa.c b/arch/s390/kernel/numa.c
> index ddc1448ea2e1..a33e20f73330 100644
> --- a/arch/s390/kernel/numa.c
> +++ b/arch/s390/kernel/numa.c
> @@ -22,10 +22,7 @@ void __init numa_setup(void)
> node_set(0, node_possible_map);
> node_set_online(0);
> for (nid = 0; nid < MAX_NUMNODES; nid++) {
> - NODE_DATA(nid) = memblock_alloc(sizeof(pg_data_t), 8);
> - if (!NODE_DATA(nid))
> - panic("%s: Failed to allocate %zu bytes align=0x%x\n",
> - __func__, sizeof(pg_data_t), 8);
> + NODE_DATA(nid) = memblock_alloc_or_panic(sizeof(pg_data_t), 8);
> }
Please, also remove the cycle body brackets.
> NODE_DATA(0)->node_spanned_pages = memblock_end_of_DRAM() >> PAGE_SHIFT;
> NODE_DATA(0)->node_id = 0;
> diff --git a/arch/s390/kernel/setup.c b/arch/s390/kernel/setup.c
> index 0ce550faf073..1298f0860733 100644
> --- a/arch/s390/kernel/setup.c
> +++ b/arch/s390/kernel/setup.c
> @@ -376,11 +376,7 @@ static unsigned long __init stack_alloc_early(void)
> {
> unsigned long stack;
>
> - stack = (unsigned long)memblock_alloc(THREAD_SIZE, THREAD_SIZE);
> - if (!stack) {
> - panic("%s: Failed to allocate %lu bytes align=0x%lx\n",
> - __func__, THREAD_SIZE, THREAD_SIZE);
> - }
> + stack = (unsigned long)memblock_alloc_or_panic(THREAD_SIZE, THREAD_SIZE);
> return stack;
> }
>
> @@ -504,10 +500,7 @@ static void __init setup_resources(void)
> bss_resource.end = __pa_symbol(__bss_stop) - 1;
>
> for_each_mem_range(i, &start, &end) {
> - res = memblock_alloc(sizeof(*res), 8);
> - if (!res)
> - panic("%s: Failed to allocate %zu bytes align=0x%x\n",
> - __func__, sizeof(*res), 8);
> + res = memblock_alloc_or_panic(sizeof(*res), 8);
> res->flags = IORESOURCE_BUSY | IORESOURCE_SYSTEM_RAM;
>
> res->name = "System RAM";
> @@ -526,10 +519,7 @@ static void __init setup_resources(void)
> std_res->start > res->end)
> continue;
> if (std_res->end > res->end) {
> - sub_res = memblock_alloc(sizeof(*sub_res), 8);
> - if (!sub_res)
> - panic("%s: Failed to allocate %zu bytes align=0x%x\n",
> - __func__, sizeof(*sub_res), 8);
> + sub_res = memblock_alloc_or_panic(sizeof(*sub_res), 8);
> *sub_res = *std_res;
> sub_res->end = res->end;
> std_res->start = res->end + 1;
> @@ -816,9 +806,7 @@ static void __init setup_randomness(void)
> {
> struct sysinfo_3_2_2 *vmms;
>
> - vmms = memblock_alloc(PAGE_SIZE, PAGE_SIZE);
> - if (!vmms)
> - panic("Failed to allocate memory for sysinfo structure\n");
> + vmms = memblock_alloc_or_panic(PAGE_SIZE, PAGE_SIZE);
> if (stsi(vmms, 3, 2, 2) == 0 && vmms->count)
> add_device_randomness(&vmms->vm, sizeof(vmms->vm[0]) * vmms->count);
> memblock_free(vmms, PAGE_SIZE);
> diff --git a/arch/s390/kernel/smp.c b/arch/s390/kernel/smp.c
> index 822d8e6f8717..d77aaefb59bd 100644
> --- a/arch/s390/kernel/smp.c
> +++ b/arch/s390/kernel/smp.c
> @@ -611,9 +611,9 @@ void __init smp_save_dump_ipl_cpu(void)
> if (!dump_available())
> return;
> sa = save_area_alloc(true);
> - regs = memblock_alloc(512, 8);
> - if (!sa || !regs)
> + if (!sa)
> panic("could not allocate memory for boot CPU save area\n");
Please, replace memblock_alloc() with memblock_alloc_or_panic() in
save_area_alloc() and remove the error handling here and also in
smp_save_dump_secondary_cpus().
> + regs = memblock_alloc_or_panic(512, 8);
> copy_oldmem_kernel(regs, __LC_FPREGS_SAVE_AREA, 512);
> save_area_add_regs(sa, regs);
> memblock_free(regs, 512);
> @@ -792,10 +792,7 @@ void __init smp_detect_cpus(void)
> u16 address;
>
> /* Get CPU information */
> - info = memblock_alloc(sizeof(*info), 8);
> - if (!info)
> - panic("%s: Failed to allocate %zu bytes align=0x%x\n",
> - __func__, sizeof(*info), 8);
> + info = memblock_alloc_or_panic(sizeof(*info), 8);
> smp_get_core_info(info, 1);
> /* Find boot CPU type */
> if (sclp.has_core_type) {
> diff --git a/arch/s390/kernel/topology.c b/arch/s390/kernel/topology.c
> index 0fd56a1cadbd..cf5ee6032c0b 100644
> --- a/arch/s390/kernel/topology.c
> +++ b/arch/s390/kernel/topology.c
> @@ -548,10 +548,7 @@ static void __init alloc_masks(struct sysinfo_15_1_x *info,
> nr_masks *= info->mag[TOPOLOGY_NR_MAG - offset - 1 - i];
> nr_masks = max(nr_masks, 1);
> for (i = 0; i < nr_masks; i++) {
> - mask->next = memblock_alloc(sizeof(*mask->next), 8);
> - if (!mask->next)
> - panic("%s: Failed to allocate %zu bytes align=0x%x\n",
> - __func__, sizeof(*mask->next), 8);
> + mask->next = memblock_alloc_or_panic(sizeof(*mask->next), 8);
> mask = mask->next;
> }
> }
> @@ -569,10 +566,7 @@ void __init topology_init_early(void)
> }
> if (!MACHINE_HAS_TOPOLOGY)
> goto out;
> - tl_info = memblock_alloc(PAGE_SIZE, PAGE_SIZE);
> - if (!tl_info)
> - panic("%s: Failed to allocate %lu bytes align=0x%lx\n",
> - __func__, PAGE_SIZE, PAGE_SIZE);
> + tl_info = memblock_alloc_or_panic(PAGE_SIZE, PAGE_SIZE);
> info = tl_info;
> store_topology(info);
> pr_info("The CPU configuration topology of the machine is: %d %d %d %d %d %d / %d\n",
Thanks!
Powered by blists - more mailing lists