Re: [PATCH] mm: vmpressure: simplify pressure ratio calculation

Re: [PATCH] mm: vmpressure: simplify pressure ratio calculation

[zbestahu]

[-- Attachment #1: Type: text/html, Size: 2709 bytes --]

[-- Attachment #2: Type: text/plain, Size: 2282 bytes --]

Hi Michal

We can think the some of scanned pages is  reclaimed as reclaimed pages and the rest of pages is just unsuccessful reclaimed pages. vmpressure is tend to unsuccessful reclaimed pages, so obviously the pressure percent is the ratio of unsuccessful reclaimed pages to scanned pages.

If my understanding is wrong, please correct  it.

Thanks.




-------- 原始邮件 --------
发件人：Michal Hocko <mhocko@kernel.org>
时间：周一 7月3日 15:44
收件人：zbestahu <zbestahu@aliyun.com>
抄送：akpm <akpm@linux-foundation.org>,minchan <minchan@kernel.org>,linux-mm <linux-mm@kvack.org>,Yue Hu <huyue2@coolpad.com>,Anton Vorontsov <anton.vorontsov@linaro.org>
主题：Re: [PATCH] mm: vmpressure: simplify pressure ratio calculation

>[CC Anton]
>
>On Sat 01-07-17 14:27:39, zbestahu@aliyun.com wrote:
>> From: Yue Hu <huyue2@coolpad.com>
>> 
>> The patch removes the needless scale in existing caluation, it
>> makes the calculation more simple and more effective.
>
>I suspect the construct is deliberate and done this way because of the
>rounding. Your code will behave slightly differently. If that is
>intentional then it should be described in the changedlog.
>
>> Signed-off-by: Yue Hu <huyue2@coolpad.com>
>> ---
>>  mm/vmpressure.c | 4 +---
>>  1 file changed, 1 insertion(+), 3 deletions(-)
>> 
>> diff --git a/mm/vmpressure.c b/mm/vmpressure.c
>> index 6063581..174b2f0 100644
>> --- a/mm/vmpressure.c
>> +++ b/mm/vmpressure.c
>> @@ -111,7 +111,6 @@ static enum vmpressure_levels vmpressure_level(unsigned long pressure)
>>  static enum vmpressure_levels vmpressure_calc_level(unsigned long scanned,
>>  						    unsigned long reclaimed)
>>  {
>> -	unsigned long scale = scanned + reclaimed;
>>  	unsigned long pressure = 0;
>>  
>>  	/*
>> @@ -128,8 +127,7 @@ static enum vmpressure_levels vmpressure_calc_level(unsigned long scanned,
>>  	 * scanned. This makes it possible to set desired reaction time
>>  	 * and serves as a ratelimit.
>>  	 */
>> -	pressure = scale - (reclaimed * scale / scanned);
>> -	pressure = pressure * 100 / scale;
>> +	pressure = (scanned - reclaimed) * 100 / scanned;
>>  
>>  out:
>>  	pr_debug("%s: %3lu  (s: %lu  r: %lu)\n", __func__, pressure,
>> -- 
>> 1.9.1
>> 
>
>-- 
>Michal Hocko
>SUSE Labs

Re: [PATCH] mm: vmpressure: simplify pressure ratio calculation

[Michal Hocko]

On Mon 03-07-17 17:45:25, zbestahu@aliyun.com wrote:
> Hi Michal
> 
> We can think the some of scanned pages is reclaimed as reclaimed pages
> and the rest of pages is just unsuccessful reclaimed pages. vmpressure
> is tend to unsuccessful reclaimed pages, so obviously the pressure
> percent is the ratio of unsuccessful reclaimed pages to scanned pages.

Yes this is correct and this is what the current code does as well.
The difference is in the rounding when the integer arithmetic is used.

Btw. are you trying to fix any existing problem or you merely checked
the code and considered this part too hard to understand and so you sent
a patch to make it simpler? Have you considered the original intention
of the code? Note that I am not saying your patch is incorrect I would
just like to uderstand your motivation and the original intention in the
code.

> -------- a??a??e?(R)a>>? --------
> a??a>>?aooi 1/4 ?Michal Hocko <mhocko@kernel.org>
> ae??e?'i 1/4 ?a??a,? 7ae??3ae?JPY 15:44
> ae??a>>?aooi 1/4 ?zbestahu <zbestahu@aliyun.com>
> ae??e??i 1/4 ?akpm <akpm@linux-foundation.org>,minchan <minchan@kernel.org>,linux-mm <linux-mm@kvack.org>,Yue Hu <huyue2@coolpad.com>,Anton Vorontsov <anton.vorontsov@linaro.org>
> a,>>ec?i 1/4 ?Re: [PATCH] mm: vmpressure: simplify pressure ratio calculation
> 
> >[CC Anton]
> >
> >On Sat 01-07-17 14:27:39, zbestahu@aliyun.com wrote:
> >> From: Yue Hu <huyue2@coolpad.com>
> >> 
> >> The patch removes the needless scale in existing caluation, it
> >> makes the calculation more simple and more effective.
> >
> >I suspect the construct is deliberate and done this way because of the
> >rounding. Your code will behave slightly differently. If that is
> >intentional then it should be described in the changedlog.
> >
> >> Signed-off-by: Yue Hu <huyue2@coolpad.com>
> >> ---
> >>A  mm/vmpressure.c | 4 +---
> >>A  1 file changed, 1 insertion(+), 3 deletions(-)
> >> 
> >> diff --git a/mm/vmpressure.c b/mm/vmpressure.c
> >> index 6063581..174b2f0 100644
> >> --- a/mm/vmpressure.c
> >> +++ b/mm/vmpressure.c
> >> @@ -111,7 +111,6 @@ static enum vmpressure_levels vmpressure_level(unsigned long pressure)
> >>A  static enum vmpressure_levels vmpressure_calc_level(unsigned long scanned,
> >>A  						A A A  unsigned long reclaimed)
> >>A  {
> >> -	unsigned long scale = scanned + reclaimed;
> >>A  	unsigned long pressure = 0;
> >>A  
> >>A  	/*
> >> @@ -128,8 +127,7 @@ static enum vmpressure_levels vmpressure_calc_level(unsigned long scanned,
> >>A  	 * scanned. This makes it possible to set desired reaction time
> >>A  	 * and serves as a ratelimit.
> >>A  	 */
> >> -	pressure = scale - (reclaimed * scale / scanned);
> >> -	pressure = pressure * 100 / scale;
> >> +	pressure = (scanned - reclaimed) * 100 / scanned;
> >>A  
> >>A  out:
> >>A  	pr_debug("%s: %3luA  (s: %luA  r: %lu)\n", __func__, pressure,
> >> -- 
> >> 1.9.1
> >> 
> >
> >-- 
> >Michal Hocko
> >SUSE Labs

-- 
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>

回复：[PATCH] mm: vmpressure: simplify pressure ratio calculation

[zbestahu]

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

Yes, the original code using scale should be about rounding to integer. I am trying to improve the calculation because i think the rounding seems to be useless, we can calculate pressure directly just like original code of "pressure = pressure * 100 / scale", no floating number issue.From the view of disassembly, the patch is also better than original.
If original code using scale is more powerful than the patch, please ignore the submit.
Thanks.------------------------------------------------------------------发件人：Michal Hocko <mhocko@kernel.org>发送时间：2017年7月3日(星期一) 18:24收件人：zbestahu@aliyun.com <zbestahu@aliyun.com>抄　送：akpm <akpm@linux-foundation.org>; minchan <minchan@kernel.org>; linux-mm <linux-mm@kvack.org>; Yue Hu <huyue2@coolpad.com>; Anton Vorontsov <anton.vorontsov@linaro.org>主　题：Re: [PATCH] mm: vmpressure: simplify pressure ratio calculation
On Mon 03-07-17 17:45:25, zbestahu@aliyun.com wrote:
> Hi Michal
> 
> We can think the some of scanned pages is reclaimed as reclaimed pages
> and the rest of pages is just unsuccessful reclaimed pages. vmpressure
> is tend to unsuccessful reclaimed pages, so obviously the pressure
> percent is the ratio of unsuccessful reclaimed pages to scanned pages.

Yes this is correct and this is what the current code does as well.
The difference is in the rounding when the integer arithmetic is used.

Btw. are you trying to fix any existing problem or you merely checked
the code and considered this part too hard to understand and so you sent
a patch to make it simpler? Have you considered the original intention
of the code? Note that I am not saying your patch is incorrect I would
just like to uderstand your motivation and the original intention in the
code.

> -------- 原始邮件 --------
> 发件人：Michal Hocko <mhocko@kernel.org>
> 时间：周一 7月3日 15:44
> 收件人：zbestahu <zbestahu@aliyun.com>
> 抄送：akpm <akpm@linux-foundation.org>,minchan <minchan@kernel.org>,linux-mm <linux-mm@kvack.org>,Yue Hu <huyue2@coolpad.com>,Anton Vorontsov <anton.vorontsov@linaro.org>
> 主题：Re: [PATCH] mm: vmpressure: simplify pressure ratio calculation
> 
> >[CC Anton]
> >
> >On Sat 01-07-17 14:27:39, zbestahu@aliyun.com wrote:
> >> From: Yue Hu <huyue2@coolpad.com>
> >> 
> >> The patch removes the needless scale in existing caluation, it
> >> makes the calculation more simple and more effective.
> >
> >I suspect the construct is deliberate and done this way because of the
> >rounding. Your code will behave slightly differently. If that is
> >intentional then it should be described in the changedlog.
> >
> >> Signed-off-by: Yue Hu <huyue2@coolpad.com>
> >> ---
> >>  mm/vmpressure.c | 4 +---
> >>  1 file changed, 1 insertion(+), 3 deletions(-)
> >> 
> >> diff --git a/mm/vmpressure.c b/mm/vmpressure.c
> >> index 6063581..174b2f0 100644
> >> --- a/mm/vmpressure.c
> >> +++ b/mm/vmpressure.c
> >> @@ -111,7 +111,6 @@ static enum vmpressure_levels vmpressure_level(unsigned long pressure)
> >>  static enum vmpressure_levels vmpressure_calc_level(unsigned long scanned,
> >>            unsigned long reclaimed)
> >>  {
> >> - unsigned long scale = scanned + reclaimed;
> >>   unsigned long pressure = 0;
> >>  
> >>   /*
> >> @@ -128,8 +127,7 @@ static enum vmpressure_levels vmpressure_calc_level(unsigned long scanned,
> >>    * scanned. This makes it possible to set desired reaction time
> >>    * and serves as a ratelimit.
> >>    */
> >> - pressure = scale - (reclaimed * scale / scanned);
> >> - pressure = pressure * 100 / scale;
> >> + pressure = (scanned - reclaimed) * 100 / scanned;
> >>  
> >>  out:
> >>   pr_debug("%s: %3lu  (s: %lu  r: %lu)\n", __func__, pressure,
> >> -- 
> >> 1.9.1
> >> 
> >
> >-- 
> >Michal Hocko
> >SUSE Labs

-- 
Michal Hocko
SUSE Labs

[-- Attachment #2: Type: text/html, Size: 8617 bytes --]

Re: 回复：[PATCH] mm: vmpressure: simplify pressure ratio calculation

[Michal Hocko]

Please do not top post.

On Mon 03-07-17 22:19:02, zbestahu wrote:
> Yes, the original code using scale should be about rounding to
> integer. I am trying to improve the calculation because i think the
> rounding seems to be useless, we can calculate pressure directly just
> like original code of "pressure = pressure * 100 / scale", no
> floating number issue.From the view of disassembly, the patch is also
> better than original.  If original code using scale is more powerful
> than the patch, please ignore the submit.

Make sure you describe all that in the changelog because your original
patch description wasn't all that clear about your intention.
-- 
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>

Re: 回复：[PATCH] mm: vmpressure: simplify pressure ratio calculation

[zbestahu]

Michal wrote: 
> Make sure you describe all that in the changelog because your original
> patch description wasn't all that clear about your intention.

The patch's description is updated as following:

The patch does not change the function, the existing percent
calculation using scale should be about rounding to intege, it
seems to be redundant, we can calculate it directly just like
"pressure = not_relaimed * 100 / scanned", no rounding issue. And
it's also better because of saving several arithmetic operations.

Thanks.

Re: 回复：[PATCH] mm: vmpressure: simplify pressure ratio calculation

[Michal Hocko]

On Tue 04-07-17 21:08:15, zbestahu wrote:
> Michal wrote: 
> > Make sure you describe all that in the changelog because your original
> > patch description wasn't all that clear about your intention.
> 
> The patch's description is updated as following:
> 
> The patch does not change the function,

yes it pretty much changes the result.

> the existing percent
> calculation using scale should be about rounding to intege, it
> seems to be redundant, we can calculate it directly just like
> "pressure = not_relaimed * 100 / scanned", no rounding issue. And
> it's also better because of saving several arithmetic operations.

and you haven't explained why that change is so much better to change
the behavior.
-- 
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>

Re: [PATCH] mm: vmpressure: simplify pressure ratio calculation

[zbestahu]

2017-07-05 18:00 Anshuman Khandual <khandual@linux.vnet.ibm.com> wrote:
> On 07/01/2017 11:43 AM, zbestahu@aliyun.com wrote:
> > From: Yue Hu <huyue2@coolpad.com>
> > 
> > The patch removes the needless scale in existing caluation, it
> > makes the calculation more simple and more effective.
> > 

> Could you please explain how the new calculation is better
> than the old one ?

Already discussed with Michal in linux-mm,  please ignore the patch, the original code has no problem currently.

Re: [PATCH] mm: vmpressure: simplify pressure ratio calculation

[Anshuman Khandual]

On 07/01/2017 11:43 AM, zbestahu@aliyun.com wrote:
> From: Yue Hu <huyue2@coolpad.com>
> 
> The patch removes the needless scale in existing caluation, it
> makes the calculation more simple and more effective.
> 

Could you please explain how the new calculation is better
than the old one ?

--
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>

Re: [PATCH] mm: vmpressure: simplify pressure ratio calculation

[Michal Hocko]

[CC Anton]

On Sat 01-07-17 14:27:39, zbestahu@aliyun.com wrote:
> From: Yue Hu <huyue2@coolpad.com>
> 
> The patch removes the needless scale in existing caluation, it
> makes the calculation more simple and more effective.

I suspect the construct is deliberate and done this way because of the
rounding. Your code will behave slightly differently. If that is
intentional then it should be described in the changedlog.

> Signed-off-by: Yue Hu <huyue2@coolpad.com>
> ---
>  mm/vmpressure.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/mm/vmpressure.c b/mm/vmpressure.c
> index 6063581..174b2f0 100644
> --- a/mm/vmpressure.c
> +++ b/mm/vmpressure.c
> @@ -111,7 +111,6 @@ static enum vmpressure_levels vmpressure_level(unsigned long pressure)
>  static enum vmpressure_levels vmpressure_calc_level(unsigned long scanned,
>  						    unsigned long reclaimed)
>  {
> -	unsigned long scale = scanned + reclaimed;
>  	unsigned long pressure = 0;
>  
>  	/*
> @@ -128,8 +127,7 @@ static enum vmpressure_levels vmpressure_calc_level(unsigned long scanned,
>  	 * scanned. This makes it possible to set desired reaction time
>  	 * and serves as a ratelimit.
>  	 */
> -	pressure = scale - (reclaimed * scale / scanned);
> -	pressure = pressure * 100 / scale;
> +	pressure = (scanned - reclaimed) * 100 / scanned;
>  
>  out:
>  	pr_debug("%s: %3lu  (s: %lu  r: %lu)\n", __func__, pressure,
> -- 
> 1.9.1
> 

-- 
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>

[PATCH] mm: vmpressure: simplify pressure ratio calculation

[zbestahu]

From: Yue Hu <huyue2@coolpad.com>

The patch removes the needless scale in existing caluation, it
makes the calculation more simple and more effective.

Signed-off-by: Yue Hu <huyue2@coolpad.com>
---
 mm/vmpressure.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/mm/vmpressure.c b/mm/vmpressure.c
index 6063581..174b2f0 100644
--- a/mm/vmpressure.c
+++ b/mm/vmpressure.c
@@ -111,7 +111,6 @@ static enum vmpressure_levels vmpressure_level(unsigned long pressure)
 static enum vmpressure_levels vmpressure_calc_level(unsigned long scanned,
 						    unsigned long reclaimed)
 {
-	unsigned long scale = scanned + reclaimed;
 	unsigned long pressure = 0;
 
 	/*
@@ -128,8 +127,7 @@ static enum vmpressure_levels vmpressure_calc_level(unsigned long scanned,
 	 * scanned. This makes it possible to set desired reaction time
 	 * and serves as a ratelimit.
 	 */
-	pressure = scale - (reclaimed * scale / scanned);
-	pressure = pressure * 100 / scale;
+	pressure = (scanned - reclaimed) * 100 / scanned;
 
 out:
 	pr_debug("%s: %3lu  (s: %lu  r: %lu)\n", __func__, pressure,
-- 
1.9.1

--
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>

[PATCH] mm: vmpressure: simplify pressure ratio calculation

[zbestahu]

From: Yue Hu <huyue2@coolpad.com>

The patch removes the needless scale in existing caluation, it
makes the calculation more simple and more effective.

Signed-off-by: Yue Hu <huyue2@coolpad.com>
---
 mm/vmpressure.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/mm/vmpressure.c b/mm/vmpressure.c
index 6063581..174b2f0 100644
--- a/mm/vmpressure.c
+++ b/mm/vmpressure.c
@@ -111,7 +111,6 @@ static enum vmpressure_levels vmpressure_level(unsigned long pressure)
 static enum vmpressure_levels vmpressure_calc_level(unsigned long scanned,
 						    unsigned long reclaimed)
 {
-	unsigned long scale = scanned + reclaimed;
 	unsigned long pressure = 0;
 
 	/*
@@ -128,8 +127,7 @@ static enum vmpressure_levels vmpressure_calc_level(unsigned long scanned,
 	 * scanned. This makes it possible to set desired reaction time
 	 * and serves as a ratelimit.
 	 */
-	pressure = scale - (reclaimed * scale / scanned);
-	pressure = pressure * 100 / scale;
+	pressure = (scanned - reclaimed) * 100 / scanned;
 
 out:
 	pr_debug("%s: %3lu  (s: %lu  r: %lu)\n", __func__, pressure,
-- 
1.9.1

--
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>