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: Will Newton <will dot newton at linaro dot org>
- To: Ondřej Bílka <neleai at seznam dot cz>
- Cc: Siddhesh Poyarekar <siddhesh at redhat dot com>, libc-alpha <libc-alpha at sourceware dot org>
- Date: Mon, 3 Mar 2014 06:02:16 +0000
- 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> <20140226194732 dot GB19987 at domone dot podge>
On 26 February 2014 19:47, OndÅej BÃlka <neleai@seznam.cz> wrote:
> 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.
My personal feeling is that the original patch did not meet the
requirements of consensus and should be reverted on that basis, rather
than continuing to patch up the results.
--
Will Newton
Toolchain Working Group, Linaro