| Message ID | 20181122094807.6985-1-richard.weiyang@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mm: check nr_initialised with PAGES_PER_SECTION directly in defer_init() | expand |
Could someone please review this? From: Wei Yang <richard.weiyang@gmail.com> Subject: mm: check nr_initialised with PAGES_PER_SECTION directly in defer_init() When DEFERRED_STRUCT_PAGE_INIT is configured, only the first section of each node's highest zone is initialized before defer stage. static_init_pgcnt is used to store the number of pages like this: pgdat->static_init_pgcnt = min_t(unsigned long, PAGES_PER_SECTION, pgdat->node_spanned_pages); because we don't want to overflow zone's range. But this is not necessary, since defer_init() is called like this: memmap_init_zone() for pfn in [start_pfn, end_pfn) defer_init(pfn, end_pfn) In case (pgdat->node_spanned_pages < PAGES_PER_SECTION), the loop would stop before calling defer_init(). BTW, comparing PAGES_PER_SECTION with node_spanned_pages is not correct, since nr_initialised is zone based instead of node based. Even node_spanned_pages is bigger than PAGES_PER_SECTION, its highest zone would have pages less than PAGES_PER_SECTION. Link: http://lkml.kernel.org/r/20181122094807.6985-1-richard.weiyang@gmail.com Signed-off-by: Wei Yang <richard.weiyang@gmail.com> Cc: Pavel Tatashin <pasha.tatashin@oracle.com> Cc: Oscar Salvador <osalvador@suse.de> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> --- include/linux/mmzone.h | 2 -- mm/page_alloc.c | 13 ++++++------- 2 files changed, 6 insertions(+), 9 deletions(-) --- a/include/linux/mmzone.h~mm-check-nr_initialised-with-pages_per_section-directly-in-defer_init +++ a/include/linux/mmzone.h @@ -692,8 +692,6 @@ typedef struct pglist_data { * is the first PFN that needs to be initialised. */ unsigned long first_deferred_pfn; - /* Number of non-deferred pages */ - unsigned long static_init_pgcnt; #endif /* CONFIG_DEFERRED_STRUCT_PAGE_INIT */ #ifdef CONFIG_TRANSPARENT_HUGEPAGE --- a/mm/page_alloc.c~mm-check-nr_initialised-with-pages_per_section-directly-in-defer_init +++ a/mm/page_alloc.c @@ -326,8 +326,13 @@ defer_init(int nid, unsigned long pfn, u /* Always populate low zones for address-constrained allocations */ if (end_pfn < pgdat_end_pfn(NODE_DATA(nid))) return false; + + /* + * We start only with one section of pages, more pages are added as + * needed until the rest of deferred pages are initialized. + */ nr_initialised++; - if ((nr_initialised > NODE_DATA(nid)->static_init_pgcnt) && + if ((nr_initialised > PAGES_PER_SECTION) && (pfn & (PAGES_PER_SECTION - 1)) == 0) { NODE_DATA(nid)->first_deferred_pfn = pfn; return true; @@ -6585,12 +6590,6 @@ static void __ref alloc_node_mem_map(str #ifdef CONFIG_DEFERRED_STRUCT_PAGE_INIT static inline void pgdat_set_deferred_range(pg_data_t *pgdat) { - /* - * We start only with one section of pages, more pages are added as - * needed until the rest of deferred pages are initialized. - */ - pgdat->static_init_pgcnt = min_t(unsigned long, PAGES_PER_SECTION, - pgdat->node_spanned_pages); pgdat->first_deferred_pfn = ULONG_MAX; } #else
On Thu, 2018-11-22 at 17:48 +0800, Wei Yang wrote: > When DEFERRED_STRUCT_PAGE_INIT is configured, only the first section of > each node's highest zone is initialized before defer stage. > > static_init_pgcnt is used to store the number of pages like this: > > pgdat->static_init_pgcnt = min_t(unsigned long, PAGES_PER_SECTION, > pgdat->node_spanned_pages); > > because we don't want to overflow zone's range. > > But this is not necessary, since defer_init() is called like this: > > memmap_init_zone() > for pfn in [start_pfn, end_pfn) > defer_init(pfn, end_pfn) > > In case (pgdat->node_spanned_pages < PAGES_PER_SECTION), the loop would > stop before calling defer_init(). > > BTW, comparing PAGES_PER_SECTION with node_spanned_pages is not correct, > since nr_initialised is zone based instead of node based. Even > node_spanned_pages is bigger than PAGES_PER_SECTION, its highest zone > would have pages less than PAGES_PER_SECTION. > > Signed-off-by: Wei Yang <richard.weiyang@gmail.com> This all seems to make sense to me, and appears to be a valid improvement. Reviewed-by: Alexander Duyck <alexander.h.duyck@linux.intel.com> > --- > include/linux/mmzone.h | 2 -- > mm/page_alloc.c | 13 ++++++------- > 2 files changed, 6 insertions(+), 9 deletions(-) > > diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h > index ae7a830a21eb..68d7b558924b 100644 > --- a/include/linux/mmzone.h > +++ b/include/linux/mmzone.h > @@ -692,8 +692,6 @@ typedef struct pglist_data { > * is the first PFN that needs to be initialised. > */ > unsigned long first_deferred_pfn; > - /* Number of non-deferred pages */ > - unsigned long static_init_pgcnt; > #endif /* CONFIG_DEFERRED_STRUCT_PAGE_INIT */ > > #ifdef CONFIG_TRANSPARENT_HUGEPAGE > diff --git a/mm/page_alloc.c b/mm/page_alloc.c > index 76e12179cd5e..b542d82400cf 100644 > --- a/mm/page_alloc.c > +++ b/mm/page_alloc.c > @@ -326,8 +326,13 @@ defer_init(int nid, unsigned long pfn, unsigned long end_pfn) > /* Always populate low zones for address-constrained allocations */ > if (end_pfn < pgdat_end_pfn(NODE_DATA(nid))) > return false; > + > + /* > + * We start only with one section of pages, more pages are added as > + * needed until the rest of deferred pages are initialized. > + */ > nr_initialised++; > - if ((nr_initialised > NODE_DATA(nid)->static_init_pgcnt) && > + if ((nr_initialised > PAGES_PER_SECTION) && > (pfn & (PAGES_PER_SECTION - 1)) == 0) { > NODE_DATA(nid)->first_deferred_pfn = pfn; > return true; > @@ -6451,12 +6456,6 @@ static void __ref alloc_node_mem_map(struct pglist_data *pgdat) { } > #ifdef CONFIG_DEFERRED_STRUCT_PAGE_INIT > static inline void pgdat_set_deferred_range(pg_data_t *pgdat) > { > - /* > - * We start only with one section of pages, more pages are added as > - * needed until the rest of deferred pages are initialized. > - */ > - pgdat->static_init_pgcnt = min_t(unsigned long, PAGES_PER_SECTION, > - pgdat->node_spanned_pages); > pgdat->first_deferred_pfn = ULONG_MAX; > } > #else
On Thu, Dec 20, 2018 at 03:50:29PM -0800, Alexander Duyck wrote: >On Thu, 2018-11-22 at 17:48 +0800, Wei Yang wrote: >> When DEFERRED_STRUCT_PAGE_INIT is configured, only the first section of >> each node's highest zone is initialized before defer stage. >> >> static_init_pgcnt is used to store the number of pages like this: >> >> pgdat->static_init_pgcnt = min_t(unsigned long, PAGES_PER_SECTION, >> pgdat->node_spanned_pages); >> >> because we don't want to overflow zone's range. >> >> But this is not necessary, since defer_init() is called like this: >> >> memmap_init_zone() >> for pfn in [start_pfn, end_pfn) >> defer_init(pfn, end_pfn) >> >> In case (pgdat->node_spanned_pages < PAGES_PER_SECTION), the loop would >> stop before calling defer_init(). >> >> BTW, comparing PAGES_PER_SECTION with node_spanned_pages is not correct, >> since nr_initialised is zone based instead of node based. Even >> node_spanned_pages is bigger than PAGES_PER_SECTION, its highest zone >> would have pages less than PAGES_PER_SECTION. >> >> Signed-off-by: Wei Yang <richard.weiyang@gmail.com> > >This all seems to make sense to me, and appears to be a valid >improvement. > >Reviewed-by: Alexander Duyck <alexander.h.duyck@linux.intel.com> Thanks :-) > >> --- >> include/linux/mmzone.h | 2 -- >> mm/page_alloc.c | 13 ++++++------- >> 2 files changed, 6 insertions(+), 9 deletions(-) >> >> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h >> index ae7a830a21eb..68d7b558924b 100644 >> --- a/include/linux/mmzone.h >> +++ b/include/linux/mmzone.h >> @@ -692,8 +692,6 @@ typedef struct pglist_data { >> * is the first PFN that needs to be initialised. >> */ >> unsigned long first_deferred_pfn; >> - /* Number of non-deferred pages */ >> - unsigned long static_init_pgcnt; >> #endif /* CONFIG_DEFERRED_STRUCT_PAGE_INIT */ >> >> #ifdef CONFIG_TRANSPARENT_HUGEPAGE >> diff --git a/mm/page_alloc.c b/mm/page_alloc.c >> index 76e12179cd5e..b542d82400cf 100644 >> --- a/mm/page_alloc.c >> +++ b/mm/page_alloc.c >> @@ -326,8 +326,13 @@ defer_init(int nid, unsigned long pfn, unsigned long end_pfn) >> /* Always populate low zones for address-constrained allocations */ >> if (end_pfn < pgdat_end_pfn(NODE_DATA(nid))) >> return false; >> + >> + /* >> + * We start only with one section of pages, more pages are added as >> + * needed until the rest of deferred pages are initialized. >> + */ >> nr_initialised++; >> - if ((nr_initialised > NODE_DATA(nid)->static_init_pgcnt) && >> + if ((nr_initialised > PAGES_PER_SECTION) && >> (pfn & (PAGES_PER_SECTION - 1)) == 0) { >> NODE_DATA(nid)->first_deferred_pfn = pfn; >> return true; >> @@ -6451,12 +6456,6 @@ static void __ref alloc_node_mem_map(struct pglist_data *pgdat) { } >> #ifdef CONFIG_DEFERRED_STRUCT_PAGE_INIT >> static inline void pgdat_set_deferred_range(pg_data_t *pgdat) >> { >> - /* >> - * We start only with one section of pages, more pages are added as >> - * needed until the rest of deferred pages are initialized. >> - */ >> - pgdat->static_init_pgcnt = min_t(unsigned long, PAGES_PER_SECTION, >> - pgdat->node_spanned_pages); >> pgdat->first_deferred_pfn = ULONG_MAX; >> } >> #else
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h index ae7a830a21eb..68d7b558924b 100644 --- a/include/linux/mmzone.h +++ b/include/linux/mmzone.h @@ -692,8 +692,6 @@ typedef struct pglist_data { * is the first PFN that needs to be initialised. */ unsigned long first_deferred_pfn; - /* Number of non-deferred pages */ - unsigned long static_init_pgcnt; #endif /* CONFIG_DEFERRED_STRUCT_PAGE_INIT */ #ifdef CONFIG_TRANSPARENT_HUGEPAGE diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 76e12179cd5e..b542d82400cf 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -326,8 +326,13 @@ defer_init(int nid, unsigned long pfn, unsigned long end_pfn) /* Always populate low zones for address-constrained allocations */ if (end_pfn < pgdat_end_pfn(NODE_DATA(nid))) return false; + + /* + * We start only with one section of pages, more pages are added as + * needed until the rest of deferred pages are initialized. + */ nr_initialised++; - if ((nr_initialised > NODE_DATA(nid)->static_init_pgcnt) && + if ((nr_initialised > PAGES_PER_SECTION) && (pfn & (PAGES_PER_SECTION - 1)) == 0) { NODE_DATA(nid)->first_deferred_pfn = pfn; return true; @@ -6451,12 +6456,6 @@ static void __ref alloc_node_mem_map(struct pglist_data *pgdat) { } #ifdef CONFIG_DEFERRED_STRUCT_PAGE_INIT static inline void pgdat_set_deferred_range(pg_data_t *pgdat) { - /* - * We start only with one section of pages, more pages are added as - * needed until the rest of deferred pages are initialized. - */ - pgdat->static_init_pgcnt = min_t(unsigned long, PAGES_PER_SECTION, - pgdat->node_spanned_pages); pgdat->first_deferred_pfn = ULONG_MAX; } #else
When DEFERRED_STRUCT_PAGE_INIT is configured, only the first section of each node's highest zone is initialized before defer stage. static_init_pgcnt is used to store the number of pages like this: pgdat->static_init_pgcnt = min_t(unsigned long, PAGES_PER_SECTION, pgdat->node_spanned_pages); because we don't want to overflow zone's range. But this is not necessary, since defer_init() is called like this: memmap_init_zone() for pfn in [start_pfn, end_pfn) defer_init(pfn, end_pfn) In case (pgdat->node_spanned_pages < PAGES_PER_SECTION), the loop would stop before calling defer_init(). BTW, comparing PAGES_PER_SECTION with node_spanned_pages is not correct, since nr_initialised is zone based instead of node based. Even node_spanned_pages is bigger than PAGES_PER_SECTION, its highest zone would have pages less than PAGES_PER_SECTION. Signed-off-by: Wei Yang <richard.weiyang@gmail.com> --- include/linux/mmzone.h | 2 -- mm/page_alloc.c | 13 ++++++------- 2 files changed, 6 insertions(+), 9 deletions(-)