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

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

[zbestahu]

Michal wrote:
> > 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.

it removes 3 below arithmetic instructions so it should be running faster.
add: scanned + reclaimed
mul: scale * reclaimed
udiv: reclaimed * scale /scanned

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

[Michal Hocko]

On Tue 04-07-17 21:43:39, zbestahu wrote:
> Michal wrote:
> > > 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.
> 
> it removes 3 below arithmetic instructions so it should be running faster.
> add: scanned + reclaimed
> mul: scale * reclaimed
> udiv: reclaimed * scale /scanned

That part is clear from the diff... What is not clear from the diff is
your motivation. This path is not hot (we are in the reclaim which is a
slow path) and few extra instructions are acceptable. Sure we can
optimize it if the resulting code is working as expected. What I am
asking (obviously unsuccessfully) is to describe _why_ it is better.
This is a requirement for _each patch_. We are not changing the code
"just because I like it more that way".
-- 
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: Re：[PATCH] mm: vmpressure: simplify pressure ratio calculation

[zbestahu]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="UTF-8", Size: 1728 bytes --]

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

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

diff --git a/mm/vmpressure.c b/mm/vmpressure.c
index 6063581..aad1fb2 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;
 
 	/*
@@ -123,13 +122,12 @@ static enum vmpressure_levels vmpressure_calc_level(unsigned long scanned,
 		goto out;
 	/*
 	 * We calculate the ratio (in percents) of how many pages were
-	 * scanned vs. reclaimed in a given time frame (window). Note that
-	 * time is in VM reclaimer's "ticks", i.e. number of pages
+	 * unsuccessful reclaimed to scanned in a given time frame (window).
+	 * Note that time is in VM reclaimer's "ticks", i.e. number of pages
 	 * 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.1N‹§²æìr¸›zǧu©ž²Æ {\b­†éì¹»\x1c®&Þ–)îÆi¢žØ^n‡r¶‰šŽŠÝ¢j$½§$¢¸\x05¢¹¨­è§~Š'.)îÄÃ,yèm¶ŸÿÃ\f%Š{±šj+ƒðèž×¦j)Z†·Ÿ