linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] mm:Add watermark slope for high mark
@ 2017-11-24 10:07 Peter Enderborg
  2017-11-24 10:14 ` Michal Hocko
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Peter Enderborg @ 2017-11-24 10:07 UTC (permalink / raw)
  To: Michal Hocko, linux-doc, linux-kernel, linux-mm, linux-fsdevel
  Cc: Jonathan Corbet, Luis R . Rodriguez, Kees Cook, Alex Deucher,
	David S . Miller, Harry Wentland, Greg Kroah-Hartman, Tony Cheng,
	David Rientjes, Peter Enderborg, Andrew Morton, Jan Kara,
	Kirill A . Shutemov, Dave Jiang, Jérôme Glisse,
	Ross Zwisler, Matthew Wilcox, Hugh Dickins, Johannes Weiner,
	Kemi Wang, Vlastimil Babka, YASUAKI ISHIMATSU, Nikolay Borisov,
	Mel Gorman, Pavel Tatashin

When tuning the watermark_scale_factor to reduce stalls and compactions
the high mark is also changed, it changed a bit too much. So this
patch introduces a slope that can reduce this overhead a bit, or
increase it if needed.

Signed-off-by: Peter Enderborg <peter.enderborg@sony.com>
---
 Documentation/sysctl/vm.txt | 15 +++++++++++++++
 include/linux/mm.h          |  1 +
 include/linux/mmzone.h      |  2 ++
 kernel/sysctl.c             |  9 +++++++++
 mm/page_alloc.c             |  6 +++++-
 5 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/Documentation/sysctl/vm.txt b/Documentation/sysctl/vm.txt
index eda628c..aecff6c 100644
--- a/Documentation/sysctl/vm.txt
+++ b/Documentation/sysctl/vm.txt
@@ -62,6 +62,7 @@ Currently, these files are in /proc/sys/vm:
 - user_reserve_kbytes
 - vfs_cache_pressure
 - watermark_scale_factor
+- watermark_high_factor_slope
 - zone_reclaim_mode
 
 ==============================================================
@@ -857,6 +858,20 @@ that the number of free pages kswapd maintains for latency reasons is
 too small for the allocation bursts occurring in the system. This knob
 can then be used to tune kswapd aggressiveness accordingly.
 
+=============================================================
+
+watermark_high_factor_slope:
+
+This factor is high mark for watermark_scale_factor.
+The unit is in percent.
+Max value is 1000 and min value is 100. (High watermark is the same as
+low water mark) Low watermark is min_wmark_pages + watermark_scale_factor.
+and high watermark is
+min_wmark_pages+(watermark_scale_factor * watermark_high_factor_slope).
+
+The default value is 200.
+
+
 ==============================================================
 
 zone_reclaim_mode:
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 7661156..c89536b 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2094,6 +2094,7 @@ extern void zone_pcp_reset(struct zone *zone);
 /* page_alloc.c */
 extern int min_free_kbytes;
 extern int watermark_scale_factor;
+extern int watermark_high_factor_slope;
 
 /* nommu.c */
 extern atomic_long_t mmap_pages_allocated;
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 67f2e3c..91bf842 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -886,6 +886,8 @@ int min_free_kbytes_sysctl_handler(struct ctl_table *, int,
 					void __user *, size_t *, loff_t *);
 int watermark_scale_factor_sysctl_handler(struct ctl_table *, int,
 					void __user *, size_t *, loff_t *);
+//int watermark_high_factor_tilt_sysctl_handler(struct ctl_table *, int,
+//					void __user *, size_t *, loff_t *);
 extern int sysctl_lowmem_reserve_ratio[MAX_NR_ZONES-1];
 int lowmem_reserve_ratio_sysctl_handler(struct ctl_table *, int,
 					void __user *, size_t *, loff_t *);
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 2fb4e27..83c48c9 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -1444,6 +1444,15 @@ static struct ctl_table vm_table[] = {
 		.extra2		= &one_thousand,
 	},
 	{
+		.procname	= "watermark_high_factor_slope",
+		.data		= &watermark_high_factor_slope,
+		.maxlen		= sizeof(watermark_high_factor_slope),
+		.mode		= 0644,
+		.proc_handler	= watermark_scale_factor_sysctl_handler,
+		.extra1		= &one_hundred,
+		.extra2		= &one_thousand,
+	},
+	{
 		.procname	= "percpu_pagelist_fraction",
 		.data		= &percpu_pagelist_fraction,
 		.maxlen		= sizeof(percpu_pagelist_fraction),
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 48b5b01..3dc50ff 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -263,6 +263,7 @@ compound_page_dtor * const compound_page_dtors[] = {
 int min_free_kbytes = 1024;
 int user_min_free_kbytes = -1;
 int watermark_scale_factor = 10;
+int watermark_high_factor_slope = 200;
 
 static unsigned long __meminitdata nr_kernel_pages;
 static unsigned long __meminitdata nr_all_pages;
@@ -6989,6 +6990,7 @@ static void __setup_per_zone_wmarks(void)
 
 	for_each_zone(zone) {
 		u64 tmp;
+		u64 tmp_high;
 
 		spin_lock_irqsave(&zone->lock, flags);
 		tmp = (u64)pages_min * zone->managed_pages;
@@ -7026,7 +7028,9 @@ static void __setup_per_zone_wmarks(void)
 				      watermark_scale_factor, 10000));
 
 		zone->watermark[WMARK_LOW]  = min_wmark_pages(zone) + tmp;
-		zone->watermark[WMARK_HIGH] = min_wmark_pages(zone) + tmp * 2;
+		tmp_high = mult_frac(tmp, watermark_high_factor_slope, 100);
+		zone->watermark[WMARK_HIGH] = min_wmark_pages(zone) + tmp_high;
+
 
 		spin_unlock_irqrestore(&zone->lock, flags);
 	}
-- 
2.7.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] mm:Add watermark slope for high mark
  2017-11-24 10:07 [PATCH] mm:Add watermark slope for high mark Peter Enderborg
@ 2017-11-24 10:14 ` Michal Hocko
  2017-11-24 10:15   ` Vlastimil Babka
  2017-11-24 13:12   ` peter enderborg
  2017-11-27  7:03 ` kbuild test robot
  2017-11-27  7:44 ` kbuild test robot
  2 siblings, 2 replies; 8+ messages in thread
