| Message ID | 1540707206-19649-1-git-send-email-hsiangkao@aol.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mm: simplify get_next_ra_size | expand |
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 >
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> >> ---
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.
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 >
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; } /*
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(-)