diff --git a/tcmalloc/cpu_cache.h b/tcmalloc/cpu_cache.h index 6af5c82bb..fde1d7254 100644 --- a/tcmalloc/cpu_cache.h +++ b/tcmalloc/cpu_cache.h @@ -1590,9 +1590,10 @@ void CpuCache::ResizeSlabIfNeeded() ABSL_NO_THREAD_SAFETY_ANALYSIS { dynamic_slab_info_.madvise_failed_bytes.fetch_add( info.old_slabs_size, std::memory_order_relaxed); } - // TODO(b/122551676): Add a return value to SystemRelease and consume it - // here. - SystemRelease(info.old_slabs, info.old_slabs_size); + if (!SystemRelease(info.old_slabs, info.old_slabs_size)) { + dynamic_slab_info_.madvise_failed_bytes.fetch_add( + info.old_slabs_size, std::memory_order_relaxed); + } forwarder_.ArenaReportNonresident(info.old_slabs_size, info.reused_bytes); } diff --git a/tcmalloc/page_heap.cc b/tcmalloc/page_heap.cc index adbb4058c..e13286e14 100644 --- a/tcmalloc/page_heap.cc +++ b/tcmalloc/page_heap.cc @@ -370,11 +370,15 @@ Length PageHeap::ReleaseLastNormalSpan(SpanListPair* slist) { pageheap_lock.Unlock(); const Length n = s->num_pages(); - SystemRelease(s->start_address(), s->bytes_in_span()); + bool success = SystemRelease(s->start_address(), s->bytes_in_span()); pageheap_lock.Lock(); - stats_.free_bytes -= s->bytes_in_span(); - s->set_location(Span::ON_RETURNED_FREELIST); + if (ABSL_PREDICT_TRUE(success)) { + stats_.free_bytes -= s->bytes_in_span(); + s->set_location(Span::ON_RETURNED_FREELIST); + } else { + s->set_location(Span::ON_NORMAL_FREELIST); + } MergeIntoFreeList(s); // Coalesces if possible. return n; } @@ -446,7 +450,7 @@ bool PageHeap::GrowHeap(Length n) { // Make sure pagemap has entries for all of the new pages. // Plus ensure one before and one after so coalescing code // does not need bounds-checking. - if (pagemap_->Ensure(p - Length(1), n + Length(2))) { + if (ABSL_PREDICT_TRUE(pagemap_->Ensure(p - Length(1), n + Length(2)))) { // Pretend the new area is allocated and then return it to cause // any necessary coalescing to occur. Span* span = Span::New(p, n); @@ -458,8 +462,10 @@ bool PageHeap::GrowHeap(Length n) { } else { // We could not allocate memory within the pagemap. // Note the following leaks virtual memory, but at least it gets rid of - // the underlying physical memory. - SystemRelease(ptr, actual_size); + // the underlying physical memory. If SystemRelease fails, there's little + // we can do (we couldn't allocate for Ensure), but we have the consolation + // that the memory has not been touched (so it is likely not populated). + (void)SystemRelease(ptr, actual_size); return false; } } diff --git a/tcmalloc/system-alloc.h b/tcmalloc/system-alloc.h index fc8a02e57..7a54475f7 100644 --- a/tcmalloc/system-alloc.h +++ b/tcmalloc/system-alloc.h @@ -63,9 +63,7 @@ int SystemReleaseErrors(); // be released, partial pages will not.) // // Returns true on success. -// -// TODO(b/122551676): Mark this ABSL_MUST_USE_RESULT. -bool SystemRelease(void* start, size_t length); +ABSL_MUST_USE_RESULT bool SystemRelease(void* start, size_t length); // This call is the inverse of SystemRelease: the pages in this range // are in use and should be faulted in. (In principle this is a