From: Michal Hocko @ 2017-11-24 10:14 UTC (permalink / raw)
  To: Peter Enderborg
  Cc: linux-doc, linux-kernel, linux-mm, linux-fsdevel,
	Jonathan Corbet, Luis R . Rodriguez, Kees Cook, Alex Deucher,
	David S . Miller, Harry Wentland, Greg Kroah-Hartman, Tony Cheng,
	David Rientjes, Andrew Morton, Jan Kara, Kirill A . Shutemov,
	Dave Jiang, Jérôme Glisse, Ross Zwisler,
	Matthew Wilcox, Hugh Dickins, Johannes Weiner, Kemi Wang,
	Vlastimil Babka, YASUAKI ISHIMATSU, Nikolay Borisov, Mel Gorman,
	Pavel Tatashin

On Fri 24-11-17 11:07:07, Peter Enderborg wrote:
> When tuning the watermark_scale_factor to reduce stalls and compactions
> the high mark is also changed, it changed a bit too much. So this
> patch introduces a slope that can reduce this overhead a bit, or
> increase it if needed.

This doesn't explain what is the problem, why it is a problem and why we
need yet another tuning to address it. Users shouldn't really care about
internal stuff like watermark tuning for each watermark independently.
This looks like a gross hack. Please start over with the problem
description and then we can move on to an approapriate fix. Piling up
tuning knobs to workaround problems is simply not acceptable.
 
