diff mbox

答复: Re: Bug 15321 - malloc/free can't give the memory back to kernel when main_arena is discontinous

Message ID 201811300908239918728@zte.com.cn
State Changes Requested
Headers show

Commit Message

He Hongjun Nov. 30, 2018, 1:08 a.m. UTC
yes, covered by the one for "Chengdu Zhongxing Software Company Limited (ZTE)" dated 23 August 2018.

I have added the copyright notice in the tst-discontinuous-main-arena.c
The new patch:

2018-11-28  Hehongjun  <He.Hongjun@zte.com.cn>

	[BZ #15321]
	* malloc/Makefile: add tst-discontinuous-main-arena.
	* malloc/arena.c (heap_trim): add main arean
	* malloc/malloc.c: modify discontinuous main arena
	with sysmalloc, modify discontinuous main arena link heap.
	with _int_free, free memory on discontinuous main arena link heap.
	* malloc/tst-discontinuous-main-arena: New test.






------------------原始邮件------------------
发件人:JosephMyers <joseph@codesourcery.com>
收件人:贺洪军10036572;
抄送人:libc-alpha@sourceware.org <libc-alpha@sourceware.org>;
日 期 :2018年11月30日 01:24
主 题 :Re: Bug 15321 - malloc/free can't give the memory back to kernel when main_arena is discontinous
On Thu, 29 Nov 2018, He.Hongjun@zte.com.cn wrote:

> Hello, I have fixed the  Bug 15321.
> patch on glibc-2.28

Do you have an FSF copyright assignment?  (Are you covered by the one for
"Chengdu Zhongxing Software Company Limited (ZTE)" dated 23 August 2018?)

> diff --git a/malloc/tst-discontinuous-main-arena.c b/malloc/tst-discontinuous-main-arena.c
> old mode 100644
> new mode 100755
> index e69de29..3f6f0e7
> --- a/malloc/tst-discontinuous-main-arena.c
> +++ b/malloc/tst-discontinuous-main-arena.c
> @@ -0,0 +1,128 @@
> +/* Ensure that main arena of discontinuity can normal free memory.
> +
> +   The GNU C Library is free software; you can redistribute it and/or

Missing copyright notice, before the license notice.

> +#define TEST_FUNCTION do_test ()
> +#include "../test-skeleton.c"

New tests should use <support/test-driver.c>, not the old-style
test-skeleton.c.

--
Joseph S. Myers
joseph@codesourcery.com

Comments

Carlos O'Donell Nov. 30, 2018, 3:04 a.m. UTC | #1
On 11/29/18 8:08 PM, He.Hongjun@zte.com.cn wrote:
> yes, covered by the one for "Chengdu Zhongxing Software Company Limited (ZTE)" dated 23 August 2018.
> 
> I have added the copyright notice in the tst-discontinuous-main-arena.c

I look forward to a v2.

(1) Include a detailed description of your changes.

At a high level can you describe what the patch does and
what it fixes? Such a description is required for the
git commit message and is an important part of establishing
the intentions of your technical change. A reviewer, like
myself, can then review that the code matches your high-level
itent (or design).

(2) Please attach the patch to the next email.

While the contribution checklist says there is a strong preference
to inline the patch in our email, this sometimes breaks the patch 
because MUAs may convert tabs and spaces or wrap lines. For v2, 
I suggest you commit your code to your local git repo, and then 
send us `git format-patch HEAD~1` which will show us everything 
you *would* have committed when you push the patch. This lets me
review everything including the commit message.
 
> 2018-11-28  Hehongjun  <He.Hongjun@zte.com.cn>
> 
> 	[BZ #15321]
> 	* malloc/Makefile: add tst-discontinuous-main-arena.
> 	* malloc/arena.c (heap_trim): add main arean
> 	* malloc/malloc.c: modify discontinuous main arena
> 	with sysmalloc, modify discontinuous main arena link heap.
> 	with _int_free, free memory on discontinuous main arena link heap.
> 	* malloc/tst-discontinuous-main-arena: New test.
> 
> diff --git a/malloc/Makefile b/malloc/Makefile
> index 7d54bad..9d60ab1 100644
> --- a/malloc/Makefile
> +++ b/malloc/Makefile
> @@ -38,6 +38,7 @@ tests := mallocbug tst-malloc tst-valloc tst-calloc tst-obstack \
>  	 tst-malloc_info \
>  	 tst-malloc-too-large \
>  	 tst-malloc-stats-cancellation \
> +    tst-discontinuous-main-arena \

I think this is the wrong place to put this test, see notes bleow.

> 
>  tests-static := \
>  	 tst-interpose-static-nothread \
> diff --git a/malloc/arena.c b/malloc/arena.c
> index 497ae47..343ef13 100755
> --- a/malloc/arena.c
> +++ b/malloc/arena.c
> @@ -634,6 +634,45 @@ heap_trim (heap_info *heap, size_t pad)
>        /*check_chunk(ar_ptr, top_chunk);*/
>      }
> 
> +  if (ar_ptr == &main_arena && heap->prev == NULL)
> +    {
> +      char *old_top;
> +      old_top = (char *) (heap + 1);
> +      p = (mchunkptr) (old_top + sizeof (unsigned long));
> +
> +      misalign = (unsigned long) chunk2mem (p) & MALLOC_ALIGN_MASK;
> +      if (misalign > 0)
> +        p = ((char *) p) + MALLOC_ALIGNMENT - misalign;
> +
> +      if (top_chunk == (mchunkptr) p)
> +  	{
> +          p = (mchunkptr) (*(unsigned long *) old_top);
> +          p = prev_chunk (p);
> +          new_size = chunksize (p) + MINSIZE;
> +
> +          if (!prev_inuse (p))
> +            new_size += prev_size (p);	
> +	  
> +          if (new_size >= pad + MINSIZE + pagesz)
> +            {
> +              ar_ptr->system_mem -= heap->size;
> +              delete_heap (heap);
> +              if (!prev_inuse (p)) { /* consolidate backward */
> +                p = prev_chunk (p);
> +              unlink (ar_ptr, p, bck, fwd);
> +            }
> +
> +          top (ar_ptr) = top_chunk = (char *) p - 2 * SIZE_SZ;
> +          set_head (top_chunk, new_size | PREV_INUSE);
> +
> +          set_contiguous (ar_ptr);
> +          if ((unsigned long)(chunksize (ar_ptr->top)) >= (unsigned long)mp_.trim_threshold)
> +            systrim (mp_.trim_threshold, ar_ptr);			
> +          return 0;
> +        }
> +      }
> +    }
> +

I need to spend some time reviewing this and matching it
with your intent. So I'll wait for v2 to review this.

