January 2015 - Darwin-dev - lists.apple.com

Re: Question about MADV_FREE range in tiny_free_no_lock.
by cee1 11 Jan '15

11 Jan '15

2015-01-07 22:16 GMT+08:00 cee1 <fykcee1(a)gmail.com>: > Hi all, > > http://www.opensource.apple.com/source/libmalloc/libmalloc-53.1.1/src/magaz…, > in function tiny_free_no_lock(): > > """ > size_t free_header_size = sizeof(free_list_t) + sizeof(msize_t); > uintptr_t safe_ptr = (uintptr_t)ptr + free_header_size; > uintptr_t round_safe = round_page_quanta(safe_ptr); > > // Calcuate the last page in the coalesced block that would be safe to > mark MADV_FREE > size_t free_tail_size = sizeof(msize_t); > uintptr_t safe_extent = (uintptr_t)ptr + TINY_BYTES_FOR_MSIZE(msize) - > free_tail_size; > uintptr_t trunc_extent = trunc_page_quanta(safe_extent); > > // The newly freed block may complete a span of bytes that cover a > page. Mark it with MADV_FREE. > if (round_safe < trunc_extent) { // Coalesced area covers a page (perhaps many) > // Extend the freed block by the free region header and tail > sizes to include pages > // we may have coalesced that no longer host free region tails > and headers. > // This may extend over in-use ranges, but the MIN/MAX > clamping below will fix that up. > uintptr_t lo = trunc_page_quanta((uintptr_t)original_ptr - > free_tail_size); > uintptr_t hi = round_page_quanta((uintptr_t)original_ptr + > original_size + free_header_size); > > uintptr_t free_lo = MAX(round_safe, lo); > uintptr_t free_hi = MIN(trunc_extent, hi); > """ > > Here 'ptr' == previous_free_block_if_exists + 'original_ptr' + > next_free_block_if_exists, hence a possible bigger block. IMHO, > MADV_FREE pages within this block(except the header and tail) should > be ok, why we need to shrink the range with MAX and MIN? Also it > should always be lo >= round_safe, trunc_extent >= hi? Besides that, I find it will skip `free_header_size=sizeof(free_list_t) + sizeof(msize_t)` while doing MADV_FREE in small_free_no_lock(), IMHO, msize is already stored in `small_meta_words`, so the actual size needs to skip is `free_header_size=sizeof(free_list_t)`. (small_meta_header_set_is_free() even does not store a valid value in the space behind free_list_t) Also, in small_free_no_lock() (line 4450, src/magazine_malloc.c), the invocation of small_meta_header_set_is_free can be eliminated, since it is called indirectly in the next line: `small_free_list_add_ptr(...)`. -- Regards, - cee1 _______________________________________________ Do not post admin requests to the list. They will be ignored. Darwin-dev mailing list (Darwin-dev(a)lists.apple.com) Help/Unsubscribe/Update your Subscription: https://lists.apple.com/mailman/options/darwin-dev/site_archiver%40lists.ap… This email sent to site_archiver(a)lists.apple.com

1 0

Question about MADV_FREE range in tiny_free_no_lock.
by cee1 07 Jan '15

07 Jan '15

