diff mbox

[v9,06/12] mm: zero struct pages during initialization

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

Commit Message

Pavel Tatashin Sept. 20, 2017, 8:17 p.m. UTC
Add struct page zeroing as a part of initialization of other fields in
__init_single_page().

This single thread performance collected on: Intel(R) Xeon(R) CPU E7-8895
v3 @ 2.60GHz with 1T of memory (268400646 pages in 8 nodes):

                        BASE            FIX
sparse_init     11.244671836s   0.007199623s
zone_sizes_init  4.879775891s   8.355182299s
                  --------------------------
Total           16.124447727s   8.362381922s

sparse_init is where memory for struct pages is zeroed, and the zeroing
part is moved later in this patch into __init_single_page(), which is
called from zone_sizes_init().

Signed-off-by: Pavel Tatashin <pasha.tatashin@oracle.com>
Reviewed-by: Steven Sistare <steven.sistare@oracle.com>
Reviewed-by: Daniel Jordan <daniel.m.jordan@oracle.com>
Reviewed-by: Bob Picco <bob.picco@oracle.com>
Acked-by: Michal Hocko <mhocko@suse.com>
---
 include/linux/mm.h | 9 +++++++++
 mm/page_alloc.c    | 1 +
 2 files changed, 10 insertions(+)

Comments

Michal Hocko Oct. 3, 2017, 1:08 p.m. UTC | #1
On Wed 20-09-17 16:17:08, Pavel Tatashin wrote:
> Add struct page zeroing as a part of initialization of other fields in
> __init_single_page().
> 
> This single thread performance collected on: Intel(R) Xeon(R) CPU E7-8895
> v3 @ 2.60GHz with 1T of memory (268400646 pages in 8 nodes):
> 
>                         BASE            FIX
> sparse_init     11.244671836s   0.007199623s
> zone_sizes_init  4.879775891s   8.355182299s
>                   --------------------------
> Total           16.124447727s   8.362381922s

Hmm, this is confusing. This assumes that sparse_init doesn't zero pages
anymore, right? So these number depend on the last patch in the series?

