Skip to content

Commit 8b78870

Browse files
JasonLC506meta-codesync[bot]
authored andcommitted
Fix uvm_to_cpu/uvm_to_device/uvm_get_guard_index for host-mapped tensors (#5816)
Summary: Pull Request resolved: #5816 X-link: https://github.com/facebookresearch/FBGEMM/pull/2742 Three functions in fbgemm_gpu's memory_utils.cu unconditionally cast the storage context to `CUDAManagedIndirectContext`, which returns nullptr for tensors created via `new_host_mapped_tensor` / `new_unified_tensor(is_host_mapped=True)`. The nullptr triggers `TORCH_CHECK(tcontext != nullptr)` with the error message "Expected tcontext != nullptr", surfacing during DCP/Pyper checkpoint loading of TBE shards whenever the destination tensor is host-mapped. Inference TBE already enables `uvm_host_mapped=True` (see `torchrec/distributed/quant_embedding_kernel.py:350,594`, originally landed in D42272528 for a NUMA OOM fix), so this crash is reachable in production today on any path that calls `fbgemm.uvm_to_cpu` / `uvm_to_device` / `cuda_mem_advise` on a host-mapped tensor (e.g. `aiplatform/modelstore/checkpointing/pyper/tensor_utils.py:74`). The fix mirrors the existing managed-tensor pattern: a new `CUDAHostMappedIndirectContext` struct holds a refcount on the original `Storage`, and each of the three functions gains a `deleter == &CUDAHostMappedContext::release` branch that constructs the appropriate CPU/device view backed by the host-mapped pointer. The original `CUDAManagedIndirectContext` code path is byte-for-byte unchanged (except a missing semicolon on one `TORCH_CHECK` that the compiler had been absorbing). All existing managed-tensor callers continue to work. Motivating use case: OneFlow preranker publish jobs intermittently timeout (~6% failure rate) due to `torch.zeros(out=72GB_UVM_tensor)` triggering THP/UVM compact_stalls on fragmented hosts. A/B results showing the host-mapped fix eliminates the variance (max 85s vs 1330s baseline, 0 torch.zeros compact_stall across 20 nodes): P2349246996. Full root cause / fix design: P2343573171. Reviewed By: q10 Differential Revision: D106211462
1 parent db3b9d3 commit 8b78870

2 files changed

Lines changed: 355 additions & 8 deletions

File tree

fbgemm_gpu/src/memory_utils/memory_utils.cu

Lines changed: 124 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,74 @@ struct CUDAManagedIndirectContext {
7979
}
8080
};
8181