> Signed-off-by: Peter Enderborg <peter.enderborg@sony.com>
> ---
>  Documentation/sysctl/vm.txt | 15 +++++++++++++++
>  include/linux/mm.h          |  1 +
>  include/linux/mmzone.h      |  2 ++
>  kernel/sysctl.c             |  9 +++++++++
>  mm/page_alloc.c             |  6 +++++-
>  5 files changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/sysctl/vm.txt b/Documentation/sysctl/vm.txt
> index eda628c..aecff6c 100644
> --- a/Documentation/sysctl/vm.txt
> +++ b/Documentation/sysctl/vm.txt
> @@ -62,6 +62,7 @@ Currently, these files are in /proc/sys/vm:
>  - user_reserve_kbytes
>  - vfs_cache_pressure
>  - watermark_scale_factor
> +- watermark_high_factor_slope
>  - zone_reclaim_mode
>  
>  ==============================================================
> @@ -857,6 +858,20 @@ that the number of free pages kswapd maintains for latency reasons is
>  too small for the allocation bursts occurring in the system. This knob
>  can then be used to tune kswapd aggressiveness accordingly.
>  
> +=============================================================
> +
> +watermark_high_factor_slope:
> +
> +This factor is high mark for watermark_scale_factor.
> +The unit is in percent.
> +Max value is 1000 and min value is 100. (High watermark is the same as
> +low water mark) Low watermark is min_wmark_pages + watermark_scale_factor.
> +and high watermark is
> +min_wmark_pages+(watermark_scale_factor * watermark_high_factor_slope).
> +
> +The default value is 200.
> +
> +
>  ==============================================================
>  
>  zone_reclaim_mode:
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 7661156..c89536b 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2094,6 +2094,7 @@ extern void zone_pcp_reset(struct zone *zone);
>  /* page_alloc.c */
>  extern int min_free_kbytes;
>  extern int watermark_scale_factor;
> +extern int watermark_high_factor_slope;
>  
>  /* nommu.c */
>  extern atomic_long_t mmap_pages_allocated;
> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
> index 67f2e3c..91bf842 100644
> --- a/include/linux/mmzone.h
> +++ b/include/linux/mmzone.h
> @@ -886,6 +886,8 @@ int min_free_kbytes_sysctl_handler(struct ctl_table *, int,
>  					void __user *, size_t *, loff_t *);
>  int watermark_scale_factor_sysctl_handler(struct ctl_table *, int,
>  					void __user *, size_t *, loff_t *);
> +//int watermark_high_factor_tilt_sysctl_handler(struct ctl_table *, int,
> +//					void __user *, size_t *, loff_t *);
>  extern int sysctl_lowmem_reserve_ratio[MAX_NR_ZONES-1];
>  int lowmem_reserve_ratio_sysctl_handler(struct ctl_table *, int,
>  					void __user *, size_t *, loff_t *);
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index 2fb4e27..83c48c9 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -1444,6 +1444,15 @@ static struct ctl_table vm_table[] = {
>  		.extra2		= &one_thousand,
>  	},
>  	{
> +		.procname	= "watermark_high_factor_slope",
> +		.data		= &watermark_high_factor_slope,
> +		.maxlen		= sizeof(watermark_high_factor_slope),
> +		.mode		= 0644,
> +		.proc_handler	= watermark_scale_factor_sysctl_handler,
> +		.extra1		= &one_hundred,
> +		.extra2		= &one_thousand,
> +	},
> +	{
>  		.procname	= "percpu_pagelist_fraction",
>  		.data		= &percpu_pagelist_fraction,
>  		.maxlen		= sizeof(percpu_pagelist_fraction),
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 48b5b01..3dc50ff 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -263,6 +263,7 @@ compound_page_dtor * const compound_page_dtors[] = {
>  int min_free_kbytes = 1024;
>  int user_min_free_kbytes = -1;
>  int watermark_scale_factor = 10;
> +int watermark_high_factor_slope = 200;
>  
>  static unsigned long __meminitdata nr_kernel_pages;
>  static unsigned long __meminitdata nr_all_pages;
> @@ -6989,6 +6990,7 @@ static void __setup_per_zone_wmarks(void)
>  
>  	for_each_zone(zone) {
>  		u64 tmp;
> +		u64 tmp_high;
>  
>  		spin_lock_irqsave(&zone->lock, flags);
>  		tmp = (u64)pages_min * zone->managed_pages;
> @@ -7026,7 +7028,9 @@ static void __setup_per_zone_wmarks(void)
>  				      watermark_scale_factor, 10000));
>  
>  		zone->watermark[WMARK_LOW]  = min_wmark_pages(zone) + tmp;
> -		zone->watermark[WMARK_HIGH] = min_wmark_pages(zone) + tmp * 2;
> +		tmp_high = mult_frac(tmp, watermark_high_factor_slope, 100);
> +		zone->watermark[WMARK_HIGH] = min_wmark_pages(zone) + tmp_high;
> +
>  
>  		spin_unlock_irqrestore(&zone->lock, flags);
>  	}
> -- 
> 2.7.4
> 

