mm/page_alloc: fix documentation error and remove magic numbers
Subsystems (2)
PATCH v2 1/1 mm/page_alloc: fix documentation error and remove magic numbers
When I increased the upper bound of the min_free_kbytes value in ee8eb9a5fe863, I forgot to tweak the above comment to reflect the new value. This patch fixes that mistake.
In addition, this patch replaces the magic number bounds with symbolic constants to clarify the logic.
changes from v1:
- declare constants via enum instead of separate integers
--- mm/page_alloc.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
mm/page_alloc.c +7 -5
@@ -7832,7 +7832,7 @@ void setup_per_zone_wmarks(void)* Initialise min_free_kbytes. * * For small machines we want it small (128k min). For large machines -* we want it large (64MB max). But it is not linear, because network +* we want it large (256MB max). But it is not linear, because network * bandwidth does not increase linearly with machine size. We use * * min_free_kbytes = 4 * sqrt(lowmem_kbytes), for better accuracy:
@@ -7852,6 +7852,8 @@ void setup_per_zone_wmarks(void)* 8192MB: 11584k * 16384MB: 16384k */ +static enum { MIN_FREE_KBYTES_LOWER_BOUND = 1 << 7, MIN_FREE_KBYTES_UPPER_BOUND = 1 << 18 }; +int __meminit init_per_zone_wmark_min (void ) {unsigned long lowmem_kbytes;
@@ -7862,10 +7864,10 @@ int __meminit init_per_zone_wmark_min(void)if (new_min_free_kbytes > user_min_free_kbytes) { min_free_kbytes = new_min_free_kbytes; -if (min_free_kbytes <128 ) - min_free_kbytes =128 ; -if (min_free_kbytes >262144 ) - min_free_kbytes =262144 ; +if (min_free_kbytes <MIN_FREE_KBYTES_LOWER_BOUND ) + min_free_kbytes =MIN_FREE_KBYTES_LOWER_BOUND ; +if (min_free_kbytes >MIN_FREE_KBYTES_UPPER_BOUND ) + min_free_kbytes =MIN_FREE_KBYTES_UPPER_BOUND ; }else {pr_warn ("min_free_kbytes is not updated to %d because user defined value %d is preferred\n" , new_min_free_kbytes, user_min_free_kbytes);
Signature · 1 line
2.23.0
Quoted 7 lines · Joel Savitz
When I increased the upper bound of the
min_free_kbytesvalue inee8eb9a5fe863, I forgot to tweak the above comment to reflect the new value. This patch fixes that mistake.In addition, this patch replaces the magic number bounds with symbolic constants to clarify the logic.
Quoted 6 lines
changes from v1:
- declare constants via enum instead of separate integersSuggested-by John Hubbard <[email protected]>Suggested-by Vlastimil Babka <[email protected]>Signed-off-by Joel Savitz <[email protected]>
NAK. Just fix the documentation. It's bad form to combine two completely different things in one patch anyway.
Andrew Morton
maintainerQuoted 11 lines · On Wed, 24 Jun 2020 18:01:31 +0100 Matthew Wilcox <[email protected]> wrote:
Quoted 10 lines · Joel Savitz
When I increased the upper bound of the
min_free_kbytesvalue inee8eb9a5fe863, I forgot to tweak the above comment to reflect the new value. This patch fixes that mistake.In addition, this patch replaces the magic number bounds with symbolic constants to clarify the logic.
changes from v1:
- declare constants via enum instead of separate integers
Quoted 6 lines
Quoted 3 lines
Suggested-by John Hubbard <[email protected]>Suggested-by Vlastimil Babka <[email protected]>Signed-off-by Joel Savitz <[email protected]>NAK. Just fix the documentation. It's bad form to combine two completely different things in one patch anyway.
I don't think the enums add value anyway - if these numbers are only used in a single place, it's clearer to just open-code them.
One advantage of using a #define/enum for single-use constants is that the #define/enum site provides a neat location for documenting (commenting) the reasoning behind the particular values which were chosen. But this patch didn't do that.