diff mbox

mm: skip invalid pages block at a time in zero_resv_unresv

Message ID 20180615155733.1175-1-pasha.tatashin@oracle.com (mailing list archive)
State New, archived
Headers show

Commit Message

Pavel Tatashin June 15, 2018, 3:57 p.m. UTC
The role of zero_resv_unavail() is to make sure that every struct page that
is allocated but is not backed by memory that is accessible by kernel is
zeroed and not in some uninitialized state.

Since struct pages are allocated in blocks (2M pages in x86 case), we can
skip pageblock_nr_pages at a time, when the first one is found to be
invalid.

This optimization may help since now on x86 every hole in e820 maps
is marked as reserved in memblock, and thus will go through this function.

This function is called before sched_clock() is initialized, so I used my
x86 early boot clock patches to measure the performance improvement.

With 1T hole on i7-8700 currently we would take 0.606918s of boot time, but
with this optimization 0.001103s.

Signed-off-by: Pavel Tatashin <pasha.tatashin@oracle.com>
---
 mm/page_alloc.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Oscar Salvador June 15, 2018, 6:11 p.m. UTC | #1
On Fri, Jun 15, 2018 at 11:57:33AM -0400, Pavel Tatashin wrote:
> The role of zero_resv_unavail() is to make sure that every struct page that
> is allocated but is not backed by memory that is accessible by kernel is
> zeroed and not in some uninitialized state.
> 
> Since struct pages are allocated in blocks (2M pages in x86 case), we can
> skip pageblock_nr_pages at a time, when the first one is found to be
> invalid.
> 
> This optimization may help since now on x86 every hole in e820 maps
> is marked as reserved in memblock, and thus will go through this function.
> 
> This function is called before sched_clock() is initialized, so I used my
> x86 early boot clock patches to measure the performance improvement.
> 
> With 1T hole on i7-8700 currently we would take 0.606918s of boot time, but
> with this optimization 0.001103s.
> 
> Signed-off-by: Pavel Tatashin <pasha.tatashin@oracle.com>
> ---
>  mm/page_alloc.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 1521100f1e63..94f1b3201735 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -6404,8 +6404,11 @@ void __paginginit zero_resv_unavail(void)
>  	pgcnt = 0;
>  	for_each_resv_unavail_range(i, &start, &end) {
>  		for (pfn = PFN_DOWN(start); pfn < PFN_UP(end); pfn++) {
> -			if (!pfn_valid(ALIGN_DOWN(pfn, pageblock_nr_pages)))
> +			if (!pfn_valid(ALIGN_DOWN(pfn, pageblock_nr_pages))) {
> +				pfn = ALIGN_DOWN(pfn, pageblock_nr_pages)
> +					+ pageblock_nr_pages - 1;
>  				continue;
> +			}
>  			mm_zero_struct_page(pfn_to_page(pfn));
>  			pgcnt++;
>  		}

Hi Pavel,

Thanks for the patch.
This looks good to me.

Reviewed-by: Oscar Salvador <osalvador@suse.de>

> -- 
> 2.17.1
> 

Best Regards
Oscar Salvador
Pavel Tatashin June 15, 2018, 7:32 p.m. UTC | #2
> Hi Pavel,
>
> Thanks for the patch.
> This looks good to me.
>
> Reviewed-by: Oscar Salvador <osalvador@suse.de>

Thank you Oscar!

Pavel
Naoya Horiguchi June 20, 2018, 2:14 a.m. UTC | #3
On Fri, Jun 15, 2018 at 11:57:33AM -0400, Pavel Tatashin wrote:
> The role of zero_resv_unavail() is to make sure that every struct page that
> is allocated but is not backed by memory that is accessible by kernel is
> zeroed and not in some uninitialized state.
> 
> Since struct pages are allocated in blocks (2M pages in x86 case), we can
> skip pageblock_nr_pages at a time, when the first one is found to be
> invalid.
> 
> This optimization may help since now on x86 every hole in e820 maps
> is marked as reserved in memblock, and thus will go through this function.
> 
> This function is called before sched_clock() is initialized, so I used my
> x86 early boot clock patches to measure the performance improvement.
> 
> With 1T hole on i7-8700 currently we would take 0.606918s of boot time, but
> with this optimization 0.001103s.
> 
> Signed-off-by: Pavel Tatashin <pasha.tatashin@oracle.com>

Looks good to me, thanks!

Reviewed-by: Naoya Horiguchi <n-horiguchi@ah.jp.nec.com>

> ---
>  mm/page_alloc.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 1521100f1e63..94f1b3201735 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -6404,8 +6404,11 @@ void __paginginit zero_resv_unavail(void)
>  	pgcnt = 0;
>  	for_each_resv_unavail_range(i, &start, &end) {
>  		for (pfn = PFN_DOWN(start); pfn < PFN_UP(end); pfn++) {
> -			if (!pfn_valid(ALIGN_DOWN(pfn, pageblock_nr_pages)))
> +			if (!pfn_valid(ALIGN_DOWN(pfn, pageblock_nr_pages))) {
> +				pfn = ALIGN_DOWN(pfn, pageblock_nr_pages)
> +					+ pageblock_nr_pages - 1;
>  				continue;
> +			}
>  			mm_zero_struct_page(pfn_to_page(pfn));
>  			pgcnt++;
>  		}
> -- 
> 2.17.1
> 
>
diff mbox

Patch

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 1521100f1e63..94f1b3201735 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -6404,8 +6404,11 @@  void __paginginit zero_resv_unavail(void)
 	pgcnt = 0;
 	for_each_resv_unavail_range(i, &start, &end) {
 		for (pfn = PFN_DOWN(start); pfn < PFN_UP(end); pfn++) {
-			if (!pfn_valid(ALIGN_DOWN(pfn, pageblock_nr_pages)))
+			if (!pfn_valid(ALIGN_DOWN(pfn, pageblock_nr_pages))) {
+				pfn = ALIGN_DOWN(pfn, pageblock_nr_pages)
+					+ pageblock_nr_pages - 1;
 				continue;
+			}
 			mm_zero_struct_page(pfn_to_page(pfn));
 			pgcnt++;
 		}