-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] mm:Add watermark slope for high mark
  2017-11-24 10:14 ` Michal Hocko
@ 2017-11-24 10:15   ` Vlastimil Babka
  2017-11-24 10:34     ` peter enderborg
  2017-11-24 13:12   ` peter enderborg
  1 sibling, 1 reply; 8+ messages in thread
From: Vlastimil Babka @ 2017-11-24 10:15 UTC (permalink / raw)
  To: Michal Hocko, Peter Enderborg
  Cc: linux-doc, linux-kernel, linux-mm, linux-fsdevel,
	Jonathan Corbet, Luis R . Rodriguez, Kees Cook, Alex Deucher,
	David S . Miller, Harry Wentland, Greg Kroah-Hartman, Tony Cheng,
	David Rientjes, Andrew Morton, Jan Kara, Kirill A . Shutemov,
	Dave Jiang, Jérôme Glisse, Ross Zwisler,
	Matthew Wilcox, Hugh Dickins, Johannes Weiner, Kemi Wang,
	YASUAKI ISHIMATSU, Nikolay Borisov, Mel Gorman, Pavel Tatashin,
	Linux API

On 11/24/2017 11:14 AM, Michal Hocko wrote:
> On Fri 24-11-17 11:07:07, Peter Enderborg wrote:
>> When tuning the watermark_scale_factor to reduce stalls and compactions
>> the high mark is also changed, it changed a bit too much. So this
>> patch introduces a slope that can reduce this overhead a bit, or
>> increase it if needed.
> 
> This doesn't explain what is the problem, why it is a problem and why we
> need yet another tuning to address it. Users shouldn't really care about
> internal stuff like watermark tuning for each watermark independently.
> This looks like a gross hack. Please start over with the problem
> description and then we can move on to an approapriate fix. Piling up
> tuning knobs to workaround problems is simply not acceptable.

Agreed. Also if you send a patch adding userspace API or a tuning knob,
please CC linux-api mailing list (did that for this reply).

