Re: [PATCH V2 3/6] mm/sparse: move mem_sections allocation out of memory_present()

From: Dong Aisheng <dongas86@gmail.com>
To: Liam Howlett <liam.howlett@oracle.com>
Cc: Dong Aisheng <aisheng.dong@nxp.com>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Mike Rapoport <rppt@kernel.org>
Subject: Re: [PATCH V2 3/6] mm/sparse: move mem_sections allocation out of memory_present()
Date: Tue, 1 Jun 2021 10:38:33 +0800	[thread overview]
Message-ID: <CAA+hA=Q0d14zkqibpNGWHaMTgFqot8YX71SHfnS4VTV3vMyXnw@mail.gmail.com> (raw)
In-Reply-To: <20210531143852.h226vpjrgpyo2eqw@revolver>

On Mon, May 31, 2021 at 10:39 PM Liam Howlett <liam.howlett@oracle.com> wrote:
>
> * Dong Aisheng <aisheng.dong@nxp.com> [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@linux-foundation.org>
> > Cc: Mike Rapoport <rppt@kernel.org>
> > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com>
> > ---
> > 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
> >
> >