Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1730403 > unrolled thread

[RFC Patch 1/1] mm/hugetlb: Clarify OOM message on size of hugetlb and requested hugepages total

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [RFC Patch 1/1] mm/hugetlb: Clarify OOM message on size of hugetlb and requested hugepages total "Liam R. Howlett" <Liam.Howlett@Oracle.com> - 2017-09-11 17:50 +0200
    Re: [RFC Patch 1/1] mm/hugetlb: Clarify OOM message on size of  hugetlb and requested hugepages total Michal Hocko <mhocko@kernel.org> - 2017-09-13 14:50 +0200
      Re: [RFC Patch 1/1] mm/hugetlb: Clarify OOM message on size of  hugetlb and requested hugepages total "Liam R. Howlett" <Liam.Howlett@Oracle.com> - 2017-09-13 18:00 +0200

#1730403 — [RFC Patch 1/1] mm/hugetlb: Clarify OOM message on size of hugetlb and requested hugepages total

Change the output of hugetlb_show_meminfo to give the size of the
hugetlb in more than just Kb and add a warning message if the requested
hugepages is larger than the allocated hugepages.  The warning message
for very badly configured hugepages has been removed in favour of this
method.

The new messages look like this:
----
Node 0 hugepages_total=1 hugepages_free=1 hugepages_surp=0
hugepages_size=1.00 GiB

Node 0 hugepages_total=1326 hugepages_free=1326 hugepages_surp=0
hugepages_size=2.00 MiB

hugepage_size 1.00 GiB: Requested 5 hugepages (5.00 GiB) but 1 hugepages
(1.00 GiB) were allocated.

hugepage_size 2.00 MiB: Requested 4000 hugepages (7.81 GiB) but 1326
hugepages (2.59 GiB) were allocated.
----

The old messages look like this:
----
Node 0 hugepages_total=1 hugepages_free=1 hugepages_surp=0
hugepages_size=1048576kB

Node 0 hugepages_total=1435 hugepages_free=1435 hugepages_surp=0
hugepages_size=2048kB
----

Signed-off-by: Liam R. Howlett <Liam.Howlett@Oracle.com>
---
 include/linux/hugetlb.h |  1 +
 mm/hugetlb.c            | 35 +++++++++++++++++++++++++++++++----
 2 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index b857fc8cc2ec..9f188d621ae0 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -313,6 +313,7 @@ struct hstate {
 	unsigned int order;
 	unsigned long mask;
 	unsigned long max_huge_pages;
+	unsigned long req_max_huge_pages;
 	unsigned long nr_huge_pages;
 	unsigned long free_huge_pages;
 	unsigned long resv_huge_pages;
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index 3eedb187e549..83c06ce89bfd 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -1461,6 +1461,7 @@ static int dissolve_free_huge_page(struct page *page)
 		h->free_huge_pages--;
 		h->free_huge_pages_node[nid]--;
 		h->max_huge_pages--;
+		h->req_max_huge_pages--;
 		update_and_free_page(h, head);
 	}
 out:
@@ -2430,6 +2431,7 @@ static ssize_t __nr_hugepages_store_common(bool obey_mempolicy,
 		goto out;
 	}
 