>> Signed-off-by: Peter Enderborg <peter.enderborg@sony.com>
>> ---
>>  Documentation/sysctl/vm.txt | 15 +++++++++++++++
>>  include/linux/mm.h          |  1 +
>>  include/linux/mmzone.h      |  2 ++
>>  kernel/sysctl.c             |  9 +++++++++
>>  mm/page_alloc.c             |  6 +++++-
>>  5 files changed, 32 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/sysctl/vm.txt b/Documentation/sysctl/vm.txt
>> index eda628c..aecff6c 100644
>> --- a/Documentation/sysctl/vm.txt
>> +++ b/Documentation/sysctl/vm.txt
>> @@ -62,6 +62,7 @@ Currently, these files are in /proc/sys/vm:
>>  - user_reserve_kbytes
>>  - vfs_cache_pressure
>>  - watermark_scale_factor
>> +- watermark_high_factor_slope
>>  - zone_reclaim_mode
>>  
>>  ==============================================================
>> @@ -857,6 +858,20 @@ that the number of free pages kswapd maintains for latency reasons is
>>  too small for the allocation bursts occurring in the system. This knob
>>  can then be used to tune kswapd aggressiveness accordingly.
>>  
>> +=============================================================
>> +
>> +watermark_high_factor_slope:
>> +
>> +This factor is high mark for watermark_scale_factor.
>> +The unit is in percent.
>> +Max value is 1000 and min value is 100. (High watermark is the same as
>> +low water mark) Low watermark is min_wmark_pages + watermark_scale_factor.
>> +and high watermark is
>> +min_wmark_pages+(watermark_scale_factor * watermark_high_factor_slope).
>> +
>> +The default value is 200.
>> +
>> +
>>  ==============================================================
>>  
>>  zone_reclaim_mode:
>> diff --git a/include/linux/mm.h b/include/linux/mm.h
>> index 7661156..c89536b 100644
>> --- a/include/linux/mm.h
>> +++ b/include/linux/mm.h
>> @@ -2094,6 +2094,7 @@ extern void zone_pcp_reset(struct zone *zone);
>>  /* page_alloc.c */
>>  extern int min_free_kbytes;
>>  extern int watermark_scale_factor;
>> +extern int watermark_high_factor_slope;
>>  
>>  /* nommu.c */
>>  extern atomic_long_t mmap_pages_allocated;
>> diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
>> index 67f2e3c..91bf842 100644
>> --- a/include/linux/mmzone.h
>> +++ b/include/linux/mmzone.h
>> @@ -886,6 +886,8 @@ int min_free_kbytes_sysctl_handler(struct ctl_table *, int,
>>  					void __user *, size_t *, loff_t *);
>>  int watermark_scale_factor_sysctl_handler(struct ctl_table *, int,
>>  					void __user *, size_t *, loff_t *);
>> +//int watermark_high_factor_tilt_sysctl_handler(struct ctl_table *, int,
>> +//					void __user *, size_t *, loff_t *);
>>  extern int sysctl_lowmem_reserve_ratio[MAX_NR_ZONES-1];
>>  int lowmem_reserve_ratio_sysctl_handler(struct ctl_table *, int,
>>  					void __user *, size_t *, loff_t *);
>> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
>> index 2fb4e27..83c48c9 100644
>> --- a/kernel/sysctl.c
>> +++ b/kernel/sysctl.c
>> @@ -1444,6 +1444,15 @@ static struct ctl_table vm_table[] = {
>>  		.extra2		= &one_thousand,
>>  	},
>>  	{
>> +		.procname	= "watermark_high_factor_slope",
>> +		.data		= &watermark_high_factor_slope,
>> +		.maxlen		= sizeof(watermark_high_factor_slope),
>> +		.mode		= 0644,
>> +		.proc_handler	= watermark_scale_factor_sysctl_handler,
>> +		.extra1		= &one_hundred,
>> +		.extra2		= &one_thousand,
>> +	},
>> +	{
>>  		.procname	= "percpu_pagelist_fraction",
>>  		.data		= &percpu_pagelist_fraction,
>>  		.maxlen		= sizeof(percpu_pagelist_fraction),
>> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
>> index 48b5b01..3dc50ff 100644
>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -263,6 +263,7 @@ compound_page_dtor * const compound_page_dtors[] = {
>>  int min_free_kbytes = 1024;
>>  int user_min_free_kbytes = -1;
>>  int watermark_scale_factor = 10;
>> +int watermark_high_factor_slope = 200;
>>  
>>  static unsigned long __meminitdata nr_kernel_pages;
>>  static unsigned long __meminitdata nr_all_pages;
>> @@ -6989,6 +6990,7 @@ static void __setup_per_zone_wmarks(void)
>>  
>>  	for_each_zone(zone) {
>>  		u64 tmp;
>> +		u64 tmp_high;
>>  
>>  		spin_lock_irqsave(&zone->lock, flags);
>>  		tmp = (u64)pages_min * zone->managed_pages;
>> @@ -7026,7 +7028,9 @@ static void __setup_per_zone_wmarks(void)
>>  				      watermark_scale_factor, 10000));
>>  
>>  		zone->watermark[WMARK_LOW]  = min_wmark_pages(zone) + tmp;
>> -		zone->watermark[WMARK_HIGH] = min_wmark_pages(zone) + tmp * 2;
>> +		tmp_high = mult_frac(tmp, watermark_high_factor_slope, 100);
>> +		zone->watermark[WMARK_HIGH] = min_wmark_pages(zone) + tmp_high;
>> +
>>  
>>  		spin_unlock_irqrestore(&zone->lock, flags);
>>  	}
>> -- 
>> 2.7.4
>>
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] mm:Add watermark slope for high mark
  2017-11-24 10:15   ` Vlastimil Babka
@ 2017-11-24 10:34     ` peter enderborg
  0 siblings, 0 replies; 8+ messages in thread
From: peter enderborg @ 2017-11-24 10:34 UTC (permalink / raw)
  To: Vlastimil Babka, Michal Hocko
  Cc: linux-doc, linux-kernel, linux-mm, linux-fsdevel,
	Jonathan Corbet, Luis R . Rodriguez, Kees Cook, Alex Deucher,
	David S . Miller, Harry Wentland, Greg Kroah-Hartman, Tony Cheng,
	David Rientjes, Andrew Morton, Jan Kara, Kirill A . Shutemov,
	Dave Jiang, Jérôme Glisse, Ross Zwisler,
	Matthew Wilcox, Hugh Dickins, Johannes Weiner, Kemi Wang,
	YASUAKI ISHIMATSU, Nikolay Borisov, Mel Gorman, Pavel Tatashin,
	Linux API

