Re: [PATCH] mm: print the promo watermark in zoneinfo

[Andrew Morton <akpm@linux-foundation.org>]

On Thu,  1 Aug 2024 18:22:03 +0000 kaiyang2@cs.cmu.edu wrote:

> From: Kaiyang Zhao <kaiyang2@cs.cmu.edu>
> 
> Printing the promo watermark in zoneinfo just like other watermarks.
> This helps users check and verify all the watermarks are appropriate.
> 
> ...
>
> --- a/mm/vmstat.c
> +++ b/mm/vmstat.c
> @@ -1725,6 +1725,7 @@ static void zoneinfo_show_print(struct seq_file *m, pg_data_t *pgdat,
>  		   "\n        min      %lu"
>  		   "\n        low      %lu"
>  		   "\n        high     %lu"
> +		   "\n        promo    %lu"
>  		   "\n        spanned  %lu"
>  		   "\n        present  %lu"
>  		   "\n        managed  %lu"
> @@ -1734,6 +1735,7 @@ static void zoneinfo_show_print(struct seq_file *m, pg_data_t *pgdat,
>  		   min_wmark_pages(zone),
>  		   low_wmark_pages(zone),
>  		   high_wmark_pages(zone),
> +		   wmark_pages(zone, WMARK_PROMO),
>  		   zone->spanned_pages,
>  		   zone->present_pages,
>  		   zone_managed_pages(zone),

Maybe.  I'll let others opine about the desirability of this.

But you made me look at include/linux/mmzone.h

#define min_wmark_pages(z) (z->_watermark[WMARK_MIN] + z->watermark_boost)
#define low_wmark_pages(z) (z->_watermark[WMARK_LOW] + z->watermark_boost)
#define high_wmark_pages(z) (z->_watermark[WMARK_HIGH] + z->watermark_boost)
#define wmark_pages(z, i) (z->_watermark[i] + z->watermark_boost)

a) how dumb is it that we open-code wmark_pages() three times and
   then define it!

b) For symmetry at least, I guess your patch should add and use
   promo_wmark_pages().

c) all the above could (and hence should) be static inline C functions.