Hi, Alexander Alexander Gordeev wrote on Tuesday, 7 January 2025 23:02 > > 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(). > About if we need add more interfaces like `or_panic` or `no_panic` , there has been a discussion, temporarily only modify the most frequently used `memblock_alloc`. - https://lore.kernel.org/lkml/CAOm6qnm4pN95kJK8bYfu7Z2Bp_6_Gn35v2WyMmYraQ+YMYQEdg@mail.gmail.com/T/#mf09ea8cb9c5e6cbb9a4b592625394b5639c73e1b > Please, find s390 comments below. > Thanks. Since the patch has been merged into `mm-everything`, I will submit another patch to fix your suggestion. > ... > > > 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. Sure. > > > 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(). OK, I confirmed that all the places where save_area_alloc is called have panic handle. > > > + 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! --- Guo