On 11/24/2017 11:15 AM, Vlastimil Babka wrote:
> Agreed. Also if you send a patch adding userspace API or a tuning knob,
> please CC linux-api mailing list (did that for this reply).
>
The cc-list is generated by get_maintainer.pl script.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] mm:Add watermark slope for high mark
  2017-11-24 10:14 ` Michal Hocko
  2017-11-24 10:15   ` Vlastimil Babka
@ 2017-11-24 13:12   ` peter enderborg
  2017-11-24 13:27     ` Michal Hocko
  1 sibling, 1 reply; 8+ messages in thread
From: peter enderborg @ 2017-11-24 13:12 UTC (permalink / raw)
  To: Michal Hocko
  Cc: linux-doc, linux-kernel, linux-mm, linux-fsdevel,
	Jonathan Corbet, Luis R . Rodriguez, Kees Cook, Alex Deucher,
	David S . Miller, Harry Wentland, Greg Kroah-Hartman, Tony Cheng,
	David Rientjes, Andrew Morton, Jan Kara, Kirill A . Shutemov,
	Dave Jiang, Jérôme Glisse, Ross Zwisler,
	Matthew Wilcox, Hugh Dickins, Johannes Weiner, Kemi Wang,
	Vlastimil Babka, YASUAKI ISHIMATSU, Nikolay Borisov, Mel Gorman,
	Pavel Tatashin, Linux API

On 11/24/2017 11:14 AM, Michal Hocko wrote:
> On Fri 24-11-17 11:07:07, Peter Enderborg wrote:
>> When tuning the watermark_scale_factor to reduce stalls and compactions
>> the high mark is also changed, it changed a bit too much. So this
>> patch introduces a slope that can reduce this overhead a bit, or
>> increase it if needed.
> This doesn't explain what is the problem, why it is a problem and why we
> need yet another tuning to address it. Users shouldn't really care about
> internal stuff like watermark tuning for each watermark independently.
> This looks like a gross hack. Please start over with the problem
> description and then we can move on to an approapriate fix. Piling up
> tuning knobs to workaround problems is simply not acceptable.
>  

In the original patch - https://lkml.org/lkml/2016/2/18/498 - had a

discussion about small systems with 8GB RAM. In the handheld world, that's
a lot of RAM. However, the magic number 2 used in the present algorithm
is out of the blue. Compaction problems are the same for both small and
big. So small devices also need to increase watermark to
get compaction to work and reduce direct reclaims. Changing the low watermark
makes direct reclaim rate drop a lot. But it will cause kswap to work more,
and that has a negative impact. Lowering the gap will smooth out the kswap
workload to suite embedded devices a lot better. This can be addressed by
reducing the high watermark using the slope patch herein. Im sort of understand
your opinion on user knobs, but hard-coded magic numbers are even worse.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] mm:Add watermark slope for high mark
  2017-11-24 13:12   ` peter enderborg
@ 2017-11-24 13:27     ` Michal Hocko
  0 siblings, 0 replies; 8+ messages in thread
From: Michal Hocko @ 2017-11-24 13:27 UTC (permalink / raw)
  To: peter enderborg
  Cc: linux-doc, linux-kernel, linux-mm, linux-fsdevel,
	Jonathan Corbet, Luis R . Rodriguez, Kees Cook, Alex Deucher,
	David S . Miller, Harry Wentland, Greg Kroah-Hartman, Tony Cheng,
	David Rientjes, Andrew Morton, Jan Kara, Kirill A . Shutemov,
	Dave Jiang, Jérôme Glisse, Ross Zwisler,
	Matthew Wilcox, Hugh Dickins, Johannes Weiner, Kemi Wang,
	Vlastimil Babka, YASUAKI ISHIMATSU, Nikolay Borisov, Mel Gorman,
	Pavel Tatashin, Linux API

