| Message ID | 20210531091908.1738465-4-aisheng.dong@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mm/sparse: a few minor fixes and improvements | expand |
* 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 > + > memblocks_present(); > > pnum_begin = first_present_section_nr(); > -- > 2.25.1 > >
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 > > > >
On 31.05.21 11:19, Dong Aisheng wrote: > 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(); > + > memblocks_present(); > > pnum_begin = first_present_section_nr(); > Acked-by: David Hildenbrand <david@redhat.com>
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(); + memblocks_present(); pnum_begin = first_present_section_nr();
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(-)