| Message ID | 20240326061134.1055295-2-bhe@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm/init: minor clean up and improvement | expand |
On Tue, Mar 26, 2024 at 02:11:27PM +0800, Baoquan He wrote: > When CONFIG_SPARSEMEM_EXTREME is enabled, mem_section need be initialized > to point at a two-dimensional array, and its 1st dimension of length > NR_SECTION_ROOTS will be dynamically allocated. Once the allocation is > done, it's available for all nodes. > > So take the 1st dimension of mem_section initialization out of > memory_present()(), and put it into memblocks_present() which is a more > appripriate place. > > Signed-off-by: Baoquan He <bhe@redhat.com> > --- > mm/sparse.c | 26 +++++++++++++------------- > 1 file changed, 13 insertions(+), 13 deletions(-) > > diff --git a/mm/sparse.c b/mm/sparse.c > index aed0951b87fa..46e88549d1a6 100644 > --- a/mm/sparse.c > +++ b/mm/sparse.c > @@ -226,19 +226,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) { > @@ -267,6 +254,19 @@ static void __init memblocks_present(void) > unsigned long start, end; > int i, nid; > > +#ifdef CONFIG_SPARSEMEM_EXTREME > + if (unlikely(!mem_section)) { With the new calling sequence sparse_init() -> memblocks_present() -> allocate mem_section, mem_section here will be NULL, so this if is not really relevant. We might want to add WARN_ON_ONCE(mem_section) just in case to catch multiple calls to sparse_init(). > + 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 > + > for_each_mem_pfn_range(i, MAX_NUMNODES, &start, &end, &nid) > memory_present(nid, start, end); > } > -- > 2.41.0 >
On 04/02/24 at 11:19am, Mike Rapoport wrote: > On Tue, Mar 26, 2024 at 02:11:27PM +0800, Baoquan He wrote: > > When CONFIG_SPARSEMEM_EXTREME is enabled, mem_section need be initialized > > to point at a two-dimensional array, and its 1st dimension of length > > NR_SECTION_ROOTS will be dynamically allocated. Once the allocation is > > done, it's available for all nodes. > > > > So take the 1st dimension of mem_section initialization out of > > memory_present()(), and put it into memblocks_present() which is a more > > appripriate place. > > > > Signed-off-by: Baoquan He <bhe@redhat.com> > > --- > > mm/sparse.c | 26 +++++++++++++------------- > > 1 file changed, 13 insertions(+), 13 deletions(-) > > > > diff --git a/mm/sparse.c b/mm/sparse.c > > index aed0951b87fa..46e88549d1a6 100644 > > --- a/mm/sparse.c > > +++ b/mm/sparse.c > > @@ -226,19 +226,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) { > > @@ -267,6 +254,19 @@ static void __init memblocks_present(void) > > unsigned long start, end; > > int i, nid; > > > > +#ifdef CONFIG_SPARSEMEM_EXTREME > > + if (unlikely(!mem_section)) { > > With the new calling sequence sparse_init() -> memblocks_present() -> > allocate mem_section, mem_section here will be NULL, so this if is not > really relevant. We might want to add WARN_ON_ONCE(mem_section) just in > case to catch multiple calls to sparse_init(). Sorry for late reply. Hmm, I am not very eager to add WARN_ON_ONCE(mem_section) becasue as you said it's not very relevant. Calling sparse_init() miltiple times is not acceptable, while it may not be good to let checking mem_section in memblocks_present() to catch it. > > > + 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 > > > + > > for_each_mem_pfn_range(i, MAX_NUMNODES, &start, &end, &nid) > > memory_present(nid, start, end); > > } > > -- > > 2.41.0 > > > > -- > Sincerely yours, > Mike. >
diff --git a/mm/sparse.c b/mm/sparse.c index aed0951b87fa..46e88549d1a6 100644 --- a/mm/sparse.c +++ b/mm/sparse.c @@ -226,19 +226,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) { @@ -267,6 +254,19 @@ static void __init memblocks_present(void) unsigned long start, end; int i, nid; +#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 + for_each_mem_pfn_range(i, MAX_NUMNODES, &start, &end, &nid) memory_present(nid, start, end); }
When CONFIG_SPARSEMEM_EXTREME is enabled, mem_section need be initialized to point at a two-dimensional array, and its 1st dimension of length NR_SECTION_ROOTS will be dynamically allocated. Once the allocation is done, it's available for all nodes. So take the 1st dimension of mem_section initialization out of memory_present()(), and put it into memblocks_present() which is a more appripriate place. Signed-off-by: Baoquan He <bhe@redhat.com> --- mm/sparse.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-)