+	h->req_max_huge_pages = count;
 	if (nid == NUMA_NO_NODE) {
 		/*
 		 * global hstate attribute
@@ -3026,14 +3028,39 @@ void hugetlb_show_meminfo(void)
 	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",
+	for_each_node_state(nid, N_MEMORY) {
+		for_each_hstate(h) {
+			char hp_size[32];
+
+			string_get_size(huge_page_size(h), 1, STRING_UNITS_2,
+					hp_size, 32);
+			pr_info("Node %d hugepages_total=%u hugepages_free=%u hugepages_surp=%u hugepages_size=%s\n",
 				nid,
 				h->nr_huge_pages_node[nid],
 				h->free_huge_pages_node[nid],
 				h->surplus_huge_pages_node[nid],
-				1UL << (huge_page_order(h) + PAGE_SHIFT - 10));
+				hp_size);
+		}
+	}
+
+	for_each_hstate(h) {
+		if (h->max_huge_pages < h->req_max_huge_pages) {
+			char hp_size[32];
+			char hpr_size[32];
+			char hpt_size[32];
+
+			string_get_size(huge_page_size(h), 1, STRING_UNITS_2,
+					hp_size, 32);
+			string_get_size(huge_page_size(h),
+					h->req_max_huge_pages, STRING_UNITS_2,
+					hpr_size, 32);
+			string_get_size(huge_page_size(h), h->max_huge_pages,
+					STRING_UNITS_2, hpt_size, 32);
+			pr_warn("hugepage_size %s: Requested %lu hugepages (%s) but %lu hugepages (%s) were allocated.\n",
+				hp_size, h->req_max_huge_pages, hpr_size,
+				h->max_huge_pages, hpt_size);
+		}
+	}
 }
 
 void hugetlb_report_usage(struct seq_file *m, struct mm_struct *mm)
-- 
2.14.1.145.gb3622a4ee

[toc] | [next] | [standalone]

#1731592 — Re: [RFC Patch 1/1] mm/hugetlb: Clarify OOM message on size of hugetlb and requested hugepages total

On Mon 11-09-17 11:48:20, Liam R. Howlett wrote:
> Change the output of hugetlb_show_meminfo to give the size of the
> hugetlb in more than just Kb and add a warning message if the requested
> hugepages is larger than the allocated hugepages.  The warning message
> for very badly configured hugepages has been removed in favour of this
> method.
> 
> The new messages look like this:
> ----
> Node 0 hugepages_total=1 hugepages_free=1 hugepages_surp=0
> hugepages_size=1.00 GiB
> 
> Node 0 hugepages_total=1326 hugepages_free=1326 hugepages_surp=0
> hugepages_size=2.00 MiB
> 
> hugepage_size 1.00 GiB: Requested 5 hugepages (5.00 GiB) but 1 hugepages
> (1.00 GiB) were allocated.
> 
> hugepage_size 2.00 MiB: Requested 4000 hugepages (7.81 GiB) but 1326
> hugepages (2.59 GiB) were allocated.
> ----
> 
> The old messages look like this:
> ----
> Node 0 hugepages_total=1 hugepages_free=1 hugepages_surp=0
> hugepages_size=1048576kB
> 
> Node 0 hugepages_total=1435 hugepages_free=1435 hugepages_surp=0
> hugepages_size=2048kB
> ----
> 
> Signed-off-by: Liam R. Howlett <Liam.Howlett@Oracle.com>

To be honest, I really dislike this. It doesn't really add anything
really new to the OOM report. We already know how much memory is
unreclaimable because it is reserved for hugetlb usage. Why does the
requested size make any difference? We could fail to allocate requested
number of pages because of memory pressure or fragmentation without any
sign of misconfiguration.

Also req_max_huge_pages would have to be per NUMA node othwerise you are
just losing information when allocation hugetlb pages via sysfs per node
interface.

> ---
>  include/linux/hugetlb.h |  1 +
>  mm/hugetlb.c            | 35 +++++++++++++++++++++++++++++++----
>  2 files changed, 32 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
> index b857fc8cc2ec..9f188d621ae0 100644
> --- a/include/linux/hugetlb.h
> +++ b/include/linux/hugetlb.h
> @@ -313,6 +313,7 @@ struct hstate {
>  	unsigned int order;
>  	unsigned long mask;
>  	unsigned long max_huge_pages;
> +	unsigned long req_max_huge_pages;
>  	unsigned long nr_huge_pages;
>  	unsigned long free_huge_pages;
>  	unsigned long resv_huge_pages;
> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 3eedb187e549..83c06ce89bfd 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -1461,6 +1461,7 @@ static int dissolve_free_huge_page(struct page *page)
>  		h->free_huge_pages--;
>  		h->free_huge_pages_node[nid]--;
>  		h->max_huge_pages--;
> +		h->req_max_huge_pages--;
>  		update_and_free_page(h, head);
>  	}
>  out:
> @@ -2430,6 +2431,7 @@ static ssize_t __nr_hugepages_store_common(bool obey_mempolicy,
>  		goto out;
>  	}
>  
> +	h->req_max_huge_pages = count;
>  	if (nid == NUMA_NO_NODE) {
>  		/*
>  		 * global hstate attribute
> @@ -3026,14 +3028,39 @@ void hugetlb_show_meminfo(void)
>  	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",
> +	for_each_node_state(nid, N_MEMORY) {
> +		for_each_hstate(h) {
> +			char hp_size[32];
> +
> +			string_get_size(huge_page_size(h), 1, STRING_UNITS_2,
> +					hp_size, 32);
> +			pr_info("Node %d hugepages_total=%u hugepages_free=%u hugepages_surp=%u hugepages_size=%s\n",
>  				nid,
>  				h->nr_huge_pages_node[nid],
>  				h->free_huge_pages_node[nid],
>  				h->surplus_huge_pages_node[nid],
> -				1UL << (huge_page_order(h) + PAGE_SHIFT - 10));
> +				hp_size);
> +		}
> +	}
> +
> +	for_each_hstate(h) {
> +		if (h->max_huge_pages < h->req_max_huge_pages) {
> +			char hp_size[32];
> +			char hpr_size[32];
> +			char hpt_size[32];
> +
> +			string_get_size(huge_page_size(h), 1, STRING_UNITS_2,
> +					hp_size, 32);
> +			string_get_size(huge_page_size(h),
> +					h->req_max_huge_pages, STRING_UNITS_2,
> +					hpr_size, 32);
> +			string_get_size(huge_page_size(h), h->max_huge_pages,
> +					STRING_UNITS_2, hpt_size, 32);
> +			pr_warn("hugepage_size %s: Requested %lu hugepages (%s) but %lu hugepages (%s) were allocated.\n",
> +				hp_size, h->req_max_huge_pages, hpr_size,
> +				h->max_huge_pages, hpt_size);
> +		}
> +	}
>  }
>  
>  void hugetlb_report_usage(struct seq_file *m, struct mm_struct *mm)
> -- 
> 2.14.1.145.gb3622a4ee
> 

-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [next] | [standalone]

#1731693 — Re: [RFC Patch 1/1] mm/hugetlb: Clarify OOM message on size of hugetlb and requested hugepages total

* Michal Hocko <mhocko@kernel.org> [170913 08:43]:
> On Mon 11-09-17 11:48:20, Liam R. Howlett wrote:
> > Change the output of hugetlb_show_meminfo to give the size of the
> > hugetlb in more than just Kb and add a warning message if the requested
> > hugepages is larger than the allocated hugepages.  The warning message
> > for very badly configured hugepages has been removed in favour of this
> > method.
> > 
> > The new messages look like this:
> > ----
> > Node 0 hugepages_total=1 hugepages_free=1 hugepages_surp=0
> > hugepages_size=1.00 GiB
> > 
> > Node 0 hugepages_total=1326 hugepages_free=1326 hugepages_surp=0
> > hugepages_size=2.00 MiB
> > 
> > hugepage_size 1.00 GiB: Requested 5 hugepages (5.00 GiB) but 1 hugepages
> > (1.00 GiB) were allocated.
> > 
> > hugepage_size 2.00 MiB: Requested 4000 hugepages (7.81 GiB) but 1326
> > hugepages (2.59 GiB) were allocated.
> > ----
> > 
> > The old messages look like this:
> > ----
> > Node 0 hugepages_total=1 hugepages_free=1 hugepages_surp=0
> > hugepages_size=1048576kB
> > 
> > Node 0 hugepages_total=1435 hugepages_free=1435 hugepages_surp=0
> > hugepages_size=2048kB
> > ----
> > 
> > Signed-off-by: Liam R. Howlett <Liam.Howlett@Oracle.com>
> 
> To be honest, I really dislike this. It doesn't really add anything
> really new to the OOM report. We already know how much memory is
> unreclaimable because it is reserved for hugetlb usage. Why does the
> requested size make any difference? We could fail to allocate requested
> number of pages because of memory pressure or fragmentation without any
> sign of misconfiguration.

Okay, thanks.  I was trying to address the issues you had with the
previous logging addition.

I understand that the OOM report is clear to many, but I thought it
would be more clear if the hugepage size was printed in a human readable
format instead of KB, especially with platforms supporting a lot of
huge page sizes and we already use the formatting elsewhere.

My thoughts for the requested size was to expose the failure to allocate
a resource which currently doesn't have any reporting back to the user -
except on boot failures, which you also disliked.  I thought reporting
in the OOM message would be less of a change than reporting at
allocation time and it would be more clear what happened on poorly
configured systems as the failure would be printed closer to the panic.

> 
> Also req_max_huge_pages would have to be per NUMA node othwerise you are
> just losing information when allocation hugetlb pages via sysfs per node
> interface.
> 

Thank you for your thorough review and time,
Liam

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web