On 8/26/23 05:15, Mike Kravetz wrote:
On 08/25/23 12:02, Xueshi Hu wrote:
On 8/10/23 15:34, David Hildenbrand wrote:
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. :)
Considering /proc/sys/vm/nr_hugepages may be widely used, and it not total
equivalent to the interfaces under /sys. What about just clarifying the
docs?
Thanks for your patient reply.
I believe just updating the docs with clarification may be the best approach.
We need to say that /proc/sys/vm/nr_hugepages and sysctl (vm.nr_hugepages)
display the number of persistent hugetlb pages in the default pool. And, we
should probably define 'persistent' as well.
With that, this patch should be dropped. Without patch 1, patch 2 is not
necessary. However, some cleanup (possible elimination) of max_huge_pages
could be done in the future.
A modified version will sent later.
Patch 3 is documentation updates which we agree need to be performed. I can
assist here if you would like.
I would greatly appreciate your assistance.
Patch 4 is most critical as it is a bug fix. Perhaps this can/should be sent
separately.
Ok, I'll send it separately.
Thanks,
Hu
