Skip to content

Commit c04f058

Browse files
committed
fix: reserve shared GPU memory before CUDA alloc
Serialize check-and-add under lock_shrreg so concurrent processes cannot both pass oom_check when only the sum exceeds the limit. Keep the expensive CUDA allocation outside the lock; roll back the reservation on failure.
1 parent 9bfe42d commit c04f058

5 files changed

Lines changed: 62 additions & 36 deletions

File tree

src/allocator/allocator.c

Lines changed: 31 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,9 @@ extern CUresult cuMemoryFree(CUdeviceptr dptr);
2727
pthread_once_t allocator_allocate_flag = PTHREAD_ONCE_INIT;
2828
pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
2929

30-
/* Optional delay after the unlocked oom pre-check in add_chunk.
31-
* Used only by the race-reproduce harness to widen the TOCTOU window
32-
* between check and usage accounting across processes.
30+
/* Optional delay after reservation, before the expensive CUDA alloc.
31+
* With reserve-then-alloc this only stretches the CUDA window; shared usage
32+
* is already committed so a concurrent peer must OOM.
3333
* Env: HAMI_ALLOC_RACE_WINDOW_US=<microseconds>, unset/0 = no delay. */
3434
static void maybe_widen_alloc_race_window(void) {
3535
const char *env = getenv("HAMI_ALLOC_RACE_WINDOW_US");
@@ -52,6 +52,23 @@ size_t round_up(size_t size, size_t unit) {
5252
return size;
5353
}
5454

55+
int reserve_device_memory(CUdevice dev, size_t size) {
56+
lock_shrreg();
57+
if (oom_check(dev, size)) {
58+
unlock_shrreg();
59+
return CUDA_ERROR_OUT_OF_MEMORY;
60+
}
61+
add_gpu_device_memory_usage(getpid(), dev, size, 2);
62+
unlock_shrreg();
63+
return 0;
64+
}
65+
66+
void release_device_memory(CUdevice dev, size_t size) {
67+
lock_shrreg();
68+
rm_gpu_device_memory_usage(getpid(), dev, size, 2);
69+
unlock_shrreg();
70+
}
71+
5572
int oom_check(const int dev, size_t addon) {
5673
CUdevice d;
5774
if (dev==-1)
@@ -131,8 +148,10 @@ int add_chunk(CUdeviceptr *address, size_t size) {
131148

132149
cuCtxGetDevice(&dev);
133150

134-
/* OOM pre-check without lock */
135-
if (oom_check(dev, size))
151+
/* Reserve under the shared-region lock so concurrent processes cannot
152+
* both pass oom_check before either commits usage. CUDA alloc stays
153+
* outside the lock. */
154+
if (reserve_device_memory(dev, size) != 0)
136155
return CUDA_ERROR_OUT_OF_MEMORY;
137156

138157
maybe_widen_alloc_race_window();
@@ -145,44 +164,28 @@ int add_chunk(CUdeviceptr *address, size_t size) {
145164
}
146165
if (res != CUDA_SUCCESS) {
147166
LOG_ERROR("cuMemoryAllocate failed res=%d", res);
167+
release_device_memory(dev, size);
148168
return res;
149169
}
150170

151-
/* Tracking inside lock — pure in-memory ops, microseconds */
171+
/* Local list tracking only — usage already reserved */
152172
pthread_mutex_lock(&mutex);
153-
154-
if (oom_check(dev, size)) {
155-
/* Another process consumed memory between our pre-check and now */
156-
pthread_mutex_unlock(&mutex);
157-
CUDA_OVERRIDE_CALL(cuda_library_entry, cuMemFree_v2, *address);
158-
return CUDA_ERROR_OUT_OF_MEMORY;
159-
}
160-
161173
allocated_list_entry *e;
162174
INIT_ALLOCATED_LIST_ENTRY(e, 0, size, dev);
163175
e->entry->address = *address;
164176
LIST_ADD(device_overallocated, e);
165-
add_gpu_device_memory_usage(getpid(), dev, size, 2);
166-
167177
pthread_mutex_unlock(&mutex);
168178
return 0;
169179
}
170180

181+
/* Track a pointer in the local list. Caller must already have reserved
182+
* `size` via reserve_device_memory() (or equivalent usage accounting). */
171183
int add_chunk_only(CUdeviceptr address, size_t size, CUdevice dev) {
172184
pthread_mutex_lock(&mutex);
173-
size_t addr=0;
174-
size_t allocsize;
175-
if (oom_check(dev,size)){
176-
pthread_mutex_unlock(&mutex);
177-
return -1;
178-
}
179185
allocated_list_entry *e;
180-
INIT_ALLOCATED_LIST_ENTRY(e, addr, size, dev);
181-
LIST_ADD(device_overallocated,e);
182-
//uint64_t t_size;
183-
e->entry->address=address;
184-
allocsize = size;
185-
add_gpu_device_memory_usage(getpid(), dev, allocsize, 2);
186+
INIT_ALLOCATED_LIST_ENTRY(e, 0, size, dev);
187+
e->entry->address = address;
188+
LIST_ADD(device_overallocated, e);
186189
pthread_mutex_unlock(&mutex);
187190
return 0;
188191
}

src/allocator/allocator.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,13 @@ CUresult view_vgpu_allocator();
156156
// Checks if oom
157157
int oom_check(const int dev,size_t addon);
158158

159+
/* Cross-process check-and-reserve under lock_shrreg.
160+
* Returns 0 on success, CUDA_ERROR_OUT_OF_MEMORY if the limit would be exceeded.
161+
* On success, size is already accounted in shared usage; call
162+
* release_device_memory() if the subsequent CUDA alloc fails. */
163+
int reserve_device_memory(CUdevice dev, size_t size);
164+
void release_device_memory(CUdevice dev, size_t size);
165+
159166
// Allocate and free device memory
160167
int allocate_raw(CUdeviceptr *dptr, size_t size);
161168
int free_raw(CUdeviceptr dptr);

src/cuda/memory.c

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -145,12 +145,14 @@ CUresult cuMemAllocManaged(CUdeviceptr* dptr, size_t bytesize, unsigned int flag
145145
ENSURE_RUNNING();
146146
CUdevice dev;
147147
CHECK_DRV_API(cuCtxGetDevice(&dev));
148-
if (oom_check(dev,bytesize)){
148+
if (reserve_device_memory(dev, bytesize) != 0) {
149149
return CUDA_ERROR_OUT_OF_MEMORY;
150150
}
151151
CUresult res = CUDA_OVERRIDE_CALL(cuda_library_entry,cuMemAllocManaged, dptr, bytesize, flags);
152152
if (res == CUDA_SUCCESS) {
153153
add_chunk_only(*dptr, bytesize, dev);
154+
} else {
155+
release_device_memory(dev, bytesize);
154156
}
155157
return res;
156158
}
@@ -163,12 +165,14 @@ CUresult cuMemAllocPitch_v2(CUdeviceptr* dptr, size_t* pPitch, size_t WidthInByt
163165
ENSURE_RUNNING();
164166
CUdevice dev;
165167
CHECK_DRV_API(cuCtxGetDevice(&dev));
166-
if (oom_check(dev,bytesize)){
168+
if (reserve_device_memory(dev, bytesize) != 0) {
167169
return CUDA_ERROR_OUT_OF_MEMORY;
168170
}
169171
CUresult res = CUDA_OVERRIDE_CALL(cuda_library_entry,cuMemAllocPitch_v2, dptr, pPitch, WidthInBytes, Height, ElementSizeBytes);
170172
if (res == CUDA_SUCCESS) {
171173
add_chunk_only(*dptr, bytesize, dev);
174+
} else {
175+
release_device_memory(dev, bytesize);
172176
}
173177
return res;
174178
}
@@ -590,13 +594,17 @@ CUresult cuMemCreate ( CUmemGenericAllocationHandle* handle, size_t size, const
590594
if (do_oom_check && cuCtxGetDevice(&dev) != CUDA_SUCCESS) {
591595
dev = prop->location.id;
592596
}
593-
if (do_oom_check && oom_check(dev, size)) {
597+
if (do_oom_check && reserve_device_memory(dev, size) != 0) {
594598
return CUDA_ERROR_OUT_OF_MEMORY;
595599
}
596600
CUresult res = CUDA_OVERRIDE_CALL(cuda_library_entry,
597601
cuMemCreate, handle, size, prop, flags);
598-
if (do_oom_check && res == CUDA_SUCCESS) {
599-
add_chunk_only(*handle, size, dev);
602+
if (do_oom_check) {
603+
if (res == CUDA_SUCCESS) {
604+
add_chunk_only(*handle, size, dev);
605+
} else {
606+
release_device_memory(dev, size);
607+
}
600608
}
601609
return res;
602610
}

test/run_concurrent_oom_race.sh

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#!/usr/bin/env bash
2-
# Run the cross-process oom_check TOCTOU reproducer against libvgpu.so.
2+
# Run the cross-process oom_check race test against libvgpu.so (race-fix).
3+
# Default: --expect-fixed (limit must hold under concurrent alloc + race window).
34
set -euo pipefail
45

56
ROOT="$(cd "$(dirname "$0")/.." && pwd)"
@@ -31,8 +32,14 @@ export HAMI_RACE_ROUNDS="$ROUNDS"
3132
export HAMI_ALLOC_RACE_WINDOW_US="$WINDOW_US"
3233
export LIBCUDA_LOG_LEVEL="${LIBCUDA_LOG_LEVEL:-1}"
3334

34-
echo "Running: $BIN $*"
35+
# Default to verifying the fix; omit args only triggers --expect-fixed.
36+
ARGS=("$@")
37+
if [[ ${#ARGS[@]} -eq 0 ]]; then
38+
ARGS=(--expect-fixed)
39+
fi
40+
41+
echo "Running: $BIN ${ARGS[*]}"
3542
echo " LD_PRELOAD=$LD_PRELOAD"
3643
echo " LIMIT=$LIMIT ALLOC=$ALLOC ROUNDS=$ROUNDS WINDOW_US=$WINDOW_US"
3744
echo " CACHE=$CACHE"
38-
exec "$BIN" "$@"
45+
exec "$BIN" "${ARGS[@]}"

test/test_concurrent_oom_race.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
* ./build/test/test_concurrent_oom_race
2525
*
2626
* Or: ./test/run_concurrent_oom_race.sh
27+
* (race-fix defaults to --expect-fixed)
2728
*
2829
* Suggested sizing: limit=1024m, alloc=600m so each request fits alone
2930
* (after two process contexts) but 2*alloc exceeds the limit.

0 commit comments

Comments
 (0)