Hi,Catalin
Catalin Marinas <catalin.marinas@xxxxxxx> wrote on Friday, 3 January 2025 01:15:
>
> On Thu, Jan 02, 2025 at 02:57:03PM +0800, Guo Weikang wrote:
> > diff --git a/include/linux/memblock.h b/include/linux/memblock.h
> > index 673d5cae7c81..b0483c534ef7 100644
> > --- a/include/linux/memblock.h
> > +++ b/include/linux/memblock.h
> > @@ -375,7 +375,13 @@ static inline int memblock_get_region_node(const struct memblock_region *r)
> > }
> > #endif /* CONFIG_NUMA */
> >
> > -/* Flags for memblock allocation APIs */
> > +/*
> > + * Flags for memblock allocation APIs
> > + * MEMBLOCK_ALLOC_ANYWHERE and MEMBLOCK_ALLOC_ACCESSIBLE
> > + * indicates wheather the allocation is limited by memblock.current_limit.
> > + * MEMBLOCK_ALLOC_NOLEAKTRACE not only indicates that it does not need to
> > + * be scanned by kmemleak, but also implies MEMBLOCK_ALLOC_ACCESSIBLE
> > + */
>
> I'd keep the comment short here, something like:
>
> /*
> * MEMBLOCK_ALLOC_NOLEAKTRACE avoids kmemleak tracing. It implies
> * MEMBLOCK_ALLOC_ACCESSIBLE.
> */
>
Thanks, the previous version would be a bit redundant.
> > -/*
> > - * Allocate a block of memory to be used to back the virtual memory map
> > - * or to back the page tables that are used to create the mapping.
> > - * Uses the main allocators if they are available, else bootmem.
> > - */
> > -
> > -static void * __ref __earlyonly_bootmem_alloc(int node,
> > - unsigned long size,
> > - unsigned long align,
> > - unsigned long goal)
> > -{
> > - return memblock_alloc_try_nid_raw(size, align, goal,
> > - MEMBLOCK_ALLOC_ACCESSIBLE, node);
> > -}
> > -
> > void * __meminit vmemmap_alloc_block(unsigned long size, int node)
> > {
> > /* If the main allocator is up use that, fallback to bootmem. */
> > @@ -66,8 +51,7 @@ void * __meminit vmemmap_alloc_block(unsigned long size, int node)
> > }
> > return NULL;
> > } else
> > - return __earlyonly_bootmem_alloc(node, size, size,
> > - __pa(MAX_DMA_ADDRESS));
> > + return memmap_alloc(size, size, __pa(MAX_DMA_ADDRESS), node, false);
> > }
>
> As the kernel test robot reported, the __ref annotation for
> __earlyonly_bootmem_alloc() is still needed, otherwise you get a warning
> that a __meminit function (vmemmap_alloc_block()) is calling an __init
> one (memmap_alloc()). So I think it's better if you keep this function.
> Maybe get it to call memmap_alloc() instead of
> memblock_alloc_try_nid_raw().
>
I have also noticed and studied the previous implementation, it is clear to me
that _ref annotation is used to prevent init section mismatch WARNING.
I will recover __earlyonly_bootmem_alloc().
> --
> Catalin
Thanks a lot
--
Guo
