Skip to content

Commit 10f93cd

Browse files
committed
Address comments in review
1 parent 31057c0 commit 10f93cd

2 files changed

Lines changed: 20 additions & 240 deletions

File tree

‎cuda_core/cuda/core/_cpp/rt/memory.cpp‎

Lines changed: 20 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
#include <atomic>
1212
#include <cstddef>
1313
#include <cstring>
14-
#include <exception>
1514
#include <memory>
1615
#include <mutex>
1716
#include <stdexcept>
@@ -207,33 +206,6 @@ set_deallocation_stream(const DevicePtrHandle &h, const StreamHandle &h_stream)
207206
return CUDA_SUCCESS;
208207
}
209208

210-
// Build the box for a freshly acquired device pointer. Binding the deallocation
211-
// stream and allocating the box can fail: make_deallocation_stream returns
212-
// false (err set) when no context is current, and either step can throw
213-
// std::bad_alloc. On any failure run undo() to release the pointer and return
214-
// nullptr; a thrown exception is left in `failure` for the caller to rethrow
215-
// once it holds no lock (undo may not report under a lock, see
216-
// deviceptr_import_ipc). Never throws.
217-
template <typename Undo>
218-
static DevicePtrBox *
219-
make_deviceptr_box(CUdeviceptr ptr, const StreamHandle &h_stream, Undo &&undo, std::exception_ptr &failure) noexcept
220-
{
221-
DevicePtrBox *box = nullptr;
222-
try {
223-
DeallocationStream ds;
224-
if (make_deallocation_stream(h_stream, ds)) {
225-
box = new DevicePtrBox{ptr, std::move(ds)};
226-
}
227-
}
228-
catch (...) {
229-
failure = std::current_exception();
230-
}
231-
if (!box) {
232-
undo();
233-
}
234-
return box;
235-
}
236-
237209
DevicePtrHandle
238210
deviceptr_alloc_from_pool(size_t size, const MemoryPoolHandle &h_pool, const StreamHandle &h_stream)
239211
{
@@ -243,19 +215,15 @@ deviceptr_alloc_from_pool(size_t size, const MemoryPoolHandle &h_pool, const Str
243215
return {};
244216
}
245217

246-
std::exception_ptr failure;
247-
DevicePtrBox *raw =
248-
make_deviceptr_box(ptr, h_stream, [&]() noexcept { pw_cuMemFreeAsync(ptr, as_cu(h_stream)); }, failure);
249-
if (!raw) {
250-
if (failure) {
251-
std::rethrow_exception(failure);
252-
}
218+
DeallocationStream ds;
219+
if (!make_deallocation_stream(h_stream, ds)) {
220+
pw_cuMemFreeAsync(ptr, as_cu(h_stream));
253221
return {};
254222
}
255223

256224
// clang-format off
257225
auto box = std::shared_ptr<DevicePtrBox>(
258-
raw,
226+
new DevicePtrBox{ptr, std::move(ds)},
259227
[h_pool](DevicePtrBox *b) {
260228
GILReleaseGuard gil;
261229
const DeallocationStream& stream = b->deallocation;
@@ -281,19 +249,15 @@ deviceptr_alloc_async(size_t size, const StreamHandle &h_stream)
281249
return {};
282250
}
283251

284-
std::exception_ptr failure;
285-
DevicePtrBox *raw =
286-
make_deviceptr_box(ptr, h_stream, [&]() noexcept { pw_cuMemFreeAsync(ptr, as_cu(h_stream)); }, failure);
287-
if (!raw) {
288-
if (failure) {
289-
std::rethrow_exception(failure);
290-
}
252+
DeallocationStream ds;
253+
if (!make_deallocation_stream(h_stream, ds)) {
254+
pw_cuMemFreeAsync(ptr, as_cu(h_stream));
291255
return {};
292256
}
293257

294258
// clang-format off
295259
auto box = std::shared_ptr<DevicePtrBox>(
296-
raw,
260+
new DevicePtrBox{ptr, std::move(ds)},
297261
[](DevicePtrBox *b) {
298262
GILReleaseGuard gil;
299263
const DeallocationStream& stream = b->deallocation;
@@ -587,7 +551,6 @@ deviceptr_import_ipc(const MemoryPoolHandle &h_pool, const void *export_data, co
587551
GILReleaseGuard gil;
588552
CUresult discard_status = CUDA_SUCCESS;
589553
CUdeviceptr discarded = 0;
590-
std::exception_ptr failure;
591554
{
592555
std::lock_guard<std::mutex> lock(ipc_import_mutex);
593556

@@ -600,22 +563,11 @@ deviceptr_import_ipc(const MemoryPoolHandle &h_pool, const void *export_data, co
600563
return {};
601564
}
602565

603-
// No deallocation stream could be recorded (or the box could not be
604-
// allocated): discard the import with the raw call (a pw_ report would
605-
// acquire the GIL under the mutex).
606-
DevicePtrBox *raw = make_deviceptr_box(
607-
ptr,
608-
h_stream,
609-
[&]() noexcept {
610-
discard_status = p_cuMemFreeAsync(ptr, as_cu(h_stream)); // raw: under ipc_import_mutex
611-
discarded = ptr;
612-
},
613-
failure
614-
);
615-
if (raw) {
566+
DeallocationStream ds;
567+
if (make_deallocation_stream(h_stream, ds)) {
616568
// clang-format off
617569
auto box = std::shared_ptr<DevicePtrBox>(
618-
raw,
570+
new DevicePtrBox{ptr, std::move(ds)},
619571
[h_pool, key](DevicePtrBox *b) {
620572
// Release the GIL first (the GIL is the outermost lock), then hold the
621573
// mutex across unregister + free: a concurrent import that finds this
@@ -643,6 +595,11 @@ deviceptr_import_ipc(const MemoryPoolHandle &h_pool, const void *export_data, co
643595
ipc_ptr_cache.register_handle(key, h);
644596
return h;
645597
}
598+
599+
// No deallocation stream could be recorded: discard the import with
600+
// the raw call (a pw_ report would acquire the GIL under the mutex).
601+
discard_status = p_cuMemFreeAsync(ptr, as_cu(h_stream)); // raw: under ipc_import_mutex
602+
discarded = ptr;
646603
}
647604
if (discard_status != CUDA_SUCCESS) {
648605
char operation[160];
@@ -654,9 +611,6 @@ deviceptr_import_ipc(const MemoryPoolHandle &h_pool, const void *export_data, co
654611
"deallocation stream; the mapping leaked"
655612
);
656613
}
657-
if (failure) {
658-
std::rethrow_exception(failure);
659-
}
660614
return {};
661615
}
662616
else {
@@ -666,19 +620,15 @@ deviceptr_import_ipc(const MemoryPoolHandle &h_pool, const void *export_data, co
666620
return {};
667621
}
668622

669-
std::exception_ptr failure;
670-
DevicePtrBox *raw =
671-
make_deviceptr_box(ptr, h_stream, [&]() noexcept { pw_cuMemFreeAsync(ptr, as_cu(h_stream)); }, failure);
672-
if (!raw) {
673-
if (failure) {
674-
std::rethrow_exception(failure);
675-
}
623+
DeallocationStream ds;
624+
if (!make_deallocation_stream(h_stream, ds)) {
625+
pw_cuMemFreeAsync(ptr, as_cu(h_stream));
676626
return {};
677627
}
678628

679629
// clang-format off
680630
auto box = std::shared_ptr<DevicePtrBox>(
681-
raw,
631+
new DevicePtrBox{ptr, std::move(ds)},
682632
[h_pool](DevicePtrBox *b) {
683633
GILReleaseGuard gil;
684634
const DeallocationStream& stream = b->deallocation;

‎cuda_core/tests/test_allocation_failure.py‎

Lines changed: 0 additions & 170 deletions
This file was deleted.

0 commit comments

Comments
 (0)