Skip to content

Commit 88049ba

Browse files
lusorisclaude
lusoris
and
claude
committed
cuda: return an error from vmaf_cuda_buffer_alloc() when the device is out of memory
vmaf_cuda_buffer_alloc() wraps cuMemAlloc() in CHECK_CUDA, which prints the error and calls assert(0). When another process holds the VRAM, cuMemAlloc() returns CUDA_ERROR_OUT_OF_MEMORY and a second run is killed with SIGABRT instead of failing: "vmaf_cuda_buffer_alloc: Assertion `0' failed". The function already returns negative errno values for its other failures. Call cuMemAlloc() directly, pop the context, free the half-built buffer and return -ENOMEM for CUDA_ERROR_OUT_OF_MEMORY and -EIO for any other error. *p_buf is now assigned only on success; before, it was set before the allocation. The ffnvcodec headers do not name CUDA_ERROR_OUT_OF_MEMORY, so the value 2 gets a local macro. All callers (the init functions of the motion, vif and adm extractors) already treat a non-zero return by jumping to their free_ref label, which releases only buffers that are not NULL; since *p_buf stays NULL on failure that holds. integer_motion_cuda.c leaked its write_score_parameters allocation on that path and now frees it. Reproduction: a second process holds all but 700, 1000 or 3000 MiB of the RTX 4090 and vmaf runs on two 4096x2160 8-bit frames with --gpumask 0. Unpatched master aborts with exit status 134 at 700 and 1000 MiB; with this change the same runs print "problem reading pictures" and exit with status 254; with 3000 MiB free both succeed. test_cuda_buffer_alloc requests SIZE_MAX / 2 bytes and checks for -ENOMEM, a NULL buffer and a working state afterwards; on unpatched master it aborts. This only converts this call site: CHECK_CUDA has 204 call sites under libvmaf/src on master (203 after this change) and the other sites still abort. Co-Authored-By: Claude Opus 5.5 <[email protected]>
1 parent 8e7a1ac commit 88049ba

4 files changed

Lines changed: 101 additions & 3 deletions

File tree

‎libvmaf/src/cuda/common.c‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,9 @@ int vmaf_cuda_release(VmafCudaState *cu_state)
153153
return CUDA_SUCCESS;
154154
}
155155