82+
// Same as CUDAManagedIndirectContext but for host-mapped (cudaHostRegister)
83+
// tensors. Prevents the underlying CUDAHostMappedContext from being freed
84+
// while a CPU-view or device-view tensor is still alive.
85+
struct CUDAHostMappedIndirectContext {
86+
Storage storage_;
87+
88+
CUDAHostMappedIndirectContext(Storage storage)
89+
: storage_(std::move(storage)) {};
90+
91+
static void release(void* ptr) {
92+
delete static_cast<CUDAHostMappedIndirectContext*>(ptr);
93+
}
94+
};
95+
96+
// Build a new tensor view over a host-mapped storage with the requested
97+
// device tag, keeping `host_mapped_storage` (the Storage owning the
98+
// CUDAHostMappedContext) alive via CUDAHostMappedIndirectContext. Used by both
99+
// uvm_to_cpu (target_device=CPU, ptr=host pointer) and uvm_to_device_d
100+
// (target_device=CUDA, ptr=device pointer from cudaHostGetDevicePointer).
101+
// Passing the unwrapped direct-host-mapped Storage on chained calls keeps the
102+
// indirect chain flat instead of nesting one indirect inside another.
103+
Tensor make_host_mapped_view(
104+
const Tensor& t,
105+
Storage host_mapped_storage,
106+
void* ptr,
107+
const at::Device& target_device) {
108+
auto storage = Storage(
109+
Storage::use_byte_size_t(),
110+
t.storage().nbytes(),
111+
at::DataPtr(
112+
ptr,
113+
new CUDAHostMappedIndirectContext(std::move(host_mapped_storage)),
114+
&CUDAHostMappedIndirectContext::release,
115+
target_device),
116+
nullptr, /* allocator */
117+
/*resizable=*/false);
118+
return at::empty({0}, t.options().device(target_device))
119+
.set_(std::move(storage), t.storage_offset(), t.sizes(), t.strides());
120+
}
121+
122+
// Resolve a tensor whose storage is either a direct CUDAHostMappedContext or
123+
// a CUDAHostMappedIndirectContext (the result of a previous uvm_to_cpu /
124+
// uvm_to_device_d call) back to the underlying host-mapped Storage and its
125+
// CUDAHostMappedContext. Returns {Storage(), nullptr} if `t` is not a
126+
// host-mapped tensor.
127+
std::pair<Storage, CUDAHostMappedContext*> resolve_host_mapped(
128+
const Tensor& t) {
129+
const auto deleter = t.storage().data_ptr().get_deleter();
130+
if (deleter == &CUDAHostMappedContext::release) {
131+
auto* hcontext = t.storage().data_ptr().cast_context<CUDAHostMappedContext>(
132+
&CUDAHostMappedContext::release);
133+
TORCH_CHECK(hcontext != nullptr);
134+
return {t.storage(), hcontext};
135+
}
136+
if (deleter == &CUDAHostMappedIndirectContext::release) {
137+
auto* hictx =
138+
t.storage().data_ptr().cast_context<CUDAHostMappedIndirectContext>(
139+
&CUDAHostMappedIndirectContext::release);
140+
TORCH_CHECK(hictx != nullptr);
141+
auto* hcontext =
142+
hictx->storage_.data_ptr().cast_context<CUDAHostMappedContext>(
143+
&CUDAHostMappedContext::release);
144+
TORCH_CHECK(hcontext != nullptr);
145+
return {hictx->storage_, hcontext};
146+
}
147+
return {Storage(), nullptr};
148+
}
149+
82150
// Get the default strides from the input Tensor dimensions
83151
std::vector<int64_t> defaultStrides(IntArrayRef sizes) {
84152
std::vector<int64_t> strides(sizes.size());
@@ -326,7 +394,18 @@ Tensor new_unified_tensor(
326394
bool uvm_storage_cuda(const Tensor& t) {
327395
auto deleter = t.storage().data_ptr().get_deleter();
328396
return deleter == &CUDAManagedIndirectContext::release ||
329-
deleter == &CUDAHostMappedContext::release;
397+
deleter == &CUDAHostMappedContext::release ||
398+
deleter == &CUDAHostMappedIndirectContext::release;
399+
}
400+
401+
// True for tensors backed by malloc + cudaHostRegister (i.e. allocated via
402+
// new_host_mapped_tensor or new_unified_tensor(is_host_mapped=True)).
403+
// cudaMemAdvise / cudaMemPrefetchAsync only support cudaMallocManaged memory;
404+
// invoking them on host-mapped pointers returns cudaErrorInvalidValue.
405+
bool is_host_mapped_uvm(const Tensor& t) {
406+
const auto deleter = t.storage().data_ptr().get_deleter();
407+
return deleter == &CUDAHostMappedContext::release ||
408+
deleter == &CUDAHostMappedIndirectContext::release;
330409
}
331410

332411
bool is_uvm_tensor_cuda(const Tensor& t) {
@@ -338,11 +417,21 @@ bool is_uvm_tensor_cuda(const Tensor& t) {
338417

339418
Tensor uvm_to_cpu_cuda(const Tensor& t) {
340419
TORCH_CHECK(is_uvm_tensor_cuda(t));
341-
// Don't copy the storage - just keep a reference to the original storage
420+
421+
if (auto [host_mapped_storage, hcontext] = resolve_host_mapped(t);
422+
hcontext != nullptr) {
423+
return make_host_mapped_view(
424+
t,
425+
std::move(host_mapped_storage),
426+
hcontext->ptr_,
427+
{at::DeviceType::CPU});
428+
}
429+
430+
// Managed memory path (original)
342431
auto* tcontext =
343432
t.storage().data_ptr().cast_context<CUDAManagedIndirectContext>(
344433
&CUDAManagedIndirectContext::release);
345-
TORCH_CHECK(tcontext != nullptr)
434+
TORCH_CHECK(tcontext != nullptr);
346435
auto* ocontext =
347436
tcontext->storage_.data_ptr().cast_context<CUDAManagedContext>(
348437
&CUDAManagedContext::release);
@@ -367,12 +456,23 @@ Tensor uvm_to_device(const Tensor& self, const Tensor& prototype) {
367456

368457
Tensor uvm_to_device_d(const Tensor& t, const at::Device& device) {
369458
TORCH_CHECK(is_uvm_tensor_cuda(t));
370-
// Don't copy the storage - just keep a reference to the original storage
459+
460+
if (auto [host_mapped_storage, hcontext] = resolve_host_mapped(t);
461+
hcontext != nullptr) {
462+
// Re-derive the device pointer from the host pointer rather than reusing
463+
// t.storage().data_ptr().get(): for a tensor produced by a previous
464+
// uvm_to_cpu call, that pointer is the *host* pointer, not the device one.
465+
void* dev_ptr;
466+
AT_CUDA_CHECK(cudaHostGetDevicePointer(&dev_ptr, hcontext->ptr_, 0));
467+
return make_host_mapped_view(
468+
t, std::move(host_mapped_storage), dev_ptr, device);
469+
}
470+
471+
// Managed memory path (original)
371472
auto* tcontext =
372473
t.storage().data_ptr().cast_context<CUDAManagedIndirectContext>(
373474
&CUDAManagedIndirectContext::release);
374-
TORCH_CHECK(tcontext != nullptr)
375-
475+
TORCH_CHECK(tcontext != nullptr);
376476
auto* ocontext =
377477
tcontext->storage_.data_ptr().cast_context<CUDAManagedContext>(
378478
&CUDAManagedContext::release);
@@ -395,14 +495,18 @@ int64_t uvm_get_guard_index(const Tensor& t) {
395495
TORCH_CHECK(uvm_storage_cuda(t));
396496
int cuda_device_index;
397497
if (t.is_cpu()) {
498+
// Host-mapped CPU views never reach this function: both
499+
// uvm_cuda_mem_advise and uvm_cuda_mem_prefetch_async early-return on
500+
// is_host_mapped_uvm(t) above. The only CPU-tensor path that reaches
501+
// uvm_get_guard_index is the managed-memory CPU view.
398502
auto* tcontext =
399503
t.storage().data_ptr().cast_context<CUDAManagedIndirectContext>(
400504
&CUDAManagedIndirectContext::release);
401-
TORCH_CHECK(tcontext != nullptr)
505+
TORCH_CHECK(tcontext != nullptr);
402506
auto* ocontext =
403507
tcontext->storage_.data_ptr().cast_context<CUDAManagedContext>(
404508
&CUDAManagedContext::release);
405-
TORCH_CHECK(ocontext != nullptr)
509+
TORCH_CHECK(ocontext != nullptr);
406510
cuda_device_index = static_cast<int64_t>(ocontext->cuda_device_);
407511
} else {
408512
TORCH_CHECK(t.is_cuda());
@@ -413,6 +517,13 @@ int64_t uvm_get_guard_index(const Tensor& t) {
413517
} // namespace
414518

415519
void uvm_cuda_mem_advise(const Tensor& t, int64_t cuda_memory_advise) {
520+
// cudaMemAdvise only applies to cudaMallocManaged memory. Host-mapped UVM
521+
// tensors (malloc + cudaHostRegister) are statically PCIe-mapped, with no
522+
// migration to advise on; the CUDA API returns cudaErrorInvalidValue. Treat
523+
// this as a no-op so callers don't need to special-case the allocator.
524+
if (is_host_mapped_uvm(t)) {
525+
return;
526+
}
416527
at::cuda::OptionalCUDAGuard device_guard;
417528
int64_t cuda_device_index = uvm_get_guard_index(t);
418529
int hint_device;
@@ -446,6 +557,11 @@ void uvm_cuda_mem_advise(const Tensor& t, int64_t cuda_memory_advise) {
446557
void uvm_cuda_mem_prefetch_async(
447558
const Tensor& t,
448559
std::optional<Tensor> device_t) {
560+
// cudaMemPrefetchAsync only applies to cudaMallocManaged memory; host-mapped
561+
// tensors have no migration to prefetch. See uvm_cuda_mem_advise note above.
562+
if (is_host_mapped_uvm(t)) {
563+
return;
564+
}
449565
// Call cudaMemPrefetchAsync on Tensor
450566
at::cuda::OptionalCUDAGuard device_guard;
451567
TORCH_CHECK(uvm_storage_cuda(t));

0 commit comments

Comments
 (0)