From 0f1046c52880ce7de873c017737a648c03a26cbe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20Ingve=20Schj=C3=B8lberg?= Date: Fri, 24 Jul 2026 14:18:05 +0200 Subject: [PATCH] [vulkan] Set BlockRegion initial usage_count to 1 on allocation BlockRegion has a usage_count that is incremented by RegionAllocator::retain and decremented by RegionAllocator::release. These methods are used in the runtime when a buffer is cropped, with the intent to ensure that the underlying memory region does not get deallocated until both the cropped buffer and original buffer are released. The order of operations as seen by the RegionAllocator when allocating a buffer, then allocating a crop of that first buffer, then releasing both, is this: 1. region = reserve(...) 2. retain(region) 3. release(region) 4. release(region) Note that after 3, but before 4, the region should still be allocated and the region allocator should not allocate any new region that overlaps it. Testing this pattern found that the region allocator considers the region to be available after step 3. In practice, this causes Halide to give the same underlying memory to two live buffers simultaneously. The cause of this issue is that RegionAllocator::alloc_block_region does not update the usage_count of the allocated block region from its default value of 0. This commit fixes the issue by setting the usage count of the newly allocated block region to 1 in the same place where its status is updated, indicating that the newly allocated region will have one usage, which must be released before the memory can be reallocated to a new region. This commit includes a regression test of retain/release with the exact same order of operations as was mentioned above. Co-authored-by: Claude Opus 4.8 --- src/runtime/internal/region_allocator.h | 1 + test/runtime/block_allocator.cpp | 48 +++++++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/src/runtime/internal/region_allocator.h b/src/runtime/internal/region_allocator.h index 8c04116a3a65..7bff360acff5 100644 --- a/src/runtime/internal/region_allocator.h +++ b/src/runtime/internal/region_allocator.h @@ -635,6 +635,7 @@ int RegionAllocator::alloc_block_region(void *user_context, BlockRegion *block_r } if (error_code == 0) { block_region->status = block_region->memory.dedicated ? AllocationStatus::Dedicated : AllocationStatus::InUse; + block_region->usage_count = 1; block->reserved += block_region->memory.allocation.size; } return error_code; diff --git a/test/runtime/block_allocator.cpp b/test/runtime/block_allocator.cpp index 7bc3d5ebd92f..0b3f009a4217 100644 --- a/test/runtime/block_allocator.cpp +++ b/test/runtime/block_allocator.cpp @@ -420,6 +420,54 @@ int main(int argc, char **argv) { HALIDE_CHECK(user_context, get_allocated_system_memory() == 0); } + // test retain/release + { + // Use custom conform allocation request callbacks + MemoryRegionAllocatorFns region_allocator = {allocate_region, deallocate_region, conform_region}; + + // Manually create a block resource and allocate memory + size_t block_size = 1024; + BlockResource block_resource = {}; + MemoryBlock *memory_block = &(block_resource.memory); + memory_block->size = block_size; + allocate_block(user_context, memory_block); + + // Create a region allocator to manage the block resource + RegionAllocator::MemoryAllocators allocators = {system_allocator, region_allocator}; + RegionAllocator *instance = RegionAllocator::create(user_context, &block_resource, allocators); + + MemoryRequest request = {0}; + request.size = 512; + request.alignment = sizeof(int); + request.properties.visibility = MemoryVisibility::DefaultVisibility; + request.properties.caching = MemoryCaching::DefaultCaching; + request.properties.usage = MemoryUsage::DefaultUsage; + + MemoryRegion *r1 = instance->reserve(user_context, request); + HALIDE_CHECK(user_context, r1 != nullptr); + HALIDE_CHECK(user_context, r1->allocation.offset == 0); + + // Do one retain/release cycle. r1 should still be allocated after the release, since it was retained. + instance->retain(user_context, r1); + instance->release(user_context, r1); + + MemoryRegion *r2 = instance->reserve(user_context, request); + HALIDE_CHECK(user_context, r2 != nullptr); + // r2 should be allocated after r1, since r1 should still be allocated. + HALIDE_CHECK(user_context, r2->allocation.offset == 512); + + instance->release(user_context, r2); + instance->release(user_context, r1); + + instance->destroy(user_context); + deallocate_block(user_context, memory_block); + HALIDE_CHECK(user_context, allocated_block_memory == 0); + HALIDE_CHECK(user_context, allocated_region_memory == 0); + + RegionAllocator::destroy(user_context, instance); + HALIDE_CHECK(user_context, get_allocated_system_memory() == 0); + } + // test conform request { uint32_t mbs = 1024; // min block size