Date:   Thu, 16 Dec 2021 14:16:47 +0100
From:   Greg KH <gregkh@linuxfoundation.org>
To:     Qi Zheng <zhengqi.arch@bytedance.com>
Cc:     akpm@linux-foundation.org, rafael@kernel.org, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org, songmuchun@bytedance.com
Subject: Re: [PATCH 1/2] mm: add MemAvailable to per-node meminfo
Message-ID: <Ybs8P3WcPtShi15o@kroah.com>
References: <20211216124655.32247-1-zhengqi.arch@bytedance.com>
 <20211216124655.32247-2-zhengqi.arch@bytedance.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20211216124655.32247-2-zhengqi.arch@bytedance.com>
Precedence: bulk

On Thu, Dec 16, 2021 at 08:46:54PM +0800, Qi Zheng wrote:
> In /proc/meminfo, we can show the sum of all the available memory
> as "MemAvailable". Add the same counter also to per-node meminfo
> under /sys.
> 
> With this counter, some processes that bind nodes can make some
> decisions by reading the "MemAvailable" of the corresponding nodes
> directly.
> 
> Signed-off-by: Qi Zheng <zhengqi.arch@bytedance.com>
> ---
>  drivers/base/node.c    |  4 ++++
>  include/linux/mm.h     |  1 +
>  include/linux/mmzone.h |  5 +++++
>  mm/page_alloc.c        | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 59 insertions(+)
> 
> diff --git a/drivers/base/node.c b/drivers/base/node.c
> index 87acc47e8951..deb2a7965ae4 100644
> --- a/drivers/base/node.c
> +++ b/drivers/base/node.c
> @@ -375,8 +375,10 @@ static ssize_t node_read_meminfo(struct device *dev,
>  	struct sysinfo i;
>  	unsigned long sreclaimable, sunreclaimable;
>  	unsigned long swapcached = 0;
> +	long available;
>  
>  	si_meminfo_node(&i, nid);
> +	available = si_mem_available_node(&i, nid);
>  	sreclaimable = node_page_state_pages(pgdat, NR_SLAB_RECLAIMABLE_B);
>  	sunreclaimable = node_page_state_pages(pgdat, NR_SLAB_UNRECLAIMABLE_B);
>  #ifdef CONFIG_SWAP
> @@ -386,6 +388,7 @@ static ssize_t node_read_meminfo(struct device *dev,
>  			    "Node %d MemTotal:       %8lu kB\n"
>  			    "Node %d MemFree:        %8lu kB\n"
>  			    "Node %d MemUsed:        %8lu kB\n"
> +			    "Node %d MemAvailable:   %8lu kB\n"

You just changed a user/kernel api without documenting it anywhere, or
ensuring that you did not just break anything.

Also, this api is crazy, and not ok, please never add anything new to
it, it is broken as-is.

thanks,

greg k-h