Hi all, http://www.opensource.apple.com/source/libmalloc/libmalloc-53.1.1/src/magaz…, in function tiny_free_no_lock(): """ size_t free_header_size = sizeof(free_list_t) + sizeof(msize_t); uintptr_t safe_ptr = (uintptr_t)ptr + free_header_size; uintptr_t round_safe = round_page_quanta(safe_ptr); // Calcuate the last page in the coalesced block that would be safe to mark MADV_FREE size_t free_tail_size = sizeof(msize_t); uintptr_t safe_extent = (uintptr_t)ptr + TINY_BYTES_FOR_MSIZE(msize) - free_tail_size; uintptr_t trunc_extent = trunc_page_quanta(safe_extent); // The newly freed block may complete a span of bytes that cover a page. Mark it with MADV_FREE. if (round_safe < trunc_extent) { // Coalesced area covers a page (perhaps many) // Extend the freed block by the free region header and tail sizes to include pages // we may have coalesced that no longer host free region tails and headers. // This may extend over in-use ranges, but the MIN/MAX clamping below will fix that up. uintptr_t lo = trunc_page_quanta((uintptr_t)original_ptr - free_tail_size); uintptr_t hi = round_page_quanta((uintptr_t)original_ptr + original_size + free_header_size); uintptr_t free_lo = MAX(round_safe, lo); uintptr_t free_hi = MIN(trunc_extent, hi); """ Here 'ptr' == previous_free_block_if_exists + 'original_ptr' + next_free_block_if_exists, hence a possible bigger block. IMHO, MADV_FREE pages within this block(except the header and tail) should be ok, why we need to shrink the range with MAX and MIN? Also it should always be lo >= round_safe, trunc_extent >= hi? BTW, we release the lock while doing MADV_FREE: """ SZONE_MAGAZINE_PTR_UNLOCK(szone, tiny_mag_ptr); madvise_free_range(szone, region, free_lo, free_hi, &szone->last_tiny_advise); SZONE_MAGAZINE_PTR_LOCK(szone, tiny_mag_ptr); """ Does that mean the MADV_FREE operation will spend much time? -- Regards, - cee1 _______________________________________________ Do not post admin requests to the list. They will be ignored. Darwin-dev mailing list (Darwin-dev(a)lists.apple.com) Help/Unsubscribe/Update your Subscription: https://lists.apple.com/mailman/options/darwin-dev/site_archiver%40lists.ap… This email sent to site_archiver(a)lists.apple.com

1 0

Re: libmalloc: forget to free something in nano_try_madvise in nano_malloc?
by Greg Parker 05 Jan '15

05 Jan '15

> On Dec 23, 2014, at 12:22 AM, cee1 <fykcee1(a)gmail.com> wrote: > > Hi all, > > I'm interesting to the internal of malloc in OS X and learning some > code snippets of it, and I find it seems forget to free page_bitarray > in nano_try_madvise() > (http://www.opensource.apple.com/source/libmalloc/libmalloc-53.1.1/src/nano_…, > line 1395): > > """ > bitarray_t slot_bitarray = bitarray_create(log_size); > bitarray_t page_bitarray = bitarray_create(log_page_count); > > if (!slot_bitarray) { > /******* Shouldn't we free(page_bitarray) here？*******/ > return bytes_toward_goal; > } > > if (!page_bitarray) { > free(slot_bitarray); > return bytes_toward_goal; > } > """ Your analysis looks correct to me. You should file a bug report. In practice this should not be a serious problem. A process that fails to allocate a bit array is almost certainly going to die very soon due to other out-of-memory failures. > BTW, in segregated_band_grow(), does it miss an unmap call if return > another vm area that's not what asked for? > > """ > kern_return_t kr = mach_vm_map(mach_task_self(), &vm_addr, BAND_SIZE, > 0, VM_MAKE_TAG(VM_MEMORY_MALLOC_NANO), > MEMORY_OBJECT_NULL, 0, FALSE, > VM_PROT_DEFAULT, VM_PROT_ALL, VM_INHERIT_DEFAULT); > > void *q = (uintptr_t)vm_addr; > if (kr || q != (void *)(p & ~((uintptr_t)(BAND_SIZE - 1)))) // > Must get exactly what we asked for > return FALSE; > > """ mach_vm_map() with that set of parameters is not allowed to return an address that is not the requested address. That part of the check will never fail unless there is a bug in mach_vm_map(). It would make sense to rearrange or annotate the code to clarify the check, but it should never result in a memory leak. -- Greg Parker gparker(a)apple.com Runtime Wrangler _______________________________________________ Do not post admin requests to the list. They will be ignored. Darwin-dev mailing list (Darwin-dev(a)lists.apple.com) Help/Unsubscribe/Update your Subscription: https://lists.apple.com/mailman/options/darwin-dev/site_archiver%40lists.ap… This email sent to site_archiver(a)lists.apple.com

1 0