Commit a110cb9f authored by Bartek Nowierski's avatar Bartek Nowierski Committed by Commit Bot

Disable PCScan temporarily

Current issues:
- PA_DCHECK fires when partition page isn't 16kB
- pages for quarantine bitmaps are committed unconditionally,
  even if the feature is never enabled

These need to be addressed before re-enabling.

Bug: 1144990, 11297512
Change-Id: Ia5fada4d84f5d9c28b5768322aacbccc72f5f487
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2515506
Commit-Queue: Bartek Nowierski <bartekn@chromium.org>
Commit-Queue: Kentaro Hara <haraken@chromium.org>
Reviewed-by: default avatarHannes Payer <hpayer@chromium.org>
Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Auto-Submit: Bartek Nowierski <bartekn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#823501}
parent 4ae64045
...@@ -20,10 +20,12 @@ const Feature kPartitionAllocGigaCage{"PartitionAllocGigaCage32bit", ...@@ -20,10 +20,12 @@ const Feature kPartitionAllocGigaCage{"PartitionAllocGigaCage32bit",
FEATURE_DISABLED_BY_DEFAULT}; FEATURE_DISABLED_BY_DEFAULT};
#endif #endif
#if ALLOW_ENABLING_PCSCAN
// If enabled, PCScan is turned on by default for all partitions that don't // If enabled, PCScan is turned on by default for all partitions that don't
// disable it explicitly. // disable it explicitly.
const Feature kPartitionAllocPCScan{"PartitionAllocPCScan", const Feature kPartitionAllocPCScan{"PartitionAllocPCScan",
FEATURE_DISABLED_BY_DEFAULT}; FEATURE_DISABLED_BY_DEFAULT};
#endif
} // namespace features } // namespace features
} // namespace base } // namespace base
...@@ -19,6 +19,8 @@ ...@@ -19,6 +19,8 @@
#include <VersionHelpers.h> #include <VersionHelpers.h>
#endif #endif
#define ALLOW_ENABLING_PCSCAN 0
namespace base { namespace base {
struct Feature; struct Feature;
...@@ -61,7 +63,7 @@ ALWAYS_INLINE bool IsPartitionAllocGigaCageEnabled() { ...@@ -61,7 +63,7 @@ ALWAYS_INLINE bool IsPartitionAllocGigaCageEnabled() {
} }
ALWAYS_INLINE bool IsPartitionAllocPCScanEnabled() { ALWAYS_INLINE bool IsPartitionAllocPCScanEnabled() {
#if !defined(PA_HAS_64_BITS_POINTERS) #if !defined(PA_HAS_64_BITS_POINTERS) || !ALLOW_ENABLING_PCSCAN
return false; return false;
#endif #endif
// TODO(bikineev): Calling this function can allocate which can cause // TODO(bikineev): Calling this function can allocate which can cause
......
...@@ -295,7 +295,12 @@ ALWAYS_INLINE void* PartitionBucket<thread_safe>::AllocNewSlotSpan( ...@@ -295,7 +295,12 @@ ALWAYS_INLINE void* PartitionBucket<thread_safe>::AllocNewSlotSpan(
char* quarantine_bitmaps = tag_bitmap + ReservedTagBitmapSize(); char* quarantine_bitmaps = tag_bitmap + ReservedTagBitmapSize();
const size_t quarantine_bitmaps_size = const size_t quarantine_bitmaps_size =
root->scannable ? 2 * sizeof(QuarantineBitmap) : 0; root->scannable ? 2 * sizeof(QuarantineBitmap) : 0;
#if ALLOW_ENABLING_PCSCAN
// TODO(bartekn): Fix the assert when partition page isn't 16kB.
PA_DCHECK(quarantine_bitmaps_size % PartitionPageSize() == 0); PA_DCHECK(quarantine_bitmaps_size % PartitionPageSize() == 0);
#else
PA_DCHECK(quarantine_bitmaps_size == 0);
#endif
char* ret = quarantine_bitmaps + quarantine_bitmaps_size; char* ret = quarantine_bitmaps + quarantine_bitmaps_size;
root->next_partition_page = ret + total_size; root->next_partition_page = ret + total_size;
root->next_partition_page_end = root->next_super_page - PartitionPageSize(); root->next_partition_page_end = root->next_super_page - PartitionPageSize();
...@@ -346,6 +351,8 @@ ALWAYS_INLINE void* PartitionBucket<thread_safe>::AllocNewSlotSpan( ...@@ -346,6 +351,8 @@ ALWAYS_INLINE void* PartitionBucket<thread_safe>::AllocNewSlotSpan(
// //
// TODO(ajwong): Refactor Page Allocator API so the SuperPage comes in // TODO(ajwong): Refactor Page Allocator API so the SuperPage comes in
// decommited initially. // decommited initially.
// TODO(bartekn): Also decommit quarantine bitmap pages here, and commit them
// only when EnablePCScan() is called.
SetSystemPagesAccess( SetSystemPagesAccess(
super_page + PartitionPageSize() + ReservedTagBitmapSize() + super_page + PartitionPageSize() + ReservedTagBitmapSize() +
quarantine_bitmaps_size + total_size, quarantine_bitmaps_size + total_size,
......
...@@ -369,7 +369,11 @@ void PartitionRoot<thread_safe>::Init(PartitionOptions opts) { ...@@ -369,7 +369,11 @@ void PartitionRoot<thread_safe>::Init(PartitionOptions opts) {
// the beginning of the slot. // the beginning of the slot.
allow_extras = (opts.alignment != PartitionOptions::Alignment::kAlignedAlloc); allow_extras = (opts.alignment != PartitionOptions::Alignment::kAlignedAlloc);
#if ALLOW_ENABLING_PCSCAN
scannable = (opts.pcscan != PartitionOptions::PCScan::kAlwaysDisabled); scannable = (opts.pcscan != PartitionOptions::PCScan::kAlwaysDisabled);
#else
scannable = false;
#endif
// Concurrent freeing in PCScan can only safely work on thread-safe // Concurrent freeing in PCScan can only safely work on thread-safe
// partitions. // partitions.
if (thread_safe && if (thread_safe &&
......
...@@ -9,6 +9,8 @@ ...@@ -9,6 +9,8 @@
#include "base/allocator/partition_allocator/partition_alloc.h" #include "base/allocator/partition_allocator/partition_alloc.h"
#include "testing/gtest/include/gtest/gtest.h" #include "testing/gtest/include/gtest/gtest.h"
#if ALLOW_ENABLING_PCSCAN
namespace base { namespace base {
namespace internal { namespace internal {
...@@ -305,4 +307,6 @@ TEST_F(PCScanTest, DanglingInnerReference) { ...@@ -305,4 +307,6 @@ TEST_F(PCScanTest, DanglingInnerReference) {
} // namespace internal } // namespace internal
} // namespace base } // namespace base
#endif
#endif // defined(MEMORY_TOOL_REPLACES_ALLOCATOR) #endif // defined(MEMORY_TOOL_REPLACES_ALLOCATOR)
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment