On Tue, Mar 01, 2022 at 10:41:10AM +0100, Vlastimil Babka wrote: > On 3/1/22 10:21, Mike Rapoport wrote: > > On Tue, Mar 01, 2022 at 12:38:11AM +0100, Vlastimil Babka wrote: > >> On 2/28/22 21:01, Mike Rapoport wrote: > >> > On Mon, Feb 28, 2022 at 08:10:18PM +0100, Vlastimil Babka wrote: > >> > > >> > If stack_depot_init() is called from kmem_cache_init(), there will be a > >> > confusion what allocator should be used because we use slab_is_available() > >> > to stop using memblock and start using kmalloc() instead in both > >> > stack_depot_init() and in memblock. > >> > >> I did check that stack_depot_init() is called from kmem_cache_init() > >> *before* we make slab_is_available() true, hence assumed that memblock would > >> be still available at that point and expected no confusion. But seems if > >> memblock is already beyond memblock_free_all() then it being still available > >> is just an illusion? > > > > Yeah, it appears it is an illusion :) > > > > I think we have to deal with allocations that happen between > > memblock_free_all() and slab_is_available() at the memblock level and then > > figure out the where to put stack_depot_init() and how to allocate memory > > there. > > > > I believe something like this (untested) patch below addresses the first > > issue. As for stack_depot_init() I'm still trying to figure out the > > possible call paths, but it seems we can use stack_depot_early_init() for > > SLUB debugging case. I'll try to come up with something Really Soon (tm). > > Yeah as you already noticed, we are pursuing an approach to decide on > calling stack_depot_early_init(), which should be a good way to solve this > given how special slab is in this case. For memblock I just wanted to point > out that it could be more robust, your patch below seems to be on the right > patch. Maybe it just doesn't have to fallback to buddy, which could be > considered a layering violation, but just return NULL that can be > immediately recognized as an error? The layering violation is anyway there for slab_is_available() case, so adding a __alloc_pages() there will be only consistent. > > diff --git a/include/linux/memblock.h b/include/linux/memblock.h > > index 50ad19662a32..4ea89d44d22a 100644 > > --- a/include/linux/memblock.h > > +++ b/include/linux/memblock.h > > @@ -90,6 +90,7 @@ struct memblock_type { > > */ > > struct memblock { > > bool bottom_up; /* is bottom up direction? */ > > + bool mem_freed; > > phys_addr_t current_limit; > > struct memblock_type memory; > > struct memblock_type reserved; > > diff --git a/mm/memblock.c b/mm/memblock.c > > index b12a364f2766..60196dc4980e 100644 > > --- a/mm/memblock.c > > +++ b/mm/memblock.c > > @@ -120,6 +120,7 @@ struct memblock memblock __initdata_memblock = { > > .reserved.name = "reserved", > > > > .bottom_up = false, > > + .mem_freed = false, > > .current_limit = MEMBLOCK_ALLOC_ANYWHERE, > > }; > > > > @@ -1487,6 +1488,13 @@ static void * __init memblock_alloc_internal( > > if (WARN_ON_ONCE(slab_is_available())) > > return kzalloc_node(size, GFP_NOWAIT, nid); > > > > + if (memblock.mem_freed) { > > + unsigned int order = get_order(size); > > + > > + pr_warn("memblock: allocating from buddy\n"); > > + return __alloc_pages_node(nid, order, GFP_KERNEL); > > + } > > + > > if (max_addr > memblock.current_limit) > > max_addr = memblock.current_limit; > > > > @@ -2116,6 +2124,7 @@ void __init memblock_free_all(void) > > > > pages = free_low_memory_core_early(); > > totalram_pages_add(pages); > > + memblock.mem_freed = true; > > } > > > > #if defined(CONFIG_DEBUG_FS) && defined(CONFIG_ARCH_KEEP_MEMBLOCK) > > -- Sincerely yours, Mike.