2022-07-05 09:28:16

by Gang Li

[permalink] [raw]
Subject: [PATCH v3] mm, hugetlb: skip irrelevant nodes in show_free_areas()

show_free_areas() allows to filter out node specific data which is
irrelevant to the allocation request. But hugetlb_show_meminfo() still
shows hugetlb on all nodes, which is redundant and unnecessary.

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 <[email protected]>
---
v3: move for_each_hstate() into hugetlb_show_meminfo_node().
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 | 18 ++++++++----------
mm/page_alloc.c | 8 ++++++--
3 files changed, 16 insertions(+), 14 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..e7f12edb120c 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -4490,22 +4490,20 @@ 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;

- 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)
+ printk("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);
}

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..93f032f32812 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,11 @@ void show_free_areas(unsigned int filter, nodemask_t *nodemask)
printk(KERN_CONT "= %lukB\n", K(total));
}

- hugetlb_show_meminfo();
+ for_each_online_node(nid) {
+ 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


2022-07-05 09:41:06

by Muchun Song

[permalink] [raw]
Subject: Re: [PATCH v3] mm, hugetlb: skip irrelevant nodes in show_free_areas()

On Tue, Jul 5, 2022 at 5:21 PM Gang Li <[email protected]> wrote:
>
> show_free_areas() allows to filter out node specific data which is
> irrelevant to the allocation request. But hugetlb_show_meminfo() still
> shows hugetlb on all nodes, which is redundant and unnecessary.
>
> 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 <[email protected]>

Reviewed-by: Muchun Song <[email protected]>

Thanks.

2022-07-05 21:33:12

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH v3] mm, hugetlb: skip irrelevant nodes in show_free_areas()

On Tue, 5 Jul 2022 17:21:19 +0800 Gang Li <[email protected]> wrote:

> show_free_areas() allows to filter out node specific data which is
> irrelevant to the allocation request. But hugetlb_show_meminfo() still
> shows hugetlb on all nodes, which is redundant and unnecessary.
>
> Use show_mem_node_skip() to skip irrelevant nodes. And replace
> hugetlb_show_meminfo() with hugetlb_show_meminfo_node(nid).

It would be helpful to include before-and-after sample output text in
the changelog to help others assess the proposed change.

> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -6202,7 +6202,11 @@ void show_free_areas(unsigned int filter, nodemask_t *nodemask)
> printk(KERN_CONT "= %lukB\n", K(total));
> }
>
> - hugetlb_show_meminfo();
> + for_each_online_node(nid) {
> + if (show_mem_node_skip(filter, nid, nodemask))
> + continue;
> + hugetlb_show_meminfo_node(nid);
> + }
>

Does this mean that potentially useful info about presently-offline
nodes will no longer be available?

2022-07-05 22:15:12

by Mike Kravetz

[permalink] [raw]
Subject: Re: [PATCH v3] mm, hugetlb: skip irrelevant nodes in show_free_areas()

On 07/05/22 14:25, Andrew Morton wrote:
> On Tue, 5 Jul 2022 17:21:19 +0800 Gang Li <[email protected]> wrote:
>
> > show_free_areas() allows to filter out node specific data which is
> > irrelevant to the allocation request. But hugetlb_show_meminfo() still
> > shows hugetlb on all nodes, which is redundant and unnecessary.
> >
> > Use show_mem_node_skip() to skip irrelevant nodes. And replace
> > hugetlb_show_meminfo() with hugetlb_show_meminfo_node(nid).
>
> It would be helpful to include before-and-after sample output text in
> the changelog to help others assess the proposed change.
>
> > --- a/mm/page_alloc.c
> > +++ b/mm/page_alloc.c
> > @@ -6202,7 +6202,11 @@ void show_free_areas(unsigned int filter, nodemask_t *nodemask)
> > printk(KERN_CONT "= %lukB\n", K(total));
> > }
> >
> > - hugetlb_show_meminfo();
> > + for_each_online_node(nid) {
> > + if (show_mem_node_skip(filter, nid, nodemask))
> > + continue;
> > + hugetlb_show_meminfo_node(nid);
> > + }
> >
>
> Does this mean that potentially useful info about presently-offline
> nodes will no longer be available?
>

I do not believe that is possible.

IIUC, all memory blocks of a node must be offline for the node to be
marked offline. To offline a memory block, all hugetlb pages must be
removed from the memory block. So, an offline node should have no
hugetlb pages. And, if there are no hugetlb pages it makes no sense to
call hugetlb_show_meminfo_node.

Also, previous code in show_free_areas skips offline nodes. So, this
new code would be consistent with existing code.
--
Mike Kravetz

2022-07-05 23:08:07

by Mike Kravetz

[permalink] [raw]
Subject: Re: [PATCH v3] mm, hugetlb: skip irrelevant nodes in show_free_areas()

On 07/05/22 17:21, Gang Li wrote:
> show_free_areas() allows to filter out node specific data which is
> irrelevant to the allocation request. But hugetlb_show_meminfo() still
> shows hugetlb on all nodes, which is redundant and unnecessary.
>
> 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 <[email protected]>
> ---
> v3: move for_each_hstate() into hugetlb_show_meminfo_node().
> 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 | 18 ++++++++----------
> mm/page_alloc.c | 8 ++++++--
> 3 files changed, 16 insertions(+), 14 deletions(-)

Thanks. That should make hugetlb information produced by show_free_areas
consistent with other node specific information.

Reviewed-by: Mike Kravetz <[email protected]>
--
Mike Kravetz