| Message ID | 20220705092120.2158-1-ligang.bdlg@bytedance.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v3] mm, hugetlb: skip irrelevant nodes in show_free_areas() | expand |
On Tue, Jul 5, 2022 at 5:21 PM 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 > 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 <ligang.bdlg@bytedance.com> Reviewed-by: Muchun Song <songmuchun@bytedance.com> Thanks.
On Tue, 5 Jul 2022 17:21:19 +0800 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 > 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?
On 07/05/22 14:25, Andrew Morton wrote: > On Tue, 5 Jul 2022 17:21:19 +0800 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 > > 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.
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 <ligang.bdlg@bytedance.com> > --- > 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 <mike.kravetz@oracle.com>
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));
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 <ligang.bdlg@bytedance.com> --- 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(-)