On Fri 24-11-17 14:12:56, peter enderborg wrote:
> On 11/24/2017 11:14 AM, Michal Hocko wrote:
> > On Fri 24-11-17 11:07:07, Peter Enderborg wrote:
> >> When tuning the watermark_scale_factor to reduce stalls and compactions
> >> the high mark is also changed, it changed a bit too much. So this
> >> patch introduces a slope that can reduce this overhead a bit, or
> >> increase it if needed.
> > This doesn't explain what is the problem, why it is a problem and why we
> > need yet another tuning to address it. Users shouldn't really care about
> > internal stuff like watermark tuning for each watermark independently.
> > This looks like a gross hack. Please start over with the problem
> > description and then we can move on to an approapriate fix. Piling up
> > tuning knobs to workaround problems is simply not acceptable.
> >  
> 
> In the original patch - https://lkml.org/lkml/2016/2/18/498 - had a
> 
> discussion about small systems with 8GB RAM. In the handheld world, that's
> a lot of RAM. However, the magic number 2 used in the present algorithm
> is out of the blue. Compaction problems are the same for both small and
> big. So small devices also need to increase watermark to
> get compaction to work and reduce direct reclaims. Changing the low watermark
> makes direct reclaim rate drop a lot. But it will cause kswap to work more,
> and that has a negative impact. Lowering the gap will smooth out the kswap
> workload to suite embedded devices a lot better. This can be addressed by
> reducing the high watermark using the slope patch herein. Im sort of understand
> your opinion on user knobs, but hard-coded magic numbers are even worse.

How can a poor user know how to tune it when _we_ cannot do a qualified
guess and we do know all the implementation details.

Really, describe problems you are seeing with the current code and we
can talk about a proper fix or a heuristic when the fix is
hard/unfeasible.
-- 
Michal Hocko
SUSE Labs

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] mm:Add watermark slope for high mark
  2017-11-24 10:07 [PATCH] mm:Add watermark slope for high mark Peter Enderborg
  2017-11-24 10:14 ` Michal Hocko
@ 2017-11-27  7:03 ` kbuild test robot
  2017-11-27  7:44 ` kbuild test robot
  2 siblings, 0 replies; 8+ messages in thread
From: kbuild test robot @ 2017-11-27  7:03 UTC (permalink / raw)
  To: Peter Enderborg
  Cc: kbuild-all, Michal Hocko, linux-doc, linux-kernel, linux-mm,
	linux-fsdevel, Jonathan Corbet, Luis R . Rodriguez, Kees Cook,
	Alex Deucher, David S . Miller, Harry Wentland,
	Greg Kroah-Hartman, Tony Cheng, David Rientjes, Peter Enderborg,
	Andrew Morton, Jan Kara, Kirill A . Shutemov, Dave Jiang,
	Jérôme Glisse, Ross Zwisler, Matthew Wilcox,
	Hugh Dickins, Johannes Weiner, Kemi Wang, Vlastimil Babka,
	YASUAKI ISHIMATSU, Nikolay Borisov, Mel Gorman, Pavel Tatashin

[-- Attachment #1: Type: text/plain, Size: 1069 bytes --]

Hi Peter,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on mmotm/master]
[also build test ERROR on v4.15-rc1 next-20171124]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Peter-Enderborg/mm-Add-watermark-slope-for-high-mark/20171127-140339
base:   git://git.cmpxchg.org/linux-mmotm.git master
config: i386-tinyconfig (attached as .config)
compiler: gcc-7 (Debian 7.2.0-12) 7.2.1 20171025
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   mm/page_alloc.o: In function `__setup_per_zone_wmarks':
>> page_alloc.c:(.text+0x9eb): undefined reference to `__umoddi3'
>> page_alloc.c:(.text+0xa06): undefined reference to `__udivdi3'
   page_alloc.c:(.text+0xa1d): undefined reference to `__udivdi3'

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 6802 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] mm:Add watermark slope for high mark
  2017-11-24 10:07 [PATCH] mm:Add watermark slope for high mark Peter Enderborg
  2017-11-24 10:14 ` Michal Hocko
  2017-11-27  7:03 ` kbuild test robot
@ 2017-11-27  7:44 ` kbuild test robot
  2 siblings, 0 replies; 8+ messages in thread
From: kbuild test robot @ 2017-11-27  7:44 UTC (permalink / raw)
  To: Peter Enderborg
  Cc: kbuild-all, Michal Hocko, linux-doc, linux-kernel, linux-mm,
	linux-fsdevel, Jonathan Corbet, Luis R . Rodriguez, Kees Cook,
	Alex Deucher, David S . Miller, Harry Wentland,
	Greg Kroah-Hartman, Tony Cheng, David Rientjes, Peter Enderborg,
	Andrew Morton, Jan Kara, Kirill A . Shutemov, Dave Jiang,
	Jérôme Glisse, Ross Zwisler, Matthew Wilcox,
	Hugh Dickins, Johannes Weiner, Kemi Wang, Vlastimil Babka,
	YASUAKI ISHIMATSU, Nikolay Borisov, Mel Gorman, Pavel Tatashin

