On Tue, Jun 23, 2020 at 11:27:12PM -0400, Joel Savitz wrote:
> In addition, this patch replaces the magic number bounds with symbolic
> constants to clarify the logic.
Why do people think this kind of thing makes the code easier to read?
It actually makes it harder. Unless the constants are used in more
than one place, just leave the numbers where they are.
> @@ -7852,6 +7852,9 @@ void setup_per_zone_wmarks(void)
> * 8192MB: 11584k
> * 16384MB: 16384k
> */
> +static const int MIN_FREE_KBYTES_LOWER_BOUND = 1 << 7;
> +static const int MIN_FREE_KBYTES_UPPER_BOUND = 1 << 18;
> +
> int __meminit init_per_zone_wmark_min(void)
> {
> unsigned long lowmem_kbytes;
> @@ -7862,10 +7865,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;
The only thing I'd consider changing there is replacing 262144 with 256
* 1024. 1 << 18 is not clearer!