> sparse_init is where memory for struct pages is zeroed, and the zeroing
> part is moved later in this patch into __init_single_page(), which is
> called from zone_sizes_init().
> 
> Signed-off-by: Pavel Tatashin <pasha.tatashin@oracle.com>
> Reviewed-by: Steven Sistare <steven.sistare@oracle.com>
> Reviewed-by: Daniel Jordan <daniel.m.jordan@oracle.com>
> Reviewed-by: Bob Picco <bob.picco@oracle.com>
> Acked-by: Michal Hocko <mhocko@suse.com>
> ---
>  include/linux/mm.h | 9 +++++++++
>  mm/page_alloc.c    | 1 +
>  2 files changed, 10 insertions(+)
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index f8c10d336e42..50b74d628243 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -94,6 +94,15 @@ extern int mmap_rnd_compat_bits __read_mostly;
>  #define mm_forbids_zeropage(X)	(0)
>  #endif
>  
> +/*
> + * On some architectures it is expensive to call memset() for small sizes.
> + * Those architectures should provide their own implementation of "struct page"
> + * zeroing by defining this macro in <asm/pgtable.h>.
> + */
> +#ifndef mm_zero_struct_page
> +#define mm_zero_struct_page(pp)  ((void)memset((pp), 0, sizeof(struct page)))
> +#endif
> +
>  /*
>   * Default maximum number of active map areas, this limits the number of vmas
>   * per mm struct. Users can overwrite this number by sysctl but there is a
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index a8dbd405ed94..4b630ee91430 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -1170,6 +1170,7 @@ static void free_one_page(struct zone *zone,
>  static void __meminit __init_single_page(struct page *page, unsigned long pfn,
>  				unsigned long zone, int nid)
>  {
> +	mm_zero_struct_page(page);
>  	set_page_links(page, zone, nid, pfn);
>  	init_page_count(page);
>  	page_mapcount_reset(page);
> -- 
> 2.14.1
Pavel Tatashin Oct. 3, 2017, 3:22 p.m. UTC | #2
On 10/03/2017 09:08 AM, Michal Hocko wrote:
> On Wed 20-09-17 16:17:08, Pavel Tatashin wrote:
>> Add struct page zeroing as a part of initialization of other fields in
>> __init_single_page().
>>
>> This single thread performance collected on: Intel(R) Xeon(R) CPU E7-8895
>> v3 @ 2.60GHz with 1T of memory (268400646 pages in 8 nodes):
>>
>>                          BASE            FIX
>> sparse_init     11.244671836s   0.007199623s
>> zone_sizes_init  4.879775891s   8.355182299s
>>                    --------------------------
>> Total           16.124447727s   8.362381922s
> 
> Hmm, this is confusing. This assumes that sparse_init doesn't zero pages
> anymore, right? So these number depend on the last patch in the series?

Correct, without the last patch sparse_init time won't change.

Pasha
Michal Hocko Oct. 4, 2017, 8:45 a.m. UTC | #3
On Tue 03-10-17 11:22:35, Pasha Tatashin wrote:
> 
> 
> On 10/03/2017 09:08 AM, Michal Hocko wrote:
> > On Wed 20-09-17 16:17:08, Pavel Tatashin wrote:
> > > Add struct page zeroing as a part of initialization of other fields in
> > > __init_single_page().
> > > 
> > > This single thread performance collected on: Intel(R) Xeon(R) CPU E7-8895
> > > v3 @ 2.60GHz with 1T of memory (268400646 pages in 8 nodes):
> > > 
> > >                          BASE            FIX
> > > sparse_init     11.244671836s   0.007199623s
> > > zone_sizes_init  4.879775891s   8.355182299s
> > >                    --------------------------
> > > Total           16.124447727s   8.362381922s
> > 
> > Hmm, this is confusing. This assumes that sparse_init doesn't zero pages
> > anymore, right? So these number depend on the last patch in the series?
> 
> Correct, without the last patch sparse_init time won't change.

THen this is just misleading.
Pavel Tatashin Oct. 4, 2017, 12:26 p.m. UTC | #4
On 10/04/2017 04:45 AM, Michal Hocko wrote:
> On Tue 03-10-17 11:22:35, Pasha Tatashin wrote:
>>
>>
>> On 10/03/2017 09:08 AM, Michal Hocko wrote:
>>> On Wed 20-09-17 16:17:08, Pavel Tatashin wrote:
>>>> Add struct page zeroing as a part of initialization of other fields in
>>>> __init_single_page().
>>>>
>>>> This single thread performance collected on: Intel(R) Xeon(R) CPU E7-8895
>>>> v3 @ 2.60GHz with 1T of memory (268400646 pages in 8 nodes):
>>>>
>>>>                           BASE            FIX
>>>> sparse_init     11.244671836s   0.007199623s
>>>> zone_sizes_init  4.879775891s   8.355182299s
>>>>                     --------------------------
>>>> Total           16.124447727s   8.362381922s
>>>
>>> Hmm, this is confusing. This assumes that sparse_init doesn't zero pages
>>> anymore, right? So these number depend on the last patch in the series?
>>
>> Correct, without the last patch sparse_init time won't change.
> 
> THen this is just misleading.
> 

OK, I will re-arrange patches the way you suggested earlier.

Pasha
diff mbox

Patch

diff --git a/include/linux/mm.h b/include/linux/mm.h
index f8c10d336e42..50b74d628243 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -94,6 +94,15 @@  extern int mmap_rnd_compat_bits __read_mostly;
 #define mm_forbids_zeropage(X)	(0)
 #endif
 
+/*
+ * On some architectures it is expensive to call memset() for small sizes.
+ * Those architectures should provide their own implementation of "struct page"
+ * zeroing by defining this macro in <asm/pgtable.h>.
+ */
+#ifndef mm_zero_struct_page
+#define mm_zero_struct_page(pp)  ((void)memset((pp), 0, sizeof(struct page)))
+#endif
+
 /*
  * Default maximum number of active map areas, this limits the number of vmas
  * per mm struct. Users can overwrite this number by sysctl but there is a
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index a8dbd405ed94..4b630ee91430 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1170,6 +1170,7 @@  static void free_one_page(struct zone *zone,
 static void __meminit __init_single_page(struct page *page, unsigned long pfn,
 				unsigned long zone, int nid)
 {
+	mm_zero_struct_page(page);
 	set_page_links(page, zone, nid, pfn);
 	init_page_count(page);
 	page_mapcount_reset(page);