[-- Attachment #1: Type: text/plain, Size: 3289 bytes --]

Hi Peter,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on mmotm/master]
[also build test ERROR on v4.15-rc1 next-20171127]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Peter-Enderborg/mm-Add-watermark-slope-for-high-mark/20171127-140339
base:   git://git.cmpxchg.org/linux-mmotm.git master
config: i386-randconfig-s0-201748 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   mm/page_alloc.o: In function `__setup_per_zone_wmarks':
>> mm/page_alloc.c:7031: undefined reference to `__umoddi3'
>> mm/page_alloc.c:7031: undefined reference to `__udivdi3'
>> mm/page_alloc.c:7031: undefined reference to `__udivdi3'

vim +7031 mm/page_alloc.c

  6977	
  6978	static void __setup_per_zone_wmarks(void)
  6979	{
  6980		unsigned long pages_min = min_free_kbytes >> (PAGE_SHIFT - 10);
  6981		unsigned long lowmem_pages = 0;
  6982		struct zone *zone;
  6983		unsigned long flags;
  6984	
  6985		/* Calculate total number of !ZONE_HIGHMEM pages */
  6986		for_each_zone(zone) {
  6987			if (!is_highmem(zone))
  6988				lowmem_pages += zone->managed_pages;
  6989		}
  6990	
  6991		for_each_zone(zone) {
  6992			u64 tmp;
  6993			u64 tmp_high;
  6994	
  6995			spin_lock_irqsave(&zone->lock, flags);
  6996			tmp = (u64)pages_min * zone->managed_pages;
  6997			do_div(tmp, lowmem_pages);
  6998			if (is_highmem(zone)) {
  6999				/*
  7000				 * __GFP_HIGH and PF_MEMALLOC allocations usually don't
  7001				 * need highmem pages, so cap pages_min to a small
  7002				 * value here.
  7003				 *
  7004				 * The WMARK_HIGH-WMARK_LOW and (WMARK_LOW-WMARK_MIN)
  7005				 * deltas control asynch page reclaim, and so should
  7006				 * not be capped for highmem.
  7007				 */
  7008				unsigned long min_pages;
  7009	
  7010				min_pages = zone->managed_pages / 1024;
  7011				min_pages = clamp(min_pages, SWAP_CLUSTER_MAX, 128UL);
  7012				zone->watermark[WMARK_MIN] = min_pages;
  7013			} else {
  7014				/*
  7015				 * If it's a lowmem zone, reserve a number of pages
  7016				 * proportionate to the zone's size.
  7017				 */
  7018				zone->watermark[WMARK_MIN] = tmp;
  7019			}
  7020	
  7021			/*
  7022			 * Set the kswapd watermarks distance according to the
  7023			 * scale factor in proportion to available memory, but
  7024			 * ensure a minimum size on small systems.
  7025			 */
  7026			tmp = max_t(u64, tmp >> 2,
  7027				    mult_frac(zone->managed_pages,
  7028					      watermark_scale_factor, 10000));
  7029	
  7030			zone->watermark[WMARK_LOW]  = min_wmark_pages(zone) + tmp;
> 7031			tmp_high = mult_frac(tmp, watermark_high_factor_slope, 100);
  7032			zone->watermark[WMARK_HIGH] = min_wmark_pages(zone) + tmp_high;
  7033	
  7034	
  7035			spin_unlock_irqrestore(&zone->lock, flags);
  7036		}
  7037	
  7038		/* update totalreserve_pages */
  7039		calculate_totalreserve_pages();
  7040	}
  7041	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 28242 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2017-11-27  7:44 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-24 10:07 [PATCH] mm:Add watermark slope for high mark Peter Enderborg
2017-11-24 10:14 ` Michal Hocko
2017-11-24 10:15   ` Vlastimil Babka
2017-11-24 10:34     ` peter enderborg
2017-11-24 13:12   ` peter enderborg
2017-11-24 13:27     ` Michal Hocko
2017-11-27  7:03 ` kbuild test robot
2017-11-27  7:44 ` kbuild test robot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).