156+
/* CUresult 2 (CUDA_ERROR_OUT_OF_MEMORY); the ffnvcodec headers do not name it. */
157+
#define VMAF_CUDA_ERROR_OUT_OF_MEMORY 2
158+
156159
int vmaf_cuda_buffer_alloc(VmafCudaState *cu_state, VmafCudaBuffer **p_buf,
157160
size_t size)
158161
{
@@ -162,13 +165,17 @@ int vmaf_cuda_buffer_alloc(VmafCudaState *cu_state, VmafCudaBuffer **p_buf,
162165
VmafCudaBuffer *buf = (VmafCudaBuffer *)calloc(1, sizeof(*buf));
163166
if (!buf) return -ENOMEM;
164167

165-
*p_buf = buf;
166168
buf->size = size;
167169

168170
CHECK_CUDA(cu_state->f, cuCtxPushCurrent(cu_state->ctx));
169-
CHECK_CUDA(cu_state->f, cuMemAlloc(&buf->data, buf->size));
170-
171+
const CUresult cu_err = cu_state->f->cuMemAlloc(&buf->data, buf->size);
171172
CHECK_CUDA(cu_state->f, cuCtxPopCurrent(NULL));
173+
if (cu_err != CUDA_SUCCESS) {
174+
free(buf);
175+
return cu_err == VMAF_CUDA_ERROR_OUT_OF_MEMORY ? -ENOMEM : -EIO;
176+
}
177+
178+
*p_buf = buf;
172179
return CUDA_SUCCESS;
173180
}
174181

‎libvmaf/src/feature/cuda/integer_motion_cuda.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -186,6 +186,7 @@ static int init_fex_cuda(VmafFeatureExtractor *fex, enum VmafPixelFormat pix_fmt
186186

187187

188188
free_ref:
189+
free(s->write_score_parameters);
189190
if (s->blur[0]) {
190191
ret |= vmaf_cuda_buffer_free(fex->cu_state, s->blur[0]);
191192
free(s->blur[0]);

‎libvmaf/test/meson.build‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,15 @@ if float_enabled
180180
endif
181181

182182
if get_option('enable_cuda')
183+
test_cuda_buffer_alloc = executable('test_cuda_buffer_alloc',
184+
['test.c', 'test_cuda_buffer_alloc.c'],
185+
include_directories : [libvmaf_inc, test_inc, include_directories('../src/')],
186+
link_with : get_option('default_library') == 'both' ? libvmaf.get_static_lib() : libvmaf,
187+
dependencies: cuda_dependency,
188+
c_args: ['-DHAVE_CUDA=1']
189+
)
190+
test('test_cuda_buffer_alloc', test_cuda_buffer_alloc)
191+
183192
test_ring_buffer = executable('test_ring_buffer',
184193
['test.c', 'test_ring_buffer.c', '../src/cuda/ring_buffer.c', '../src/cuda/picture_cuda.c'],
185194
include_directories : [libvmaf_inc, test_inc, include_directories('../src/')],

‎libvmaf/test/test_cuda_buffer_alloc.c‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
/**
2+
*
3+
* Copyright 2016-2023 Netflix, Inc.
4+
*
5+
* Licensed under the BSD+Patent License (the "License");
6+
* you may not use this file except in compliance with the License.
7+
* You may obtain a copy of the License at
8+
*
9+
* https://opensource.org/licenses/BSDplusPatent
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*
17+
*/
18+
19+
#include <errno.h>
20+
#include <stddef.h>
21+
#include <stdint.h>
22+
#include <stdlib.h>
23+
24+
#include "test.h"
25+
26+
#include "libvmaf/libvmaf_cuda.h"
27+
28+
#include "cuda/common.h"
29+
30+
static char *test_cuda_buffer_alloc_arguments()
31+
{
32+
VmafCudaState *cu_state;
33+
VmafCudaConfiguration cuda_cfg = { 0 };
34+
int err = vmaf_cuda_state_init(&cu_state, cuda_cfg);
35+
mu_assert("problem during vmaf_cuda_state_init", !err);
36+
37+
VmafCudaBuffer *buf = NULL;
38+
err = vmaf_cuda_buffer_alloc(cu_state, NULL, 1024);
39+
mu_assert("a NULL buffer pointer must give -EINVAL", err == -EINVAL);
40+
err = vmaf_cuda_buffer_alloc(NULL, &buf, 1024);
41+
mu_assert("a NULL state must give -EINVAL", err == -EINVAL);
42+
mu_assert("buffer must stay NULL after an argument error", !buf);
43+
44+
err = vmaf_cuda_release(cu_state);
45+
mu_assert("problem during vmaf_cuda_release", !err);
46+
return NULL;
47+
}
48+
49+
static char *test_cuda_buffer_alloc_out_of_memory()
50+
{
51+
VmafCudaState *cu_state;
52+
VmafCudaConfiguration cuda_cfg = { 0 };
53+
int err = vmaf_cuda_state_init(&cu_state, cuda_cfg);
54+
mu_assert("problem during vmaf_cuda_state_init", !err);
55+
56+
/* a request no device can satisfy: it must fail with an error code, not abort */
57+
VmafCudaBuffer *buf = NULL;
58+
err = vmaf_cuda_buffer_alloc(cu_state, &buf, SIZE_MAX / 2);
59+
mu_assert("an impossible allocation must fail with -ENOMEM", err == -ENOMEM);
60+
mu_assert("buffer must stay NULL after a failed allocation", !buf);
61+
62+
/* the state is still usable afterwards: the context was popped */
63+
err = vmaf_cuda_buffer_alloc(cu_state, &buf, 1 << 20);
64+
mu_assert("allocation after a failed one must succeed", !err);
65+
mu_assert("buffer was not returned", buf);
66+
mu_assert("buffer has the requested size", buf->size == (1 << 20));
67+
err = vmaf_cuda_buffer_free(cu_state, buf);
68+
mu_assert("problem during vmaf_cuda_buffer_free", !err);
69+
free(buf);
70+
71+
err = vmaf_cuda_release(cu_state);
72+
mu_assert("problem during vmaf_cuda_release", !err);
73+
return NULL;
74+
}
75+
76+
char *run_tests()
77+
{
78+
mu_run_test(test_cuda_buffer_alloc_arguments);
79+
mu_run_test(test_cuda_buffer_alloc_out_of_memory);
80+
return NULL;
81+
}

0 commit comments

Comments
 (0)