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

[David Hildenbrand <david@redhat.com>]

On 10.08.23 02:17, Mike Kravetz wrote:
> On 08/08/23 17:13, Xueshi Hu wrote:
>> On 8/8/23 15:58, David Hildenbrand wrote:
>>> On 08.08.23 04:28, Xueshi Hu wrote:
>>>> On 8/7/23 23:15, David Hildenbrand wrote:
>>>>> On 06.08.23 09:48, Xueshi Hu wrote:
> 
> Sorry for jumping in late, I was away for a while.
> 
> Hu and myself discussed this previously in,
> https://lore.kernel.org/linux-mm/20230802182031.GA4762@monkey/T/#r1bdc8eeebafa08699fda5b15f247f3f966ddd090
> 
> The documentation around what is displayed with the hugetlb proc/sys
> interfaces is at best confusing and at worst wrong in places.
> 
> One source of confusion is use of term 'persistent hugetlb pages'.  The
> documentation does not define this term.  However, there is this
> definition in the code:
> #define persistent_huge_pages(h) (h->nr_huge_pages - h->surplus_huge_pages)
> 
> All of the write/update interfaces modify the number of persistent hugetlb
> pages as defined by the code (#define).  Only one read/show interface
> displays the number of persistent hugetlb pages as defined by the code
> (#define).  That is /proc/sys/vm/nr_hugepages (and sysctl).

Yes.

> 
> When thinking about this more, I am 'guessing' that when the documentation was
> originally written the term 'persistent hugetlb pages' did not refer to the
> #define in the code.  Rather, it was just the number of allocated hugetlb pages
> that 'persisted' until modified by the admin/user.
> 
> There is little doubt the documentation could/should be updated.

Absolutely.

> 
> The question is 'Should we change the /proc/sys/vm/nr_hugepages (and sysctl)
> interfaces to be consistent with all the other read/show interfaces?
> 
> The argument for changing is that consistency is good.  Why have one interface
> that is not like the others?
> 
> The reason for not changing is that this is the oldest interface.  The
> information/interfaces originally available in /proc were created in /sys.
> And, as mentioned in the documentation the /proc interfaces were kept
> for backward compatibility.  Unfortunately, the meaning of nr_hugepages
> was changed the /sys interfaces were created.  Sigh!!!

Indeed, they were designed to be different and to just leave the /proc 
interface alone.

> 
> In the thread mentioned above, I was in agreement with Hu about changing
> /proc/sys/vm/nr_hugepages to be consistent with other read/show interfaces.
> Now, I am not sure.

My take would be to just leave /proc/sys/vm/nr_hugepages alone. Maybe 
pr_warn_once() when the interface is used to guide people away from that 
legacy interface + clarify the docs.

Your call. :)

-- 
Cheers,

David / dhildenb