Re: [PATCH 1/3] mm/hugetlb: fix the inconsistency of /proc/sys/vm/nr_huge_pages

[Mike Kravetz <mike.kravetz@oracle.com>]

On 07/30/23 20:51, Xueshi Hu wrote:
> When writing to /proc/sys/vm/nr_huge_pages, it indicates global number of
> huge pages of the default hstate. But when reading from it, it indicates
> the current number of "persistent" huge pages in the kernel's huge page
> pool.
> 
> There are currently four interfaces used to export the number of huge
> pages:
> - /proc/meminfo
> - /proc/sys/vm/*hugepages*
> - /sys/devices/system/node/node0/hugepages/hugepages-2048kB/*
> - /sys/kernel/mm/hugepages/hugepages-2048kB/*
> 
> But only the /proc/sys/vm/nr_huge_pages provides the 'persistent'
> semantics when reading from it. This inconsistency is very subtle and can
> be easily misunderstood.

Thanks for looking into this.

The hugetlb documentation (./admin-guide/mm/hugetlbpage.rst) mentions
the term 'persistent hugetlb pages', but never provides a definition.

We can get the definition from the code as:
#define persistent_huge_pages(h) (h->nr_huge_pages - h->surplus_huge_pages)

Further, the documentation says:
"The ``/proc/meminfo`` file provides information about the total number of
 persistent hugetlb pages in the kernel's huge page pool."

"``/proc/sys/vm/nr_hugepages`` indicates the current number of "persistent"
 huge pages in the kernel's huge page pool."

"The administrator may shrink the pool of persistent huge pages for
 the default huge page size by setting the ``nr_hugepages`` sysctl to a
 smaller value."

So, the documentation implies that these interfaces should display the
number of persistent hugetlb pages.  As you have discovered, all but the
sysctl interface (and /proc/sys/vm/nr_hugepages) displays the total
number of hugetlb pages rather than the number of persistent hugetlb
pages.

If we wanted to match the documentation, it seems we should change all
the "show" interfaces to display persistent huge pages.  However, I am a
bit concerned about how this may impact end users.

There are two types if inconsistencies in these interfaces.
1) As this patch points out, not all "show" interfaces provide the same
   information.  sysctl (/proc/sys/vm/nr_hugepages) displays the number
   of persistent hugetlb pages, while the others display the total number
   of hugetlb pages.
2) The show/read interfaces generally provide the total number of
   hugetlb pages, and the update/write interfaces update the number of
   persistent hugetlb pages.

Both of these situations can lead to user confusion.  My 'guess' is that
this has not been a widespread issue as most hugetlb users do not
configure overcommit/surplus hugetlb pages and thus total number of
hugetlb pages is the same as number of persistent hugetlb pages.

Right now, I would suggest making all these interfaces display/take the
number of persistent hugetlb pages for consistency.  This also matches
the documentation.

Thoughts?
-- 
Mike Kravetz

> 
> Signed-off-by: Xueshi Hu <xueshi.hu@smartx.com>
> ---
>  mm/hugetlb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index e327a5a7602c..76af189053f0 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -4606,7 +4606,7 @@ static int hugetlb_sysctl_handler_common(bool obey_mempolicy,
>  			 void *buffer, size_t *length, loff_t *ppos)
>  {
>  	struct hstate *h = &default_hstate;
> -	unsigned long tmp = h->max_huge_pages;
> +	unsigned long tmp = h->nr_huge_pages;
>  	int ret;
>  
>  	if (!hugepages_supported())
> -- 
> 2.40.1
> 
>