malloc: Account for all heaps in an arena in malloc_info [BZ #22439]

Message ID 20171114180833.5FA9E423CD593@oldenburg.str.redhat.com
State New
Headers show
Series
  • malloc: Account for all heaps in an arena in malloc_info [BZ #22439]
Related show

Commit Message

Florian Weimer Nov. 14, 2017, 6:08 p.m.
This commit adds a "subheads" statistics to the malloc_info output.

2017-11-14  Florian Weimer  <fweimer@redhat.com>

	[BZ #22408]
	* malloc/malloc.c (__malloc_info): Count all heaps in an arena,
	not just the top one.  Output a new "subheaps" statistic.

Comments

Siddhesh Poyarekar Nov. 15, 2017, 3:33 a.m. | #1
On Tuesday 14 November 2017 11:38 PM, Florian Weimer wrote:
> This commit adds a "subheads" statistics to the malloc_info output.

How about something like this for the description:

"This commit adds a "subheaps" field to the malloc_info output that
shows the number of heaps that were allocated to extend a non-main arena."

Looks OK otherwise.

Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>

> 2017-11-14  Florian Weimer  <fweimer@redhat.com>
> 
> 	[BZ #22408]
> 	* malloc/malloc.c (__malloc_info): Count all heaps in an arena,
> 	not just the top one.  Output a new "subheaps" statistic.
> 
> diff --git a/NEWS b/NEWS
> index b7281621f4..520db40982 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -63,6 +63,9 @@ Deprecated and removed features, and other changes affecting compatibility:
>  * The res_hnok, res_dnok, res_mailok and res_ownok functions now check that
>    the specified string can be parsed as a domain name.
>  
> +* In the malloc_info output, the <heap> element may contain another <aspace>
> +  element, "subheaps", which contains the number of sub-heaps.
> +
>  Changes to build and runtime requirements:
>  
>    [Add changes to build and runtime requirements here]
> diff --git a/malloc/malloc.c b/malloc/malloc.c
> index 0494e8c39f..2999ac4d2f 100644
> --- a/malloc/malloc.c
> +++ b/malloc/malloc.c
> @@ -5457,11 +5457,19 @@ __malloc_info (int options, FILE *fp)
>  
>        size_t heap_size = 0;
>        size_t heap_mprotect_size = 0;
> +      size_t heap_count = 0;
>        if (ar_ptr != &main_arena)
>  	{
> +	  /* Iterate over the arena heaps from back to front.  */
>  	  heap_info *heap = heap_for_ptr (top (ar_ptr));
> -	  heap_size = heap->size;
> -	  heap_mprotect_size = heap->mprotect_size;
> +	  do
> +	    {
> +	      heap_size += heap->size;
> +	      heap_mprotect_size += heap->mprotect_size;
> +	      heap = heap->prev;
> +	      ++heap_count;
> +	    }
> +	  while (heap != NULL);
>  	}
>  
>        __libc_lock_unlock (ar_ptr->mutex);
> @@ -5499,8 +5507,9 @@ __malloc_info (int options, FILE *fp)
>  	{
>  	  fprintf (fp,
>  		   "<aspace type=\"total\" size=\"%zu\"/>\n"
> -		   "<aspace type=\"mprotect\" size=\"%zu\"/>\n",
> -		   heap_size, heap_mprotect_size);
> +		   "<aspace type=\"mprotect\" size=\"%zu\"/>\n"
> +		   "<aspace type=\"subheaps\" size=\"%zu\"/>\n",
> +		   heap_size, heap_mprotect_size, heap_count);
>  	  total_aspace += heap_size;
>  	  total_aspace_mprotect += heap_mprotect_size;
>  	}
>

Patch

diff --git a/NEWS b/NEWS
index b7281621f4..520db40982 100644
--- a/NEWS
+++ b/NEWS
@@ -63,6 +63,9 @@  Deprecated and removed features, and other changes affecting compatibility:
 * The res_hnok, res_dnok, res_mailok and res_ownok functions now check that
   the specified string can be parsed as a domain name.
 
+* In the malloc_info output, the <heap> element may contain another <aspace>
+  element, "subheaps", which contains the number of sub-heaps.
+
 Changes to build and runtime requirements:
 
   [Add changes to build and runtime requirements here]
diff --git a/malloc/malloc.c b/malloc/malloc.c
index 0494e8c39f..2999ac4d2f 100644
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -5457,11 +5457,19 @@  __malloc_info (int options, FILE *fp)
 
       size_t heap_size = 0;
       size_t heap_mprotect_size = 0;
+      size_t heap_count = 0;
       if (ar_ptr != &main_arena)
 	{
+	  /* Iterate over the arena heaps from back to front.  */
 	  heap_info *heap = heap_for_ptr (top (ar_ptr));
-	  heap_size = heap->size;
-	  heap_mprotect_size = heap->mprotect_size;
+	  do
+	    {
+	      heap_size += heap->size;
+	      heap_mprotect_size += heap->mprotect_size;
+	      heap = heap->prev;
+	      ++heap_count;
+	    }
+	  while (heap != NULL);
 	}
 
       __libc_lock_unlock (ar_ptr->mutex);
@@ -5499,8 +5507,9 @@  __malloc_info (int options, FILE *fp)
 	{
 	  fprintf (fp,
 		   "<aspace type=\"total\" size=\"%zu\"/>\n"
-		   "<aspace type=\"mprotect\" size=\"%zu\"/>\n",
-		   heap_size, heap_mprotect_size);
+		   "<aspace type=\"mprotect\" size=\"%zu\"/>\n"
+		   "<aspace type=\"subheaps\" size=\"%zu\"/>\n",
+		   heap_size, heap_mprotect_size, heap_count);
 	  total_aspace += heap_size;
 	  total_aspace_mprotect += heap_mprotect_size;
 	}