| Message ID | 20210517112044.233138-4-aisheng.dong@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mm/sparse: a few minor fixes and improvements | expand |
On Mon, May 17, 2021 at 07:20:42PM +0800, 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> > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com> > --- > mm/sparse.c | 23 ++++++++++------------- > 1 file changed, 10 insertions(+), 13 deletions(-) > > diff --git a/mm/sparse.c b/mm/sparse.c > index 98bfacc763da..df4418c12f04 100644 > --- a/mm/sparse.c > +++ b/mm/sparse.c > @@ -254,19 +254,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 Maybe split this into a helper function and call it directly from sparse_init()? > - > start &= PAGE_SECTION_MASK; > mminit_validate_memmodel_limits(&start, &end); > for (pfn = start; pfn < end; pfn += PAGES_PER_SECTION) { > @@ -292,9 +279,19 @@ static void __init memory_present(int nid, unsigned long start, unsigned long en > */ > static void __init memblocks_present(void) > { > + unsigned long __maybe_unused size, align; > unsigned long start, end; > int i, nid; > > +#ifdef CONFIG_SPARSEMEM_EXTREME > + 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 > + > for_each_mem_pfn_range(i, MAX_NUMNODES, &start, &end, &nid) > memory_present(nid, start, end); > } > -- > 2.25.1 > >
On Tue, May 18, 2021 at 6:12 PM Mike Rapoport <rppt@kernel.org> wrote: > > On Mon, May 17, 2021 at 07:20:42PM +0800, 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> > > Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com> > > --- > > mm/sparse.c | 23 ++++++++++------------- > > 1 file changed, 10 insertions(+), 13 deletions(-) > > > > diff --git a/mm/sparse.c b/mm/sparse.c > > index 98bfacc763da..df4418c12f04 100644 > > --- a/mm/sparse.c > > +++ b/mm/sparse.c > > @@ -254,19 +254,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 > > Maybe split this into a helper function and call it directly from > sparse_init()? > Sounds good to me e.g. sparse_alloc_section_root() called directly from sparse_init(). Thanks for the suggestion Regards Aisheng > > - > > start &= PAGE_SECTION_MASK; > > mminit_validate_memmodel_limits(&start, &end); > > for (pfn = start; pfn < end; pfn += PAGES_PER_SECTION) { > > @@ -292,9 +279,19 @@ static void __init memory_present(int nid, unsigned long start, unsigned long en > > */ > > static void __init memblocks_present(void) > > { > > + unsigned long __maybe_unused size, align; > > unsigned long start, end; > > int i, nid; > > > > +#ifdef CONFIG_SPARSEMEM_EXTREME > > + 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 > > + > > for_each_mem_pfn_range(i, MAX_NUMNODES, &start, &end, &nid) > > memory_present(nid, start, end); > > } > > -- > > 2.25.1 > > > > > > -- > Sincerely yours, > Mike.
diff --git a/mm/sparse.c b/mm/sparse.c index 98bfacc763da..df4418c12f04 100644 --- a/mm/sparse.c +++ b/mm/sparse.c @@ -254,19 +254,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) { @@ -292,9 +279,19 @@ static void __init memory_present(int nid, unsigned long start, unsigned long en */ static void __init memblocks_present(void) { + unsigned long __maybe_unused size, align; unsigned long start, end; int i, nid; +#ifdef CONFIG_SPARSEMEM_EXTREME + 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 + for_each_mem_pfn_range(i, MAX_NUMNODES, &start, &end, &nid) memory_present(nid, start, end); }
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> Signed-off-by: Dong Aisheng <aisheng.dong@nxp.com> --- mm/sparse.c | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-)