Re: [PATCH v2] mm, hugetlb: skip irrelevant nodes in show_free_areas()

Muchun Song
2022-07-05T05:21:04Z

On Tue, Jul 5, 2022 at 11:49 AM Gang Li <ligang.bdlg@bytedance.com> wrote:
>
> show_free_areas() allows to filter out node specific data which is
> irrelevant to the allocation request. But hugetlb_show_meminfo() still
> show hugetlb on all nodes, which is redundant and unnecessary.

s/show/shows/g

>
> Use show_mem_node_skip() to skip irrelevant nodes. And replace
> hugetlb_show_meminfo() with hugetlb_show_meminfo_node(nid).
>
> Signed-off-by: Gang Li <ligang.bdlg@bytedance.com>
> ---
> v2: replace hugetlb_show_meminfo() with hugetlb_show_meminfo_node(nid) to avoid
> exporting show_mem_node_skip.
> ---
> include/linux/hugetlb.h | 4 ++--
> mm/hugetlb.c | 21 ++++++++-------------
> mm/page_alloc.c | 10 ++++++++--
> 3 files changed, 18 insertions(+), 17 deletions(-)
>
> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
> index 29c4d0883d36..21795e62398b 100644
> --- a/include/linux/hugetlb.h
> +++ b/include/linux/hugetlb.h
> @@ -155,7 +155,7 @@ void __unmap_hugepage_range_final(struct mmu_gather *tlb,
> struct page *ref_page, zap_flags_t zap_flags);
> void hugetlb_report_meminfo(struct seq_file *);
> int hugetlb_report_node_meminfo(char *buf, int len, int nid);
> -void hugetlb_show_meminfo(void);
> +void hugetlb_show_meminfo_node(int nid);
> unsigned long hugetlb_total_pages(void);
> vm_fault_t hugetlb_fault(struct mm_struct *mm, struct vm_area_struct *vma,
> unsigned long address, unsigned int flags);
> @@ -301,7 +301,7 @@ static inline int hugetlb_report_node_meminfo(char *buf, int len, int nid)
> return 0;
> }
>
> -static inline void hugetlb_show_meminfo(void)
> +static inline void hugetlb_show_meminfo_node(int nid)
> {
> }
>
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index ca081078e814..c87049c4126e 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -4490,22 +4490,17 @@ int hugetlb_report_node_meminfo(char *buf, int len, int nid)
> nid, h->surplus_huge_pages_node[nid]);
> }
>
> -void hugetlb_show_meminfo(void)
> +void hugetlb_show_meminfo_node(int nid)
> {
> struct hstate *h;
> - int nid;
>
> - if (!hugepages_supported())
> - return;

It is better not to delete this, I know you want to lower the overhead
for users when
!hugepages_supported(). However, it is not a common case and not a hot path.

> -
> - for_each_node_state(nid, N_MEMORY)
> - for_each_hstate(h)
> - pr_info("Node %d hugepages_total=%u hugepages_free=%u hugepages_surp=%u hugepages_size=%lukB\n",
> - nid,
> - h->nr_huge_pages_node[nid],
> - h->free_huge_pages_node[nid],
> - h->surplus_huge_pages_node[nid],
> - huge_page_size(h) / SZ_1K);
> + for_each_hstate(h)
> + pr_info("Node %d hugepages_total=%u hugepages_free=%u hugepages_surp=%u hugepages_size=%lukB\n",

It is better to use printk here. Two reasons.

1) To keep consistent with the print in show_free_areas().
2) If we want to introduce the macro "#define pr_fmt(fmt) "HugeTLB: "
fmt" to hugetlb.c
in the future, the print here will not be affected.

> + nid,
> + h->nr_huge_pages_node[nid],
> + h->free_huge_pages_node[nid],
> + h->surplus_huge_pages_node[nid],
> + huge_page_size(h) / SZ_1K);
> }
>
> void hugetlb_report_usage(struct seq_file *m, struct mm_struct *mm)
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 2eb6ad5a650a..684c2e410923 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -6014,7 +6014,7 @@ static void show_migration_types(unsigned char type)
> void show_free_areas(unsigned int filter, nodemask_t *nodemask)
> {
> unsigned long free_pcp = 0;
> - int cpu;
> + int cpu, nid;
> struct zone *zone;
> pg_data_t *pgdat;
>
> @@ -6202,7 +6202,13 @@ void show_free_areas(unsigned int filter, nodemask_t *nodemask)
> printk(KERN_CONT "= %lukB\n", K(total));
> }
>
> - hugetlb_show_meminfo();
> + if (hugepages_supported()) {

As I said above, embed this check into hugetlb_show_meminfo_node().

> + for_each_node_state(nid, N_MEMORY) {

Please use for_each_online_node here.

Thanks.

> + if (show_mem_node_skip(filter, nid, nodemask))
> + continue;
> + hugetlb_show_meminfo_node(nid);
> + }
> + }
>
> printk("%ld total pagecache pages\n", global_node_page_state(NR_FILE_PAGES));
>
> --
> 2.20.1
>