[PATCH] mm: simplify get_next_ra_size

[PATCH] mm: simplify get_next_ra_size

[Gao Xiang]

It's a trivial simplification for get_next_ra_size and
clear enough for humans to understand.

It also fixes potential overflow if ra->size(< ra_pages) is too large.

Cc: Fengguang Wu <fengguang.wu@intel.com>
Signed-off-by: Gao Xiang <hsiangkao@aol.com>
---
 mm/readahead.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/mm/readahead.c b/mm/readahead.c
index 4e63014..205ac34 100644
--- a/mm/readahead.c
+++ b/mm/readahead.c
@@ -272,17 +272,15 @@ static unsigned long get_init_ra_size(unsigned long size, unsigned long max)
  *  return it as the new window size.
  */
 static unsigned long get_next_ra_size(struct file_ra_state *ra,
-						unsigned long max)
+				      unsigned long max)
 {
 	unsigned long cur = ra->size;
-	unsigned long newsize;
 
 	if (cur < max / 16)
-		newsize = 4 * cur;
-	else
-		newsize = 2 * cur;
-
-	return min(newsize, max);
+		return 4 * cur;
+	if (cur <= max / 2)
+		return 2 * cur;
+	return max;
 }
 
 /*
-- 
2.7.4

Re: [PATCH] mm: simplify get_next_ra_size

[Fengguang Wu]

Looks good to me, thanks!

Reviewed-by: Fengguang Wu <fengguang.wu@intel.com>

On Sun, Oct 28, 2018 at 02:13:26PM +0800, Gao Xiang wrote:
>It's a trivial simplification for get_next_ra_size and
>clear enough for humans to understand.
>
>It also fixes potential overflow if ra->size(< ra_pages) is too large.
>
>Cc: Fengguang Wu <fengguang.wu@intel.com>
>Signed-off-by: Gao Xiang <hsiangkao@aol.com>
>---
> mm/readahead.c | 12 +++++-------
> 1 file changed, 5 insertions(+), 7 deletions(-)
>
>diff --git a/mm/readahead.c b/mm/readahead.c
>index 4e63014..205ac34 100644
>--- a/mm/readahead.c
>+++ b/mm/readahead.c
>@@ -272,17 +272,15 @@ static unsigned long get_init_ra_size(unsigned long size, unsigned long max)
>  *  return it as the new window size.
>  */
> static unsigned long get_next_ra_size(struct file_ra_state *ra,
>-						unsigned long max)
>+				      unsigned long max)
> {
> 	unsigned long cur = ra->size;
>-	unsigned long newsize;
>
> 	if (cur < max / 16)
>-		newsize = 4 * cur;
>-	else
>-		newsize = 2 * cur;
>-
>-	return min(newsize, max);
>+		return 4 * cur;
>+	if (cur <= max / 2)
>+		return 2 * cur;
>+	return max;
> }
>
> /*
>-- 
>2.7.4
>

Re: [PATCH] mm: simplify get_next_ra_size

[Gao Xiang]

Hi Fengguang,

On 2018/10/28 14:27, Fengguang Wu wrote:
> Looks good to me, thanks!
> 
> Reviewed-by: Fengguang Wu <fengguang.wu@intel.com>

Thanks for taking time and the quickly review. :)

Best regards,
Gao Xiang

> 
> On Sun, Oct 28, 2018 at 02:13:26PM +0800, Gao Xiang wrote:
>> It's a trivial simplification for get_next_ra_size and
>> clear enough for humans to understand.
>>
>> It also fixes potential overflow if ra->size(< ra_pages) is too large.
>>
>> Cc: Fengguang Wu <fengguang.wu@intel.com>
>> Signed-off-by: Gao Xiang <hsiangkao@aol.com>
>> --- 

Re: [PATCH] mm: simplify get_next_ra_size

[Matthew Wilcox]

On Sun, Oct 28, 2018 at 02:13:26PM +0800, Gao Xiang wrote:
> It's a trivial simplification for get_next_ra_size and
> clear enough for humans to understand.
> 
> It also fixes potential overflow if ra->size(< ra_pages) is too large.
> 
> Cc: Fengguang Wu <fengguang.wu@intel.com>
> Signed-off-by: Gao Xiang <hsiangkao@aol.com>

Reviewed-by: Matthew Wilcox <willy@infradead.org>

I also considered what would happen with underflow (passing a 'max'
less than 16, or less than 2) and it would seem to do the right thing
in that case.

Re: [PATCH] mm: simplify get_next_ra_size

[Gao Xiang]

Hi Matthew,

On 2018/10/28 18:23, Matthew Wilcox wrote:
> On Sun, Oct 28, 2018 at 02:13:26PM +0800, Gao Xiang wrote:
>> It's a trivial simplification for get_next_ra_size and
>> clear enough for humans to understand.
>>
>> It also fixes potential overflow if ra->size(< ra_pages) is too large.
>>
>> Cc: Fengguang Wu <fengguang.wu@intel.com>
>> Signed-off-by: Gao Xiang <hsiangkao@aol.com>
> 
> Reviewed-by: Matthew Wilcox <willy@infradead.org>
> 
> I also considered what would happen with underflow (passing a 'max'
> less than 16, or less than 2) and it would seem to do the right thing
> in that case.

Yeah, thanks for the review ;)

I also made a simple tester to test this in order to ensure its correctness
and the result shows the same behavior except for the overflowed case.

Thanks,
Gao Xiang

> 

Re: [PATCH] mm: simplify get_next_ra_size

[The Dellia’s]

Sent from my iPhone

Re: [PATCH] mm: simplify get_next_ra_size

[The Dellia’s]

Fuck off

Sent from my iPhone

> On Nov 2, 2020, at 11:01 AM, The Dellia’s <denisedellia@icloud.com> wrote:
> 
> 
> 
> 
> Sent from my iPhone