On Mon, May 31, 2021 at 10:39 PM Liam Howlett <liam.howlett@xxxxxxxxxx> wrote:
>
> * Dong Aisheng <aisheng.dong@xxxxxxx> [210531 05:20]:
> > The only path to call memory_present() is from memblocks_present().
> > The struct mem_section **mem_section only needs to be initialized once,
> > so no need put the initialization/allocation code in memory_present()
> > which will be called multiple times for each section.
> >
> > After moving, the 'unlikely' condition statement becomes to be
> > meaningless as it's only initialized one time, so dropped as well.
> >
> > Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> > Cc: Mike Rapoport <rppt@xxxxxxxxxx>
> > Signed-off-by: Dong Aisheng <aisheng.dong@xxxxxxx>
> > ---
> > ChangeLog:
> > v1->v2:
> > * split into a helper function and called directly from sparse_init
> > ---
> > mm/sparse.c | 29 ++++++++++++++++-------------
> > 1 file changed, 16 insertions(+), 13 deletions(-)
> >
> > diff --git a/mm/sparse.c b/mm/sparse.c
> > index 408b737e168e..d02ee6bb7cbc 100644
> > --- a/mm/sparse.c
> > +++ b/mm/sparse.c
> > @@ -60,6 +60,18 @@ static inline void set_section_nid(unsigned long section_nr, int nid)
> > #endif
> >
> > #ifdef CONFIG_SPARSEMEM_EXTREME
> > +static void __init sparse_alloc_section_roots(void)
> > +{
> > + unsigned long size, align;
> > +
> > + size = sizeof(struct mem_section *) * NR_SECTION_ROOTS;
> > + align = 1 << (INTERNODE_CACHE_SHIFT);
> > + mem_section = memblock_alloc(size, align);
> > + if (!mem_section)
> > + panic("%s: Failed to allocate %lu bytes align=0x%lx\n",
> > + __func__, size, align);
> > +}
> > +
> > static noinline struct mem_section __ref *sparse_index_alloc(int nid)
> > {
> > struct mem_section *section = NULL;
> > @@ -107,6 +119,8 @@ static inline int sparse_index_init(unsigned long section_nr, int nid)
> > {
> > return 0;
> > }
> > +
> > +static inline void sparse_alloc_section_roots(void) {}
> > #endif
> >
> > #ifdef CONFIG_SPARSEMEM_EXTREME
> > @@ -254,19 +268,6 @@ static void __init memory_present(int nid, unsigned long start, unsigned long en
> > {
> > unsigned long pfn;
> >
> > -#ifdef CONFIG_SPARSEMEM_EXTREME
> > - if (unlikely(!mem_section)) {
> > - unsigned long size, align;
> > -
> > - size = sizeof(struct mem_section *) * NR_SECTION_ROOTS;
> > - align = 1 << (INTERNODE_CACHE_SHIFT);
> > - mem_section = memblock_alloc(size, align);
> > - if (!mem_section)
> > - panic("%s: Failed to allocate %lu bytes align=0x%lx\n",
> > - __func__, size, align);
> > - }
> > -#endif
> > -
> > start &= PAGE_SECTION_MASK;
> > mminit_validate_memmodel_limits(&start, &end);
> > for (pfn = start; pfn < end; pfn += PAGES_PER_SECTION) {
> > @@ -582,6 +583,8 @@ void __init sparse_init(void)
> > unsigned long pnum_end, pnum_begin, map_count = 1;
> > int nid_begin;
> >
> > + sparse_alloc_section_roots();
>
> nit: The newline below is unnecessary
>
Can drop it , thanks
Regards
AIsheng
> > +
> > memblocks_present();
> >
> > pnum_begin = first_present_section_nr();
> > --
> > 2.25.1
> >
> >