>    /* Uses similar logic for per-thread arenas as the main arena with systrim
>       and _int_free by preserving the top pad and rounding down to the nearest
>       page.  */
> diff --git a/malloc/malloc.c b/malloc/malloc.c
> index e247c77..ad53ef7 100755
> --- a/malloc/malloc.c
> +++ b/malloc/malloc.c
> @@ -2388,7 +2388,7 @@ sysmalloc (INTERNAL_SIZE_T nb, mstate av)
>    assert ((unsigned long) (old_size) < (unsigned long) (nb + MINSIZE));
> 
> 
> -  if (av != &main_arena)
> +  if ((av != &main_arena) || (av == &main_arena && !contiguous (av)))
>      {
>        heap_info *old_heap, *heap;
>        size_t old_heap_size;
> @@ -2438,8 +2438,6 @@ sysmalloc (INTERNAL_SIZE_T nb, mstate av)
>          goto try_mmap;
>      }
>    else     /* av == main_arena */
> -
> -

Please avoid any white space changes which are not related to the
change you are making.

Please feel free to submit another patch to fix this independently
of the change.

>      { /* Request enough space for nb + pad + overhead */
>        size = nb + mp_.top_pad + MINSIZE;
> 
> @@ -2491,35 +2489,62 @@ sysmalloc (INTERNAL_SIZE_T nb, mstate av)
>               and threshold limits, since the space will not be used as a
>               segregated mmap region.
>             */
> -
> -          /* Cannot merge with old top, so add its size back in */
> -          if (contiguous (av))
> -            size = ALIGN_UP (size + old_size, pagesize);
> -
> -          /* If we are relying on mmap as backup, then use larger units */
> -          if ((unsigned long) (size) < (unsigned long) (MMAP_AS_MORECORE_SIZE))
> -            size = MMAP_AS_MORECORE_SIZE;
> -
> -          /* Don't try if size wraps around 0 */
> -          if ((unsigned long) (size) > (unsigned long) (nb))
> -            {
> -              char *mbrk = (char *) (MMAP (0, size, PROT_READ | PROT_WRITE, 0));
> -
> -              if (mbrk != MAP_FAILED)
> +          heap_info *heap;
> +          long misalign;
> +          char *ptr;
> +          heap = new_heap (nb + (MINSIZE + sizeof (*heap) + MALLOC_ALIGNMENT), mp_.top_pad);
> +          if (!heap)
> +   	    {
> +   	      if (!tried_mmap)       	        
> +       	        goto try_mmap;
> +              else
>                  {
> -                  /* We do not need, and cannot use, another sbrk call to find end */
> -                  brk = mbrk;
> -                  snd_brk = brk + size;
> +       	          __set_errno (ENOMEM);
> +   		  return 0;
> +                } 
> +   	    }
> +			
> +          heap->ar_ptr = av;
> +          heap->prev = NULL;
> +          av->system_mem += heap->size;
> +	  
> +          ptr = (char *) (heap + 1);
> +          ptr += sizeof (unsigned long);
> +          misalign = (unsigned long) chunk2mem (ptr) & MALLOC_ALIGN_MASK;
> +          if (misalign > 0)
> +            ptr += MALLOC_ALIGNMENT - misalign;
> +          /* Set up the new top.  */		  
> +          top (av) = ptr;
> +          set_head (top (av), (((char *) heap + heap->size) - ptr) | PREV_INUSE);
> 
> -                  /*
> -                     Record that we no longer have a contiguous sbrk region.
> -                     After the first time mmap is used as backup, we do not
> -                     ever rely on contiguous space since this could incorrectly
> -                     bridge regions.
> -                   */
> -                  set_noncontiguous (av);
> -                }
> +          /* Setup fencepost and free the old top chunk with a multiple of
> +             MALLOC_ALIGNMENT in size. */
> +          /* The fencepost takes at least MINSIZE bytes, because it might
> +             become the top chunk again later.  Note that a footer is set
> +             up, too, although the chunk is marked in use. */
> +          old_size = (old_size - MINSIZE) & ~MALLOC_ALIGN_MASK;
> +          set_head (chunk_at_offset (old_top, old_size + 2 * SIZE_SZ), 0 | PREV_INUSE);
> +          *(unsigned long *)((heap + 1)) = chunk_at_offset (old_top, old_size + 2 * SIZE_SZ);
> +          if (old_size >= MINSIZE)
> +            {
> +              set_head (chunk_at_offset (old_top, old_size), (2 * SIZE_SZ) | PREV_INUSE);
> +              set_foot (chunk_at_offset (old_top, old_size), (2 * SIZE_SZ));
> +              set_head (old_top, old_size | PREV_INUSE);			  
> +              _int_free (av, old_top, 1);
> +            }
> +          else
> +            {
> +              set_head (old_top, (old_size + 2 * SIZE_SZ) | PREV_INUSE);
> +              set_foot (old_top, (old_size + 2 * SIZE_SZ));
>              }
> +
> +          /*
> +            Record that we no longer have a contiguous sbrk region.
> +            After the first time mmap is used as backup, we do not
> +            ever rely on contiguous space since this could incorrectly
> +            bridge regions.
> +            */
> +          set_noncontiguous (av);

Again, this needs more review and I'll try to get to that in your v2.

>          }
> 
>        if (brk != (char *) (MORECORE_FAILURE))
> @@ -4347,7 +4372,7 @@ _int_free (mstate av, mchunkptr p, int have_lock)
>        if (atomic_load_relaxed (&av->have_fastchunks))
>  	malloc_consolidate(av);
> 
> -      if (av == &main_arena) {
> +      if ((av == &main_arena ) && contiguous (av)) {
>  #ifndef MORECORE_CANNOT_TRIM
>  	if ((unsigned long)(chunksize(av->top)) >=
>  	    (unsigned long)(mp_.trim_threshold))
> diff --git a/malloc/tst-discontinuous-main-arena.c b/malloc/tst-discontinuous-main-arena.c
> old mode 100644
> new mode 100755
> index e69de29..ef7249f
> --- a/malloc/tst-discontinuous-main-arena.c
> +++ b/malloc/tst-discontinuous-main-arena.c

This file is linux-specific and parses /proc/self/statm.

I suggest:

sysdeps/unix/sysv/linux/tst-discontig-main-arena.c

Add it to:

sysdeps/unix/sysv/linux/Makefile

here (around line 10):
~~~
...
ifeq ($(subdir),malloc)
CFLAGS-malloc.c += -DMORECORE_CLEARS=2
tests += tst-discontig-main-arena
endif
...
~~~

> @@ -0,0 +1,133 @@

It should look like this:

/* <One sentence description.>
   <Copyright line, with 2018 only because this is a new test.>
   <Rest of the license>  */

We no longer use 'Contributed by' lines, we rely on ChangeLogs and
git commit messages to provide attribution.

I suggest:

/* Test that a discontinuous main arena can free through the gap.
   Copyright (C) 2018 Free Software Foundation, Inc.
   This file is part of the GNU C Library.


> +/* Copyright (C) 1999-2018 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +   Contributed by He.Hongjun@zte.com.cn, 2018.
> +   
> +   Ensure that main arena of discontinuity can normal free memory.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public License as
> +   published by the Free Software Foundation; either version 2.1 of the
> +   License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; see the file COPYING.LIB.  If
> +   not, see <http://www.gnu.org/licenses/>.  */
> +
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <sys/mman.h>
> +
> +#define MEM_SIZE 1024*128

This isn't a size, but rather a count? Should we call it BLK_COUNT?

> +#define BLK_SIZE 1024
> +#define TOP_PAD_SIZE 1024 * 128
> +

Test needs a block comment at the top explaining in detail the intent of the test.

> +static int
> +do_test (void)
> +{
> +  /* Get the peak physical memory usage. */
> +  FILE *f;  
> +  unsigned n_rss = 0;
> +  unsigned n_resident = 0;
> +  unsigned n_share = 0;
> +  unsigned n_text = 0;
> +  unsigned n_lib = 0;
> +  unsigned n_data = 0;
> +  unsigned n_dt = 0;
> +  int i = 0;
> +
> +  f = fopen ("/proc/self/statm", "r");

Suggest:

TEST_VERIFY_EXIT (f != NULL);
fscanf (f,"%u %u %u %u %u %u %u",
	&n_rss,&n_resident,&n_share,&n_text,&n_lib,&n_data,&n_dt);
printf ("n_rss=%d\n", n_rss);
printf ("n_resident=%d\n", n_resident);
fclose (f);

~~~
> +  if (f)
> +    {
> +      fscanf (f,"%u %u %u %u %u %u %u",
> +        &n_rss,&n_resident,&n_share,&n_text,&n_lib,&n_data,&n_dt);
> +      printf ("n_rss=%d\n", n_rss);
> +      printf ("n_resident=%d\n", n_resident);
> +      fclose (f);
> +    } 
> +  else
> +    return -1;
~~~


> +
> +  void **mem1 = (void**) malloc (sizeof (void*) * MEM_SIZE); 
> +  void **mem2 = (void**) malloc (sizeof (void*) * MEM_SIZE); 
> +
> +  for (i = 0; i < MEM_SIZE; i++)
> +    {
> +      mem1[i] = malloc (BLK_SIZE);
> +      if (mem1[i] == NULL)
> +        {
> +          printf ("malloc(BLK_SIZE) failed.\n");
> +          return -1;
> +        }
> +      memset (mem1[i], 0xa, BLK_SIZE);
> +    }
> +
> +  /* Place a fence in front of the program break to make the main arena
> +     discoutinuous. Make sure we do not cover any region that have been
> +     accquire by sbrk. */

Suggest:

/* Place a private anonymous mapping after the process break region to
   force the creation of a discontiguous main arena (a main arena with
   a hole in the middle).  This operation must be done with care to
   ensure that the mapping does not cover any regions that have already
   been acquired by sbrk.  */

> +  long page_sz = sysconf (_SC_PAGESIZE);
> +  void *fence_addr = mem1[MEM_SIZE - 1] + TOP_PAD_SIZE + BLK_SIZE + page_sz;
> +  fence_addr = (void *)((long)fence_addr & ~(page_sz - 1));

I believe you can include 'libc-pointer-arith.h' and use PTR_ALIGN_UP() for this.

> +
> +  void *tt = mmap (fence_addr, page_sz, PROT_READ | PROT_WRITE, 
> +               MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0);
> +  if (tt == MAP_FAILED || tt != fence_addr)
> +    {
> +      printf ("mmap(fence_addr, page_sz, PROT_READ | PROT_WRITE, \
> +                MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0) failed.\n");
> +      return -1;  

All of this can become:

FAIL_EXIT1 ("...", ..., ..., ...);

See support/check.h.

You can use the following to access the routines:

#include <support/check.h>
#include <support/support.h>

(at the top with your other includes).
  
> +    }
> +

Needs some comments explaining what is is being attempted here.

> +  for (i = 0; i < MEM_SIZE; i++) 
> +    {
> +      mem2[i] = malloc (BLK_SIZE);


> +      if (mem2[i] == NULL) 
> +        {
> +          printf ("malloc(BLK_SIZE) failed.\n");
> +          return -1;
> +        }

Suggest just using `TEST_VERIFY_EXIT (mem2[i] != NULL);`


> +      memset (mem2[i], 0xb, BLK_SIZE);
> +    }  
> +
> +  for (i = 0; i < MEM_SIZE; i++) 
> +    {
> +      free (mem1[i]);
> +      mem1[i] = NULL;	
> +    }
> +  free (mem1);  
> +  
> +  for (i = 0; i < MEM_SIZE; i++) 
> +    {
> +      free (mem2[i]);
> +      mem2[i] = NULL;	
> +    }
> +  free (mem2);
> +
> +  f = fopen ("/proc/self/statm", "r");

TEST_VERIFY_EXIT (f != NULL);

> +  if (f)
> +    {
> +      fscanf (f,"%u %u %u %u %u %u %u",
> +        &n_rss,&n_resident,&n_share,&n_text,&n_lib,&n_data,&n_dt);
> +      printf ("### n_rss=%d\n", n_rss);
> +      printf ("### n_resident=%d\n", n_resident);
> +      fclose (f);
> +    } 
> +  else
> +    return -1;

If you use TEST_VERIFY_EXIT as noted above then this whole
block of code just comes out of the 'if' and is called
unconditionally.

> +
> +  if (n_resident > 1000)
> +    return -1;

Needs a comment explaining why this is a failure.

TEST_VERIFY_EXIT (n_resident > 1000);

I do not suggest doing it this way.

I suggest sampling n_resident at the start *before*
any allocations happen in the test, then again at the
end, and looking for a difference that is small.

Checking the absolute value of n_resident will need
adjustment for all the other architectures that glibc
supports. While the difference in memory usage will
likely be constant for all architectures.

> +
> +  return 0;

> +}
> +
> +#define TEST_FUNCTION do_test ()
> +#include "../test-skeleton.c"

As Joseph pointed out this should use:

#include <support/test-driver.c>

> +
> 
> 
> 
> 
> 
> ------------------原始邮件------------------
> 发件人:JosephMyers <joseph@codesourcery.com>
> 收件人:贺洪军10036572;
> 抄送人:libc-alpha@sourceware.org <libc-alpha@sourceware.org>;
> 日 期 :2018年11月30日 01:24
> 主 题 :Re: Bug 15321 - malloc/free can't give the memory back to kernel when main_arena is discontinous
> On Thu, 29 Nov 2018, He.Hongjun@zte.com.cn wrote:
> 
>> Hello, I have fixed the  Bug 15321.
>> patch on glibc-2.28
> 
> Do you have an FSF copyright assignment?  (Are you covered by the one for
> "Chengdu Zhongxing Software Company Limited (ZTE)" dated 23 August 2018?)
> 
>> diff --git a/malloc/tst-discontinuous-main-arena.c b/malloc/tst-discontinuous-main-arena.c
>> old mode 100644
>> new mode 100755
>> index e69de29..3f6f0e7
>> --- a/malloc/tst-discontinuous-main-arena.c
>> +++ b/malloc/tst-discontinuous-main-arena.c
>> @@ -0,0 +1,128 @@
>> +/* Ensure that main arena of discontinuity can normal free memory.
>> +
>> +   The GNU C Library is free software; you can redistribute it and/or
> 
> Missing copyright notice, before the license notice.
> 
>> +#define TEST_FUNCTION do_test ()
>> +#include "../test-skeleton.c"
> 
> New tests should use <support/test-driver.c>, not the old-style
> test-skeleton.c.
> 
> --
> Joseph S. Myers
> joseph@codesourcery.com
>
He Hongjun Nov. 30, 2018, 6:09 a.m. UTC | #2
Hello, this is the new patch





------------------原始邮件------------------
发件人:CarlosO'Donell <carlos@redhat.com>
收件人:贺洪军10036572;joseph@codesourcery.com <joseph@codesourcery.com>;
抄送人:libc-alpha@sourceware.org <libc-alpha@sourceware.org>;
日 期 :2018年11月30日 11:05
主 题 :Re: 答复: Re: Bug 15321 - malloc/free can't give the memory back to kernel when main_arena is discontinous
On 11/29/18 8:08 PM, He.Hongjun@zte.com.cn wrote:
> yes, covered by the one for "Chengdu Zhongxing Software Company Limited (ZTE)" dated 23 August 2018.
>
> I have added the copyright notice in the tst-discontinuous-main-arena.c

I look forward to a v2.

(1) Include a detailed description of your changes.

At a high level can you describe what the patch does and
what it fixes? Such a description is required for the
git commit message and is an important part of establishing
the intentions of your technical change. A reviewer, like
myself, can then review that the code matches your high-level
itent (or design).

(2) Please attach the patch to the next email.

While the contribution checklist says there is a strong preference
to inline the patch in our email, this sometimes breaks the patch
because MUAs may convert tabs and spaces or wrap lines. For v2,
I suggest you commit your code to your local git repo, and then
send us `git format-patch HEAD~1` which will show us everything
you *would* have committed when you push the patch. This lets me
review everything including the commit message.
> 2018-11-28  Hehongjun  <He.Hongjun@zte.com.cn>
>
>     [BZ #15321]
>     * malloc/Makefile: add tst-discontinuous-main-arena.
>     * malloc/arena.c (heap_trim): add main arean
>     * malloc/malloc.c: modify discontinuous main arena
>     with sysmalloc, modify discontinuous main arena link heap.
>     with _int_free, free memory on discontinuous main arena link heap.
>     * malloc/tst-discontinuous-main-arena: New test.
>
> diff --git a/malloc/Makefile b/malloc/Makefile
> index 7d54bad..9d60ab1 100644
> --- a/malloc/Makefile
> +++ b/malloc/Makefile
> @@ -38,6 +38,7 @@ tests := mallocbug tst-malloc tst-valloc tst-calloc tst-obstack \
>       tst-malloc_info \
>       tst-malloc-too-large \
>       tst-malloc-stats-cancellation \
> +    tst-discontinuous-main-arena \

I think this is the wrong place to put this test, see notes bleow.

>
>  tests-static := \
>       tst-interpose-static-nothread \
> diff --git a/malloc/arena.c b/malloc/arena.c
> index 497ae47..343ef13 100755
> --- a/malloc/arena.c
> +++ b/malloc/arena.c
> @@ -634,6 +634,45 @@ heap_trim (heap_info *heap, size_t pad)
>        /*check_chunk(ar_ptr, top_chunk);*/
>      }
>
> +  if (ar_ptr == &main_arena && heap->prev == NULL)
> +    {
> +      char *old_top;
> +      old_top = (char *) (heap + 1);
> +      p = (mchunkptr) (old_top + sizeof (unsigned long));
> +
> +      misalign = (unsigned long) chunk2mem (p) & MALLOC_ALIGN_MASK;
> +      if (misalign > 0)
> +        p = ((char *) p) + MALLOC_ALIGNMENT - misalign;
> +
> +      if (top_chunk == (mchunkptr) p)
> +      {
> +          p = (mchunkptr) (*(unsigned long *) old_top);
> +          p = prev_chunk (p);
> +          new_size = chunksize (p) + MINSIZE;
> +
> +          if (!prev_inuse (p))
> +            new_size += prev_size (p);
> +
> +          if (new_size >= pad + MINSIZE + pagesz)
> +            {
> +              ar_ptr->system_mem -= heap->size;
> +              delete_heap (heap);
> +              if (!prev_inuse (p)) { /* consolidate backward */
> +                p = prev_chunk (p);
> +              unlink (ar_ptr, p, bck, fwd);
> +            }
> +
> +          top (ar_ptr) = top_chunk = (char *) p - 2 * SIZE_SZ;
> +          set_head (top_chunk, new_size | PREV_INUSE);
> +
> +          set_contiguous (ar_ptr);
> +          if ((unsigned long)(chunksize (ar_ptr->top)) >= (unsigned long)mp_.trim_threshold)
> +            systrim (mp_.trim_threshold, ar_ptr);
> +          return 0;
> +        }
> +      }
> +    }
> +

I need to spend some time reviewing this and matching it
with your intent. So I'll wait for v2 to review this.

>    /* Uses similar logic for per-thread arenas as the main arena with systrim
>       and _int_free by preserving the top pad and rounding down to the nearest
>       page.  */
> diff --git a/malloc/malloc.c b/malloc/malloc.c
> index e247c77..ad53ef7 100755
> --- a/malloc/malloc.c
> +++ b/malloc/malloc.c
> @@ -2388,7 +2388,7 @@ sysmalloc (INTERNAL_SIZE_T nb, mstate av)
>    assert ((unsigned long) (old_size) < (unsigned long) (nb + MINSIZE));
>
>
> -  if (av != &main_arena)
> +  if ((av != &main_arena) || (av == &main_arena && !contiguous (av)))
>      {
>        heap_info *old_heap, *heap;
>        size_t old_heap_size;
> @@ -2438,8 +2438,6 @@ sysmalloc (INTERNAL_SIZE_T nb, mstate av)
>          goto try_mmap;
>      }
>    else     /* av == main_arena */
> -
> -

Please avoid any white space changes which are not related to the
change you are making.

Please feel free to submit another patch to fix this independently
of the change.

>      { /* Request enough space for nb + pad + overhead */
>        size = nb + mp_.top_pad + MINSIZE;
>
> @@ -2491,35 +2489,62 @@ sysmalloc (INTERNAL_SIZE_T nb, mstate av)
>               and threshold limits, since the space will not be used as a
>               segregated mmap region.
>             */
> -
> -          /* Cannot merge with old top, so add its size back in */
> -          if (contiguous (av))
> -            size = ALIGN_UP (size + old_size, pagesize);
> -
> -          /* If we are relying on mmap as backup, then use larger units */
> -          if ((unsigned long) (size) < (unsigned long) (MMAP_AS_MORECORE_SIZE))
> -            size = MMAP_AS_MORECORE_SIZE;
> -
> -          /* Don't try if size wraps around 0 */
> -          if ((unsigned long) (size) > (unsigned long) (nb))
> -            {
> -              char *mbrk = (char *) (MMAP (0, size, PROT_READ | PROT_WRITE, 0));
> -
> -              if (mbrk != MAP_FAILED)
> +          heap_info *heap;
> +          long misalign;
> +          char *ptr;
> +          heap = new_heap (nb + (MINSIZE + sizeof (*heap) + MALLOC_ALIGNMENT), mp_.top_pad);
> +          if (!heap)
> +           {
> +             if (!tried_mmap)
> +                   goto try_mmap;
> +              else
>                  {
> -                  /* We do not need, and cannot use, another sbrk call to find end */
> -                  brk = mbrk;
> -                  snd_brk = brk + size;
> +                     __set_errno (ENOMEM);
> +             return 0;
> +                }
> +           }
> +
> +          heap->ar_ptr = av;
> +          heap->prev = NULL;
> +          av->system_mem += heap->size;
> +
> +          ptr = (char *) (heap + 1);
> +          ptr += sizeof (unsigned long);
> +          misalign = (unsigned long) chunk2mem (ptr) & MALLOC_ALIGN_MASK;
> +          if (misalign > 0)
> +            ptr += MALLOC_ALIGNMENT - misalign;
> +          /* Set up the new top.  */
> +          top (av) = ptr;
> +          set_head (top (av), (((char *) heap + heap->size) - ptr) | PREV_INUSE);
>
> -                  /*
> -                     Record that we no longer have a contiguous sbrk region.
> -                     After the first time mmap is used as backup, we do not
> -                     ever rely on contiguous space since this could incorrectly
> -                     bridge regions.
> -                   */
> -                  set_noncontiguous (av);
> -                }
> +          /* Setup fencepost and free the old top chunk with a multiple of
> +             MALLOC_ALIGNMENT in size. */
> +          /* The fencepost takes at least MINSIZE bytes, because it might
> +             become the top chunk again later.  Note that a footer is set
> +             up, too, although the chunk is marked in use. */
> +          old_size = (old_size - MINSIZE) & ~MALLOC_ALIGN_MASK;
> +          set_head (chunk_at_offset (old_top, old_size + 2 * SIZE_SZ), 0 | PREV_INUSE);
> +          *(unsigned long *)((heap + 1)) = chunk_at_offset (old_top, old_size + 2 * SIZE_SZ);
> +          if (old_size >= MINSIZE)
> +            {
> +              set_head (chunk_at_offset (old_top, old_size), (2 * SIZE_SZ) | PREV_INUSE);
> +              set_foot (chunk_at_offset (old_top, old_size), (2 * SIZE_SZ));
> +              set_head (old_top, old_size | PREV_INUSE);
> +              _int_free (av, old_top, 1);
> +            }
> +          else
> +            {
> +              set_head (old_top, (old_size + 2 * SIZE_SZ) | PREV_INUSE);
> +              set_foot (old_top, (old_size + 2 * SIZE_SZ));
>              }
> +
> +          /*
> +            Record that we no longer have a contiguous sbrk region.
> +            After the first time mmap is used as backup, we do not
> +            ever rely on contiguous space since this could incorrectly
> +            bridge regions.
> +            */
> +          set_noncontiguous (av);

Again, this needs more review and I'll try to get to that in your v2.

>          }
>
>        if (brk != (char *) (MORECORE_FAILURE))
> @@ -4347,7 +4372,7 @@ _int_free (mstate av, mchunkptr p, int have_lock)
>        if (atomic_load_relaxed (&av->have_fastchunks))
>      malloc_consolidate(av);
>
> -      if (av == &main_arena) {
> +      if ((av == &main_arena ) && contiguous (av)) {
>  #ifndef MORECORE_CANNOT_TRIM
>      if ((unsigned long)(chunksize(av->top)) >=
>          (unsigned long)(mp_.trim_threshold))
> diff --git a/malloc/tst-discontinuous-main-arena.c b/malloc/tst-discontinuous-main-arena.c
> old mode 100644
> new mode 100755
> index e69de29..ef7249f
> --- a/malloc/tst-discontinuous-main-arena.c
> +++ b/malloc/tst-discontinuous-main-arena.c

This file is linux-specific and parses /proc/self/statm.

I suggest:

sysdeps/unix/sysv/linux/tst-discontig-main-arena.c

Add it to:

sysdeps/unix/sysv/linux/Makefile

here (around line 10):
~~~
....
ifeq ($(subdir),malloc)
CFLAGS-malloc.c += -DMORECORE_CLEARS=2
tests += tst-discontig-main-arena
endif
....
~~~

> @@ -0,0 +1,133 @@

It should look like this:

/* <One sentence description.>
<Copyright line, with 2018 only because this is a new test.>
<Rest of the license>  */

We no longer use 'Contributed by' lines, we rely on ChangeLogs and
git commit messages to provide attribution.

I suggest:

/* Test that a discontinuous main arena can free through the gap.
Copyright (C) 2018 Free Software Foundation, Inc.
This file is part of the GNU C Library.


> +/* Copyright (C) 1999-2018 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +   Contributed by He.Hongjun@zte.com.cn, 2018.
> +
> +   Ensure that main arena of discontinuity can normal free memory.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public License as
> +   published by the Free Software Foundation; either version 2.1 of the
> +   License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; see the file COPYING.LIB.  If
> +   not, see <http://www.gnu.org/licenses/>.  */
> +
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +#include <unistd.h>
> +#include <sys/mman.h>
> +
> +#define MEM_SIZE 1024*128

This isn't a size, but rather a count? Should we call it BLK_COUNT?

> +#define BLK_SIZE 1024
> +#define TOP_PAD_SIZE 1024 * 128
> +

Test needs a block comment at the top explaining in detail the intent of the test.

> +static int
> +do_test (void)
> +{
> +  /* Get the peak physical memory usage. */
> +  FILE *f;
> +  unsigned n_rss = 0;
> +  unsigned n_resident = 0;
> +  unsigned n_share = 0;
> +  unsigned n_text = 0;
> +  unsigned n_lib = 0;
> +  unsigned n_data = 0;
> +  unsigned n_dt = 0;
> +  int i = 0;
> +
> +  f = fopen ("/proc/self/statm", "r");

Suggest:

TEST_VERIFY_EXIT (f != NULL);
fscanf (f,"%u %u %u %u %u %u %u",
&n_rss,&n_resident,&n_share,&n_text,&n_lib,&n_data,&n_dt);
printf ("n_rss=%d\n", n_rss);
printf ("n_resident=%d\n", n_resident);
fclose (f);

~~~
> +  if (f)
> +    {
> +      fscanf (f,"%u %u %u %u %u %u %u",
> +        &n_rss,&n_resident,&n_share,&n_text,&n_lib,&n_data,&n_dt);
> +      printf ("n_rss=%d\n", n_rss);
> +      printf ("n_resident=%d\n", n_resident);
> +      fclose (f);
> +    }
> +  else
> +    return -1;
~~~


> +
> +  void **mem1 = (void**) malloc (sizeof (void*) * MEM_SIZE);
> +  void **mem2 = (void**) malloc (sizeof (void*) * MEM_SIZE);
> +
> +  for (i = 0; i < MEM_SIZE; i++)
> +    {
> +      mem1[i] = malloc (BLK_SIZE);
> +      if (mem1[i] == NULL)
> +        {
> +          printf ("malloc(BLK_SIZE) failed.\n");
> +          return -1;
> +        }
> +      memset (mem1[i], 0xa, BLK_SIZE);
> +    }
> +
> +  /* Place a fence in front of the program break to make the main arena
> +     discoutinuous. Make sure we do not cover any region that have been
> +     accquire by sbrk. */

Suggest:

/* Place a private anonymous mapping after the process break region to
force the creation of a discontiguous main arena (a main arena with
a hole in the middle).  This operation must be done with care to
ensure that the mapping does not cover any regions that have already
been acquired by sbrk.  */

> +  long page_sz = sysconf (_SC_PAGESIZE);
> +  void *fence_addr = mem1[MEM_SIZE - 1] + TOP_PAD_SIZE + BLK_SIZE + page_sz;
> +  fence_addr = (void *)((long)fence_addr & ~(page_sz - 1));

I believe you can include 'libc-pointer-arith.h' and use PTR_ALIGN_UP() for this.

> +
> +  void *tt = mmap (fence_addr, page_sz, PROT_READ | PROT_WRITE,
> +               MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0);
> +  if (tt == MAP_FAILED || tt != fence_addr)
> +    {
> +      printf ("mmap(fence_addr, page_sz, PROT_READ | PROT_WRITE, \
> +                MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0) failed..\n");
> +      return -1;

All of this can become:

FAIL_EXIT1 ("...", ..., ..., ...);

See support/check.h.

You can use the following to access the routines:

#include <support/check.h>
#include <support/support.h>

(at the top with your other includes).
> +    }
> +

Needs some comments explaining what is is being attempted here.

> +  for (i = 0; i < MEM_SIZE; i++)
> +    {
> +      mem2[i] = malloc (BLK_SIZE);


> +      if (mem2[i] == NULL)
> +        {
> +          printf ("malloc(BLK_SIZE) failed.\n");
> +          return -1;
> +        }

Suggest just using `TEST_VERIFY_EXIT (mem2[i] != NULL);`


> +      memset (mem2[i], 0xb, BLK_SIZE);
> +    }
> +
> +  for (i = 0; i < MEM_SIZE; i++)
> +    {
> +      free (mem1[i]);
> +      mem1[i] = NULL;
> +    }
> +  free (mem1);
> +
> +  for (i = 0; i < MEM_SIZE; i++)
> +    {
> +      free (mem2[i]);
> +      mem2[i] = NULL;
> +    }
> +  free (mem2);
> +
> +  f = fopen ("/proc/self/statm", "r");

TEST_VERIFY_EXIT (f != NULL);

> +  if (f)
> +    {
> +      fscanf (f,"%u %u %u %u %u %u %u",
> +        &n_rss,&n_resident,&n_share,&n_text,&n_lib,&n_data,&n_dt);
> +      printf ("### n_rss=%d\n", n_rss);
> +      printf ("### n_resident=%d\n", n_resident);
> +      fclose (f);
> +    }
> +  else
> +    return -1;

If you use TEST_VERIFY_EXIT as noted above then this whole
block of code just comes out of the 'if' and is called
unconditionally.

> +
> +  if (n_resident > 1000)
> +    return -1;

Needs a comment explaining why this is a failure.

TEST_VERIFY_EXIT (n_resident > 1000);

I do not suggest doing it this way.

I suggest sampling n_resident at the start *before*
any allocations happen in the test, then again at the
end, and looking for a difference that is small.

Checking the absolute value of n_resident will need
adjustment for all the other architectures that glibc
supports. While the difference in memory usage will
likely be constant for all architectures.

> +
> +  return 0;

> +}
> +
> +#define TEST_FUNCTION do_test ()
> +#include "../test-skeleton.c"

As Joseph pointed out this should use:

#include <support/test-driver.c>

> +
>
>
>
>
>
> ------------------原始邮件------------------
> 发件人:JosephMyers <joseph@codesourcery.com>
> 收件人:贺洪军10036572;
> 抄送人:libc-alpha@sourceware.org <libc-alpha@sourceware.org>;
> 日 期 :2018年11月30日 01:24
> 主 题 :Re: Bug 15321 - malloc/free can't give the memory back to kernel when main_arena is discontinous
> On Thu, 29 Nov 2018, He.Hongjun@zte.com.cn wrote:
>
>> Hello, I have fixed the  Bug 15321.
>> patch on glibc-2.28
>
> Do you have an FSF copyright assignment?  (Are you covered by the one for
> "Chengdu Zhongxing Software Company Limited (ZTE)" dated 23 August 2018?)
>
>> diff --git a/malloc/tst-discontinuous-main-arena.c b/malloc/tst-discontinuous-main-arena.c
>> old mode 100644
>> new mode 100755
>> index e69de29..3f6f0e7
>> --- a/malloc/tst-discontinuous-main-arena.c
>> +++ b/malloc/tst-discontinuous-main-arena.c
>> @@ -0,0 +1,128 @@
>> +/* Ensure that main arena of discontinuity can normal free memory.
>> +
>> +   The GNU C Library is free software; you can redistribute it and/or
>
> Missing copyright notice, before the license notice.
>
>> +#define TEST_FUNCTION do_test ()
>> +#include "../test-skeleton.c"
>
> New tests should use <support/test-driver.c>, not the old-style
> test-skeleton.c.
>
> --
> Joseph S. Myers
> joseph@codesourcery.com
>


--
Cheers,
Carlos.
Joseph Myers Nov. 30, 2018, 3:27 p.m. UTC | #3
On Fri, 30 Nov 2018, He.Hongjun@zte.com.cn wrote:

> Hello, this is the new patch

This appears not to have addressed most of the comments that Carlos and I 
had.  Please review our comments carefully and make sure you send the 
correct version of the patch that addresses *all* those comments, or state 
explicitly if you disagree with a particular comment.
diff mbox

Patch

diff --git a/malloc/Makefile b/malloc/Makefile
index 7d54bad..9d60ab1 100644
--- a/malloc/Makefile
+++ b/malloc/Makefile
@@ -38,6 +38,7 @@  tests := mallocbug tst-malloc tst-valloc tst-calloc tst-obstack \
 	 tst-malloc_info \
 	 tst-malloc-too-large \
 	 tst-malloc-stats-cancellation \
+    tst-discontinuous-main-arena \

 tests-static := \
 	 tst-interpose-static-nothread \
diff --git a/malloc/arena.c b/malloc/arena.c
index 497ae47..343ef13 100755
--- a/malloc/arena.c
+++ b/malloc/arena.c
@@ -634,6 +634,45 @@  heap_trim (heap_info *heap, size_t pad)
       /*check_chunk(ar_ptr, top_chunk);*/
     }

+  if (ar_ptr == &main_arena && heap->prev == NULL)
+    {
+      char *old_top;
+      old_top = (char *) (heap + 1);
+      p = (mchunkptr) (old_top + sizeof (unsigned long));
+
+      misalign = (unsigned long) chunk2mem (p) & MALLOC_ALIGN_MASK;
+      if (misalign > 0)
+        p = ((char *) p) + MALLOC_ALIGNMENT - misalign;
+
+      if (top_chunk == (mchunkptr) p)
+  	{
+          p = (mchunkptr) (*(unsigned long *) old_top);
+          p = prev_chunk (p);
+          new_size = chunksize (p) + MINSIZE;
+
+          if (!prev_inuse (p))
+            new_size += prev_size (p);	
+	  
+          if (new_size >= pad + MINSIZE + pagesz)
+            {
+              ar_ptr->system_mem -= heap->size;
+              delete_heap (heap);
+              if (!prev_inuse (p)) { /* consolidate backward */
+                p = prev_chunk (p);
+              unlink (ar_ptr, p, bck, fwd);
+            }
+
+          top (ar_ptr) = top_chunk = (char *) p - 2 * SIZE_SZ;
+          set_head (top_chunk, new_size | PREV_INUSE);
+
+          set_contiguous (ar_ptr);
+          if ((unsigned long)(chunksize (ar_ptr->top)) >= (unsigned long)mp_.trim_threshold)
+            systrim (mp_.trim_threshold, ar_ptr);			
+          return 0;
+        }
+      }
+    }
+
   /* Uses similar logic for per-thread arenas as the main arena with systrim
      and _int_free by preserving the top pad and rounding down to the nearest
      page.  */
diff --git a/malloc/malloc.c b/malloc/malloc.c
index e247c77..ad53ef7 100755
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -2388,7 +2388,7 @@  sysmalloc (INTERNAL_SIZE_T nb, mstate av)
   assert ((unsigned long) (old_size) < (unsigned long) (nb + MINSIZE));


-  if (av != &main_arena)
+  if ((av != &main_arena) || (av == &main_arena && !contiguous (av)))
     {
       heap_info *old_heap, *heap;
       size_t old_heap_size;
@@ -2438,8 +2438,6 @@  sysmalloc (INTERNAL_SIZE_T nb, mstate av)
         goto try_mmap;
     }
   else     /* av == main_arena */
-
-
     { /* Request enough space for nb + pad + overhead */
       size = nb + mp_.top_pad + MINSIZE;

@@ -2491,35 +2489,62 @@  sysmalloc (INTERNAL_SIZE_T nb, mstate av)
              and threshold limits, since the space will not be used as a
              segregated mmap region.
            */
-
-          /* Cannot merge with old top, so add its size back in */
-          if (contiguous (av))
-            size = ALIGN_UP (size + old_size, pagesize);
-
-          /* If we are relying on mmap as backup, then use larger units */
-          if ((unsigned long) (size) < (unsigned long) (MMAP_AS_MORECORE_SIZE))
-            size = MMAP_AS_MORECORE_SIZE;
-
-          /* Don't try if size wraps around 0 */
-          if ((unsigned long) (size) > (unsigned long) (nb))
-            {
-              char *mbrk = (char *) (MMAP (0, size, PROT_READ | PROT_WRITE, 0));
-
-              if (mbrk != MAP_FAILED)
+          heap_info *heap;
+          long misalign;
+          char *ptr;
+          heap = new_heap (nb + (MINSIZE + sizeof (*heap) + MALLOC_ALIGNMENT), mp_.top_pad);
+          if (!heap)
+   	    {
+   	      if (!tried_mmap)       	        
+       	        goto try_mmap;
+              else
                 {
-                  /* We do not need, and cannot use, another sbrk call to find end */
-                  brk = mbrk;
-                  snd_brk = brk + size;
+       	          __set_errno (ENOMEM);
+   		  return 0;
+                } 
+   	    }
+			
+          heap->ar_ptr = av;
+          heap->prev = NULL;
+          av->system_mem += heap->size;
+	  
+          ptr = (char *) (heap + 1);
+          ptr += sizeof (unsigned long);
+          misalign = (unsigned long) chunk2mem (ptr) & MALLOC_ALIGN_MASK;
+          if (misalign > 0)
+            ptr += MALLOC_ALIGNMENT - misalign;
+          /* Set up the new top.  */		  
+          top (av) = ptr;
+          set_head (top (av), (((char *) heap + heap->size) - ptr) | PREV_INUSE);

-                  /*
-                     Record that we no longer have a contiguous sbrk region.
-                     After the first time mmap is used as backup, we do not
-                     ever rely on contiguous space since this could incorrectly
-                     bridge regions.
-                   */
-                  set_noncontiguous (av);
-                }
+          /* Setup fencepost and free the old top chunk with a multiple of
+             MALLOC_ALIGNMENT in size. */
+          /* The fencepost takes at least MINSIZE bytes, because it might
+             become the top chunk again later.  Note that a footer is set
+             up, too, although the chunk is marked in use. */
+          old_size = (old_size - MINSIZE) & ~MALLOC_ALIGN_MASK;
+          set_head (chunk_at_offset (old_top, old_size + 2 * SIZE_SZ), 0 | PREV_INUSE);
+          *(unsigned long *)((heap + 1)) = chunk_at_offset (old_top, old_size + 2 * SIZE_SZ);
+          if (old_size >= MINSIZE)
+            {
+              set_head (chunk_at_offset (old_top, old_size), (2 * SIZE_SZ) | PREV_INUSE);
+              set_foot (chunk_at_offset (old_top, old_size), (2 * SIZE_SZ));
+              set_head (old_top, old_size | PREV_INUSE);			  
+              _int_free (av, old_top, 1);
+            }
+          else
+            {
+              set_head (old_top, (old_size + 2 * SIZE_SZ) | PREV_INUSE);
+              set_foot (old_top, (old_size + 2 * SIZE_SZ));
             }
+
+          /*
+            Record that we no longer have a contiguous sbrk region.
+            After the first time mmap is used as backup, we do not
+            ever rely on contiguous space since this could incorrectly
+            bridge regions.
+            */
+          set_noncontiguous (av);
         }

       if (brk != (char *) (MORECORE_FAILURE))
@@ -4347,7 +4372,7 @@  _int_free (mstate av, mchunkptr p, int have_lock)
       if (atomic_load_relaxed (&av->have_fastchunks))
 	malloc_consolidate(av);

-      if (av == &main_arena) {
+      if ((av == &main_arena ) && contiguous (av)) {
 #ifndef MORECORE_CANNOT_TRIM
 	if ((unsigned long)(chunksize(av->top)) >=
 	    (unsigned long)(mp_.trim_threshold))
diff --git a/malloc/tst-discontinuous-main-arena.c b/malloc/tst-discontinuous-main-arena.c
old mode 100644
new mode 100755
index e69de29..ef7249f
--- a/malloc/tst-discontinuous-main-arena.c
+++ b/malloc/tst-discontinuous-main-arena.c
@@ -0,0 +1,133 @@ 
+/* Copyright (C) 1999-2018 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+   Contributed by He.Hongjun@zte.com.cn, 2018.
+   
+   Ensure that main arena of discontinuity can normal free memory.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public License as
+   published by the Free Software Foundation; either version 2.1 of the
+   License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; see the file COPYING.LIB.  If
+   not, see <http://www.gnu.org/licenses/>.  */
+
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <sys/mman.h>
+
+#define MEM_SIZE 1024*128
+#define BLK_SIZE 1024
+#define TOP_PAD_SIZE 1024 * 128
+
+static int
+do_test (void)
+{
+  /* Get the peak physical memory usage. */
+  FILE *f;  
+  unsigned n_rss = 0;
+  unsigned n_resident = 0;
+  unsigned n_share = 0;
+  unsigned n_text = 0;
+  unsigned n_lib = 0;
+  unsigned n_data = 0;
+  unsigned n_dt = 0;
+  int i = 0;
+
+  f = fopen ("/proc/self/statm", "r");
+  if (f)
+    {
+      fscanf (f,"%u %u %u %u %u %u %u",
+        &n_rss,&n_resident,&n_share,&n_text,&n_lib,&n_data,&n_dt);
+      printf ("n_rss=%d\n", n_rss);
+      printf ("n_resident=%d\n", n_resident);
+      fclose (f);
+    } 
+  else
+    return -1;
+
+  void **mem1 = (void**) malloc (sizeof (void*) * MEM_SIZE); 
+  void **mem2 = (void**) malloc (sizeof (void*) * MEM_SIZE); 
+
+  for (i = 0; i < MEM_SIZE; i++)
+    {
+      mem1[i] = malloc (BLK_SIZE);
+      if (mem1[i] == NULL)
+        {
+          printf ("malloc(BLK_SIZE) failed.\n");
+          return -1;
+        }
+      memset (mem1[i], 0xa, BLK_SIZE);
+    }
+
+  /* Place a fence in front of the program break to make the main arena
+     discoutinuous. Make sure we do not cover any region that have been
+     accquire by sbrk. */
+  long page_sz = sysconf (_SC_PAGESIZE);
+  void *fence_addr = mem1[MEM_SIZE - 1] + TOP_PAD_SIZE + BLK_SIZE + page_sz;
+  fence_addr = (void *)((long)fence_addr & ~(page_sz - 1));
+
+  void *tt = mmap (fence_addr, page_sz, PROT_READ | PROT_WRITE, 
+               MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0);
+  if (tt == MAP_FAILED || tt != fence_addr)
+    {
+      printf ("mmap(fence_addr, page_sz, PROT_READ | PROT_WRITE, \
+                MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0) failed.\n");
+      return -1;    
+    }
+
+  for (i = 0; i < MEM_SIZE; i++) 
+    {
+      mem2[i] = malloc (BLK_SIZE);
+      if (mem2[i] == NULL) 
+        {
+          printf ("malloc(BLK_SIZE) failed.\n");
+          return -1;
+        }
+      memset (mem2[i], 0xb, BLK_SIZE);
+    }  
+
+  for (i = 0; i < MEM_SIZE; i++) 
+    {
+      free (mem1[i]);
+      mem1[i] = NULL;	
+    }
+  free (mem1);  
+  
+  for (i = 0; i < MEM_SIZE; i++) 
+    {
+      free (mem2[i]);
+      mem2[i] = NULL;	
+    }
+  free (mem2);
+
+  f = fopen ("/proc/self/statm", "r");
+  if (f)
+    {
+      fscanf (f,"%u %u %u %u %u %u %u",
+        &n_rss,&n_resident,&n_share,&n_text,&n_lib,&n_data,&n_dt);
+      printf ("### n_rss=%d\n", n_rss);
+      printf ("### n_resident=%d\n", n_resident);
+      fclose (f);
+    } 
+  else
+    return -1;
+
+  if (n_resident > 1000)
+    return -1;
+
+  return 0;
+}
+
+#define TEST_FUNCTION do_test ()
+#include "../test-skeleton.c"
+