encounter
/
dawn-cmake
mirror of https://github.com/encounter/dawn-cmake.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Handle OOM buffer allocations better 

This CL checks buffer sizes before creating map read/write handles.
It is an error to map a buffer that can't be addressed on the CPU.

It also changes client-side synchronous errors on mapAsync to be
normal map failures, and not device lost errors. These should be
recoverable.

The CL adds additional testing for really large, but not UINT64_MAX
buffers, and fixes a VVL warning when buffer allocations exceed the
size of their memory heap.

Bug: dawn:450, dawn:398, chromium:1014740
Change-Id: Ieb34c04c3d01c429b7e3b7810729d5e91ecb6270
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/22626
Commit-Queue: Austin Eng <enga@chromium.org>
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
This commit is contained in:
Austin Eng 2020-06-15 23:42:13 +00:00 committed by Commit Bot service account
parent c3c2c9b6b0
commit cf1fdf413c
12 changed files with 248 additions and 46 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
dawn.json
View File

@ -520,6 +520,11 @@
{"name": "descriptor", "type": "buffer descriptor", "annotation": "const*"}
]
},
{
"name": "create error buffer",
"returns": "buffer",
"TODO": "enga@: Make this part of a dawn_wire extension"
},
{
"name": "create buffer mapped",
"returns": "create buffer mapped result",

4
src/dawn_native/Buffer.cpp
View File

@ -210,7 +210,7 @@ namespace dawn_native {
void BufferBase::CallMapReadCallback(uint32_t serial,
WGPUBufferMapAsyncStatus status,
const void* pointer,
uint32_t dataLength) {
uint64_t dataLength) {
ASSERT(!IsError());
if (mMapReadCallback != nullptr && serial == mMapSerial) {
ASSERT(mMapWriteCallback == nullptr);
@ -231,7 +231,7 @@ namespace dawn_native {
void BufferBase::CallMapWriteCallback(uint32_t serial,
WGPUBufferMapAsyncStatus status,
void* pointer,
uint32_t dataLength) {
uint64_t dataLength) {
ASSERT(!IsError());
if (mMapWriteCallback != nullptr && serial == mMapSerial) {
ASSERT(mMapReadCallback == nullptr);

4
src/dawn_native/Buffer.h
View File

@ -71,11 +71,11 @@ namespace dawn_native {
void CallMapReadCallback(uint32_t serial,
WGPUBufferMapAsyncStatus status,
const void* pointer,
uint32_t dataLength);
uint64_t dataLength);
void CallMapWriteCallback(uint32_t serial,
WGPUBufferMapAsyncStatus status,
void* pointer,
uint32_t dataLength);
uint64_t dataLength);
void DestroyInternal();

6
src/dawn_native/Device.cpp
View File

@ -731,6 +731,12 @@ namespace dawn_native {
return result;
}
// For Dawn Wire
BufferBase* DeviceBase::CreateErrorBuffer() {
return BufferBase::MakeError(this);
}
// Other Device API methods
void DeviceBase::Tick() {

3
src/dawn_native/Device.h
View File

@ -158,6 +158,9 @@ namespace dawn_native {
TextureViewBase* CreateTextureView(TextureBase* texture,
const TextureViewDescriptor* descriptor);
// For Dawn Wire
BufferBase* CreateErrorBuffer();
QueueBase* GetDefaultQueue();
void InjectError(wgpu::ErrorType type, const char* message);

6
src/dawn_native/d3d12/ResourceAllocatorManagerD3D12.cpp
View File

@ -185,8 +185,12 @@ namespace dawn_native { namespace d3d12 {
ResourceHeapAllocation directAllocation;
DAWN_TRY_ASSIGN(directAllocation,
CreateCommittedResource(heapType, resourceDescriptor, initialUsage));
if (directAllocation.GetInfo().mMethod != AllocationMethod::kInvalid) {
return std::move(directAllocation);
}
return std::move(directAllocation);
// If direct allocation fails, the system is probably out of memory.
return DAWN_OUT_OF_MEMORY_ERROR("Allocation failed");
}
void ResourceAllocatorManager::Tick(Serial completedSerial) {

2
src/dawn_native/vulkan/BufferVk.cpp
View File

@ -148,7 +148,7 @@ namespace dawn_native { namespace vulkan {
createInfo.pQueueFamilyIndices = 0;
Device* device = ToBackend(GetDevice());
DAWN_TRY(CheckVkSuccess(
DAWN_TRY(CheckVkOOMThenSuccess(
device->fn.CreateBuffer(device->GetVkDevice(), &createInfo, nullptr, &*mHandle),
"vkCreateBuffer"));

24
src/dawn_native/vulkan/ResourceMemoryAllocatorVk.cpp
View File

@ -14,6 +14,7 @@
#include "dawn_native/vulkan/ResourceMemoryAllocatorVk.h"
#include "common/Math.h"
#include "dawn_native/BuddyMemoryAllocator.h"
#include "dawn_native/ResourceHeapAllocator.h"
#include "dawn_native/vulkan/DeviceVk.h"
@ -28,8 +29,7 @@ namespace dawn_native { namespace vulkan {
// TODO(cwallez@chromium.org): This is a hardcoded heurstic to choose when to
// suballocate but it should ideally depend on the size of the memory heaps and other
// factors.
constexpr uint64_t kMaxBuddySystemSize = 32ull * 1024ull * 1024ull * 1024ull; // 32GB
constexpr uint64_t kMaxSizeForSubAllocation = 4ull * 1024ull * 1024ull; // 4MB
constexpr uint64_t kMaxSizeForSubAllocation = 4ull * 1024ull * 1024ull; // 4MiB
// Have each bucket of the buddy system allocate at least some resource of the maximum
// size
@ -42,10 +42,18 @@ namespace dawn_native { namespace vulkan {
class ResourceMemoryAllocator::SingleTypeAllocator : public ResourceHeapAllocator {
public:
SingleTypeAllocator(Device* device, size_t memoryTypeIndex)
SingleTypeAllocator(Device* device, size_t memoryTypeIndex, VkDeviceSize memoryHeapSize)
: mDevice(device),
mMemoryTypeIndex(memoryTypeIndex),
mBuddySystem(kMaxBuddySystemSize, kBuddyHeapsSize, this) {
mMemoryHeapSize(memoryHeapSize),
mBuddySystem(
// Round down to a power of 2 that's <= mMemoryHeapSize. This will always
// be a multiple of kBuddyHeapsSize because kBuddyHeapsSize is a power of 2.
uint64_t(1) << Log2(mMemoryHeapSize),
// Take the min in the very unlikely case the memory heap is tiny.
std::min(uint64_t(1) << Log2(mMemoryHeapSize), kBuddyHeapsSize),
this) {
ASSERT(IsPowerOfTwo(kBuddyHeapsSize));
}
~SingleTypeAllocator() override = default;
@ -62,6 +70,10 @@ namespace dawn_native { namespace vulkan {
ResultOrError<std::unique_ptr<ResourceHeapBase>> AllocateResourceHeap(
uint64_t size) override {
if (size > mMemoryHeapSize) {
return DAWN_OUT_OF_MEMORY_ERROR("Allocation size too large");
}
VkMemoryAllocateInfo allocateInfo;
allocateInfo.sType = VK_STRUCTURE_TYPE_MEMORY_ALLOCATE_INFO;
allocateInfo.pNext = nullptr;
@ -87,6 +99,7 @@ namespace dawn_native { namespace vulkan {
private:
Device* mDevice;
size_t mMemoryTypeIndex;
VkDeviceSize mMemoryHeapSize;
BuddyMemoryAllocator mBuddySystem;
};
@ -97,7 +110,8 @@ namespace dawn_native { namespace vulkan {
mAllocatorsPerType.reserve(info.memoryTypes.size());
for (size_t i = 0; i < info.memoryTypes.size(); i++) {
mAllocatorsPerType.emplace_back(std::make_unique<SingleTypeAllocator>(mDevice, i));
mAllocatorsPerType.emplace_back(std::make_unique<SingleTypeAllocator>(
mDevice, i, info.memoryHeaps[info.memoryTypes[i].heapIndex].size));
}
}

67
src/dawn_wire/client/ApiProcs.cpp
View File

@ -54,12 +54,25 @@ namespace dawn_wire { namespace client {
uint32_t serial = buffer->requestSerial++;
ASSERT(buffer->requests.find(serial) == buffer->requests.end());
if (buffer->size > std::numeric_limits<size_t>::max()) {
// On buffer creation, we check that mappable buffers do not exceed this size.
// So this buffer must not have mappable usage. Inject a validation error.
ClientDeviceInjectError(reinterpret_cast<WGPUDevice>(buffer->device),
WGPUErrorType_Validation,
"Buffer needs the correct map usage bit");
callback(WGPUBufferMapAsyncStatus_Error, nullptr, 0, userdata);
return;
}
// Create a ReadHandle for the map request. This is the client's intent to read GPU
// memory.
MemoryTransferService::ReadHandle* readHandle =
buffer->device->GetClient()->GetMemoryTransferService()->CreateReadHandle(buffer->size);
buffer->device->GetClient()->GetMemoryTransferService()->CreateReadHandle(
static_cast<size_t>(buffer->size));
if (readHandle == nullptr) {
callback(WGPUBufferMapAsyncStatus_DeviceLost, nullptr, 0, userdata);
ClientDeviceInjectError(reinterpret_cast<WGPUDevice>(buffer->device),
WGPUErrorType_OutOfMemory, "Failed to create buffer mapping");
callback(WGPUBufferMapAsyncStatus_Error, nullptr, 0, userdata);
return;
}
@ -84,13 +97,25 @@ namespace dawn_wire { namespace client {
uint32_t serial = buffer->requestSerial++;
ASSERT(buffer->requests.find(serial) == buffer->requests.end());
if (buffer->size > std::numeric_limits<size_t>::max()) {
// On buffer creation, we check that mappable buffers do not exceed this size.
// So this buffer must not have mappable usage. Inject a validation error.
ClientDeviceInjectError(reinterpret_cast<WGPUDevice>(buffer->device),
WGPUErrorType_Validation,
"Buffer needs the correct map usage bit");
callback(WGPUBufferMapAsyncStatus_Error, nullptr, 0, userdata);
return;
}
// Create a WriteHandle for the map request. This is the client's intent to write GPU
// memory.
MemoryTransferService::WriteHandle* writeHandle =
buffer->device->GetClient()->GetMemoryTransferService()->CreateWriteHandle(
buffer->size);
static_cast<size_t>(buffer->size));
if (writeHandle == nullptr) {
callback(WGPUBufferMapAsyncStatus_DeviceLost, nullptr, 0, userdata);
ClientDeviceInjectError(reinterpret_cast<WGPUDevice>(buffer->device),
WGPUErrorType_OutOfMemory, "Failed to create buffer mapping");
callback(WGPUBufferMapAsyncStatus_Error, nullptr, 0, userdata);
return;
}
@ -112,6 +137,13 @@ namespace dawn_wire { namespace client {
Device* device = reinterpret_cast<Device*>(cDevice);
Client* wireClient = device->GetClient();
if ((descriptor->usage & (WGPUBufferUsage_MapRead | WGPUBufferUsage_MapWrite)) != 0 &&
descriptor->size > std::numeric_limits<size_t>::max()) {
ClientDeviceInjectError(cDevice, WGPUErrorType_OutOfMemory,
"Buffer is too large for map usage");
return ClientDeviceCreateErrorBuffer(cDevice);
}
auto* bufferObjectAndSerial = wireClient->BufferAllocator().New(device);
Buffer* buffer = bufferObjectAndSerial->object.get();
// Store the size of the buffer so that mapping operations can allocate a
@ -136,15 +168,18 @@ namespace dawn_wire { namespace client {
Device* device = reinterpret_cast<Device*>(cDevice);
Client* wireClient = device->GetClient();
auto* bufferObjectAndSerial = wireClient->BufferAllocator().New(device);
Buffer* buffer = bufferObjectAndSerial->object.get();
buffer->size = descriptor->size;
WGPUCreateBufferMappedResult result;
result.buffer = reinterpret_cast<WGPUBuffer>(buffer);
result.data = nullptr;
result.dataLength = 0;
// This buffer is too large to be mapped and to make a WriteHandle for.
if (descriptor->size > std::numeric_limits<size_t>::max()) {
ClientDeviceInjectError(cDevice, WGPUErrorType_OutOfMemory,
"Buffer is too large for mapping");
result.buffer = ClientDeviceCreateErrorBuffer(cDevice);
return result;
}
// Create a WriteHandle for the map request. This is the client's intent to write GPU
// memory.
std::unique_ptr<MemoryTransferService::WriteHandle> writeHandle =
@ -152,7 +187,9 @@ namespace dawn_wire { namespace client {
wireClient->GetMemoryTransferService()->CreateWriteHandle(descriptor->size));
if (writeHandle == nullptr) {
// TODO(enga): Support context lost generated by the client.
ClientDeviceInjectError(cDevice, WGPUErrorType_OutOfMemory,
"Buffer mapping allocation failed");
result.buffer = ClientDeviceCreateErrorBuffer(cDevice);
return result;
}
@ -161,15 +198,21 @@ namespace dawn_wire { namespace client {
// Open the WriteHandle. This returns a pointer and size of mapped memory.
// |result.data| may be null on error.
std::tie(result.data, result.dataLength) = writeHandle->Open();
if (result.data == nullptr) {
// TODO(enga): Support context lost generated by the client.
ClientDeviceInjectError(cDevice, WGPUErrorType_OutOfMemory,
"Buffer mapping allocation failed");
result.buffer = ClientDeviceCreateErrorBuffer(cDevice);
return result;
}
auto* bufferObjectAndSerial = wireClient->BufferAllocator().New(device);
Buffer* buffer = bufferObjectAndSerial->object.get();
buffer->size = descriptor->size;
// Successfully created staging memory. The buffer now owns the WriteHandle.
buffer->writeHandle = std::move(writeHandle);
result.buffer = reinterpret_cast<WGPUBuffer>(buffer);
// Get the serialization size of the WriteHandle.
size_t handleCreateInfoLength = buffer->writeHandle->SerializeCreateSize();

10
src/dawn_wire/client/ClientInlineMemoryTransferService.cpp
View File

@ -43,7 +43,10 @@ namespace dawn_wire { namespace client {
return false;
}
mStagingData = std::unique_ptr<uint8_t[]>(new uint8_t[mSize]);
mStagingData = std::unique_ptr<uint8_t[]>(new (std::nothrow) uint8_t[mSize]);
if (!mStagingData) {
return false;
}
memcpy(mStagingData.get(), deserializePointer, mSize);
ASSERT(data != nullptr);
@ -74,7 +77,10 @@ namespace dawn_wire { namespace client {
}
std::pair<void*, size_t> Open() override {
mStagingData = std::unique_ptr<uint8_t[]>(new uint8_t[mSize]);
mStagingData = std::unique_ptr<uint8_t[]>(new (std::nothrow) uint8_t[mSize]);
if (!mStagingData) {
return std::make_pair(nullptr, 0);
}
memset(mStagingData.get(), 0, mSize);
return std::make_pair(mStagingData.get(), mSize);
}

149
src/tests/end2end/BufferTests.cpp
View File

@ -463,17 +463,6 @@ TEST_P(CreateBufferMappedTests, CreateThenMapBeforeUnmapFailure) {
EXPECT_BUFFER_U32_EQ(myData, result.buffer, 0);
}
// Test that creating a very large buffers fails gracefully.
TEST_P(CreateBufferMappedTests, LargeBufferFails) {
// TODO(http://crbug.com/dawn/27): Missing support.
DAWN_SKIP_TEST_IF(IsOpenGL());
wgpu::BufferDescriptor descriptor;
descriptor.size = std::numeric_limits<uint64_t>::max();
descriptor.usage = wgpu::BufferUsage::MapRead | wgpu::BufferUsage::CopyDst;
ASSERT_DEVICE_ERROR(device.CreateBuffer(&descriptor));
}
// Test that creating a zero-sized buffer mapped is allowed.
TEST_P(CreateBufferMappedTests, ZeroSized) {
wgpu::BufferDescriptor descriptor;
@ -534,14 +523,148 @@ TEST_P(BufferTests, ZeroSizedBuffer) {
}
// Test that creating a very large buffers fails gracefully.
TEST_P(BufferTests, LargeBufferFails) {
TEST_P(BufferTests, CreateBufferOOM) {
// TODO(http://crbug.com/dawn/27): Missing support.
DAWN_SKIP_TEST_IF(IsOpenGL());
wgpu::BufferDescriptor descriptor;
descriptor.usage = wgpu::BufferUsage::CopyDst;
descriptor.size = std::numeric_limits<uint64_t>::max();
descriptor.usage = wgpu::BufferUsage::MapRead | wgpu::BufferUsage::CopyDst;
ASSERT_DEVICE_ERROR(device.CreateBuffer(&descriptor));
// UINT64_MAX may be special cased. Test a smaller, but really large buffer also fails
// This hangs on the Metal AMD driver
if (!(IsMetal() && IsAMD())) {
descriptor.size = 1ull << 50;
ASSERT_DEVICE_ERROR(device.CreateBuffer(&descriptor));
}
}
// Test that a very large CreateBufferMapped fails gracefully.
TEST_P(BufferTests, CreateBufferMappedOOM) {
// TODO(http://crbug.com/dawn/27): Missing support.
DAWN_SKIP_TEST_IF(IsOpenGL());
// Test non-mappable buffer
{
wgpu::BufferDescriptor descriptor;
descriptor.size = 4;
descriptor.usage = wgpu::BufferUsage::CopyDst;
// Control: test a small buffer works.
device.CreateBufferMapped(&descriptor);
// Test an enormous buffer fails
descriptor.size = std::numeric_limits<uint64_t>::max();
ASSERT_DEVICE_ERROR(device.CreateBufferMapped(&descriptor));
// UINT64_MAX may be special cased. Test a smaller, but really large buffer also fails
// This hangs on the Metal AMD driver
if (!(IsMetal() && IsAMD())) {
descriptor.size = 1ull << 50;
ASSERT_DEVICE_ERROR(device.CreateBufferMapped(&descriptor));
}
}
// Test mappable buffer
{
wgpu::BufferDescriptor descriptor;
descriptor.size = 4;
descriptor.usage = wgpu::BufferUsage::CopySrc | wgpu::BufferUsage::MapWrite;
// Control: test a small buffer works.
device.CreateBufferMapped(&descriptor);
// Test an enormous buffer fails
descriptor.size = std::numeric_limits<uint64_t>::max();
ASSERT_DEVICE_ERROR(device.CreateBufferMapped(&descriptor));
if (!(IsMetal() && IsAMD())) {
// UINT64_MAX may be special cased. Test a smaller, but really large buffer also fails
descriptor.size = 1ull << 50;
ASSERT_DEVICE_ERROR(device.CreateBufferMapped(&descriptor));
}
}
}
// Test that mapping an OOM buffer for reading fails gracefully
TEST_P(BufferTests, CreateBufferOOMMapReadAsync) {
// TODO(http://crbug.com/dawn/27): Missing support.
DAWN_SKIP_TEST_IF(IsOpenGL());
auto RunTest = [this](const wgpu::BufferDescriptor& descriptor) {
wgpu::Buffer buffer;
ASSERT_DEVICE_ERROR(buffer = device.CreateBuffer(&descriptor));
bool done = false;
ASSERT_DEVICE_ERROR(buffer.MapReadAsync(
[](WGPUBufferMapAsyncStatus status, const void* ptr, uint64_t dataLength,
void* userdata) {
EXPECT_EQ(status, WGPUBufferMapAsyncStatus_Error);
EXPECT_EQ(ptr, nullptr);
EXPECT_EQ(dataLength, 0u);
*static_cast<bool*>(userdata) = true;
},
&done));
while (!done) {
WaitABit();
}
};
// Test an enormous buffer
wgpu::BufferDescriptor descriptor;
descriptor.usage = wgpu::BufferUsage::CopyDst | wgpu::BufferUsage::MapRead;
descriptor.size = std::numeric_limits<uint64_t>::max();
RunTest(descriptor);
// UINT64_MAX may be special cased. Test a smaller, but really large buffer also fails
// This hangs on the Metal AMD driver
if (!(IsMetal() && IsAMD())) {
descriptor.size = 1ull << 50;
RunTest(descriptor);
}
}
// Test that mapping an OOM buffer for reading fails gracefully
TEST_P(BufferTests, CreateBufferOOMMapWriteAsync) {
// TODO(http://crbug.com/dawn/27): Missing support.
DAWN_SKIP_TEST_IF(IsOpenGL());
auto RunTest = [this](const wgpu::BufferDescriptor& descriptor) {
wgpu::Buffer buffer;
ASSERT_DEVICE_ERROR(buffer = device.CreateBuffer(&descriptor));
bool done = false;
ASSERT_DEVICE_ERROR(buffer.MapWriteAsync(
[](WGPUBufferMapAsyncStatus status, void* ptr, uint64_t dataLength, void* userdata) {
EXPECT_EQ(status, WGPUBufferMapAsyncStatus_Error);
EXPECT_EQ(ptr, nullptr);
EXPECT_EQ(dataLength, 0u);
*static_cast<bool*>(userdata) = true;
},
&done));
while (!done) {
WaitABit();
}
};
wgpu::BufferDescriptor descriptor;
descriptor.usage = wgpu::BufferUsage::CopySrc | wgpu::BufferUsage::MapWrite;
// Test an enormous buffer
descriptor.size = std::numeric_limits<uint64_t>::max();
RunTest(descriptor);
// UINT64_MAX may be special cased. Test a smaller, but really large buffer also fails
// This hangs on the Metal AMD driver
if (!(IsMetal() && IsAMD())) {
descriptor.size = 1ull << 50;
RunTest(descriptor);
}
}
DAWN_INSTANTIATE_TEST(BufferTests,

14
src/tests/unittests/wire/WireMemoryTransferServiceTests.cpp
View File

@ -474,10 +474,9 @@ TEST_F(WireMemoryTransferServiceTests, BufferMapReadHandleCreationFailure) {
// Mock a ReadHandle creation failure
MockReadHandleCreationFailure();
// Failed creation of a ReadHandle is a fatal failure and the client synchronously receives a
// DEVICE_LOST callback.
EXPECT_CALL(*mockBufferMapReadCallback,
Call(WGPUBufferMapAsyncStatus_DeviceLost, nullptr, 0, _))
// Failed creation of a ReadHandle is a mapping failure and the client synchronously receives
// an error callback.
EXPECT_CALL(*mockBufferMapReadCallback, Call(WGPUBufferMapAsyncStatus_Error, nullptr, 0, _))
.Times(1);
wgpuBufferMapReadAsync(buffer, ToMockBufferMapReadCallback, nullptr);
@ -716,10 +715,9 @@ TEST_F(WireMemoryTransferServiceTests, BufferMapWriteHandleCreationFailure) {
// Mock a WriteHandle creation failure
MockWriteHandleCreationFailure();
// Failed creation of a WriteHandle is a fatal failure and the client synchronously receives a
// DEVICE_LOST callback.
EXPECT_CALL(*mockBufferMapWriteCallback,
Call(WGPUBufferMapAsyncStatus_DeviceLost, nullptr, 0, _))
// Failed creation of a WriteHandle is a mapping failure and the client synchronously receives
// an error callback.
EXPECT_CALL(*mockBufferMapWriteCallback, Call(WGPUBufferMapAsyncStatus_Error, nullptr, 0, _))
.Times(1);
wgpuBufferMapWriteAsync(buffer, ToMockBufferMapWriteCallback, nullptr);