This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] calloc should not duplicate malloc logic.
- From: OndÅej BÃlka <neleai at seznam dot cz>
- To: Siddhesh Poyarekar <siddhesh at redhat dot com>
- Cc: libc-alpha at sourceware dot org
- Date: Wed, 26 Feb 2014 20:47:32 +0100
- Subject: Re: [PATCH] calloc should not duplicate malloc logic.
- Authentication-results: sourceware.org; auth=none
- References: <20140221150417 dot GA4198 at domone dot podge> <20140226143648 dot GA32752 at spoyarek dot pnq dot redhat dot com> <20140226162521 dot GA24933 at domone dot podge> <20140226165123 dot GA6419 at spoyarek dot pnq dot redhat dot com> <20140226172412 dot GA18515 at domone dot podge> <20140226174050 dot GF6419 at spoyarek dot pnq dot redhat dot com> <20140226185116 dot GB24933 at domone dot podge>
On Wed, Feb 26, 2014 at 07:51:16PM +0100, OndÅej BÃlka wrote:
> On Wed, Feb 26, 2014 at 11:10:50PM +0530, Siddhesh Poyarekar wrote:
> > On Wed, Feb 26, 2014 at 06:24:12PM +0100, OndÅej BÃlka wrote:
> > > I did not said so, I said there could be minor regression and asked if
> > > that is problem. Also there is problem of synchronization as independent
> > > modifications would conflict and I want minimize these issue before they
> > > happen
> >
> > Right, and I opined that it is a problem. There need to be more
> > opinions on this so that a consensus can be achieved.
> >
> > > > You haven't understood why this is a bad idea. The pages will have
> > > > been zeroed twice with your patch, once by mmap and again with memset.
> > > >
> > > I understood you perfecly and you are wrong here. Current implementation
> > > also zeroes memory twice as this example demonstrates. Your point is
> > > invalid here.
> >
> > Your example does not demonstrate that. Your example only
> > demonstrates that the allocated memory is being touched, which is
> > obvious since the metadata is written to the chunk. I am surprised
> > that the kernel decided to page in the entire block for it though.
> >
> Its not kernel, calloc does that. After debugging a while I found its
> because we installed a malloc_hook_ini that skip this path, so I assumed
> that calloc design is broken. I will send revised patch.
>
A faster way would be additionally apply following. There are three
parts covered, first one is locking and libc_malloc has almost identical
code as this. Second is mmap that I for some reason forgotten. Third
part is optimizing memset which should go away as it contains
assumptions that will be broken, like that chunk is old multiple of 8.
* malloc/malloc.c ( __libc_calloc): Add back mmaped memory
handling.
diff --git a/malloc/malloc.c b/malloc/malloc.c
index 74ad92d..d2d3447 100644
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -3143,6 +3143,7 @@ __libc_calloc (size_t n, size_t elem_size)
{
INTERNAL_SIZE_T bytes;
void *mem;
+ mchunkptr p;
/* size_t is unsigned so the behavior on overflow is defined. */
bytes = n * elem_size;
@@ -3169,6 +3170,17 @@ __libc_calloc (size_t n, size_t elem_size)
if (mem == 0)
return 0;
+ p = mem2chunk (mem);
+
+ /* Optional case in which clearing not necessary */
+ if (chunk_is_mmapped (p))
+ {
+ if (__builtin_expect (perturb_byte, 0))
+ return memset (mem, 0, bytes);
+
+ return mem;
+ }
+
return memset (mem, 0, bytes);
}