Skip to content

shim: take the cubin CRC off the application's thread, which deadlocked it (#211) #212

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dpsoft merged 1 commit into from
Oct 27, 2026
Merged

shim: take the cubin CRC off the application's thread, which deadlocked it (#211) #212

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions shim/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,10 @@ nvidia/testdata/cuda_concurrent: nvidia/testdata/cuda_concurrent.cu
# that the enforcement works, by trying to violate it five ways and requiring
# every one to be rejected by the compiler.
#
# Mode 6 guards the opposite direction (issue #211): capture() takes no CRC
# function, because the CRC is a guarded CUPTI call and the callback can run
# on an application thread holding a driver lock the guard's holder needs.
#
# Mode 0 is the control and it MUST compile. Without it the whole check would
# pass just as happily on a file that fails to build for an unrelated reason -
# a typo, a renamed header - and would then be proving nothing, which is the
Expand All @@ -344,17 +348,18 @@ check-cubin-defer: core/cubin_defer_test.cc core/cubinqueue.h
echo "check-cubin-defer: the COMPLIANT capture does not compile; this check would"; \
echo " otherwise pass for the wrong reason. Fix core/cubinqueue.h."; \
exit 1; }
@rc=0; for m in 1 2 3 4 5; do \
@rc=0; for m in 1 2 3 4 5 6; do \
if $(CXX) -std=c++17 -Wall -Werror -I core -fsyntax-only \
-DPERFAGENT_CUBIN_DEFER=$$m core/cubin_defer_test.cc 2>/dev/null; then \
echo "check-cubin-defer: deferral mode $$m COMPILED. The vendor's pointer can escape"; \
echo "check-cubin-defer: mode $$m COMPILED. Modes 1-5: the vendor's pointer can escape"; \
echo " the callback, so the copy can be deferred past the buffer's"; \
echo " lifetime and a late reader gets silently WRONG bytes (spec"; \
echo " 6.3 finding 2). See core/cubin_defer_test.cc mode $$m."; \
echo " 6.3 finding 2). Mode 6: the callback can take the CRC, a guarded"; \
echo " CUPTI call, which deadlocks (issue #211). See mode $$m."; \
rc=1; \
fi; \
done; \
[ $$rc -eq 0 ] && echo "check-cubin-defer: OK - the compliant capture compiles, all 5 deferrals do not"; \
[ $$rc -eq 0 ] && echo "check-cubin-defer: OK - the compliant capture compiles, all 6 violations do not"; \
exit $$rc

# Issue #99's structural assertion, run as a BUILD step for the same reason
Expand Down
25 changes: 22 additions & 3 deletions shim/core/cubin_defer_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,18 @@
// 3 move the view into a container
// 4 read the borrowed pointer straight out of the view
// 5 assign one view over another
// 6 compute the CRC inside capture (issue #211)
//
// Each of 1-5 is a real thing an implementer would reach for while "just
// deferring the copy to the drain thread"; each is a compile error, and the
// Makefile prints which.
//
// Mode 6 is the opposite temptation: doing MORE in the callback. The CRC is
// a vendor call behind the CUPTI guard, and the callback can run on an
// application thread inside cuLaunchKernel, holding a driver lock that the
// guard's holder is waiting for. capture() therefore takes no CRC function,
// and putting one back is a compile error rather than a deadlock found a
// release later.
#include "cubinqueue.h"

#include <cstdint>
Expand All @@ -34,12 +42,14 @@

namespace {

#if PERFAGENT_CUBIN_DEFER == 6
uint64_t fake_crc(const void *bytes, size_t len) {
(void)bytes;
return len ? 0x1234u : 0u;
}

void fake_captured(void *, uint64_t, const void *, size_t) {}
void fake_captured(void *, uint64_t, uint64_t, uint64_t, const void *, size_t) {}
#endif

} // namespace

Expand All @@ -49,7 +59,7 @@ void fake_captured(void *, uint64_t, const void *, size_t) {}
// capture, and dies there. The copy is taken before the function returns.
bool compliant(perfagent::CubinQueue &q, const void *vendor_bytes, size_t len) {
const perfagent::CubinView view(vendor_bytes, len);
return q.capture(view, fake_crc, fake_captured, nullptr);
return q.capture(view, /*module_id=*/1, /*load_ns=*/0);
}

#elif PERFAGENT_CUBIN_DEFER == 1
Expand Down Expand Up @@ -100,6 +110,15 @@ void defer_by_reassigning(perfagent::CubinView &slot, const void *vendor_bytes,
slot = perfagent::CubinView(vendor_bytes, len);
}

#elif PERFAGENT_CUBIN_DEFER == 6

// "The CRC is cheap, take it with the copy like it used to be." It is a
// vendor call behind the CUPTI guard, and that is issue #211.
bool crc_in_the_callback(perfagent::CubinQueue &q, const void *vendor_bytes, size_t len) {
const perfagent::CubinView view(vendor_bytes, len);
return q.capture(view, fake_crc, fake_captured, nullptr);
}

#else
#error "PERFAGENT_CUBIN_DEFER must be 0..5"
#error "PERFAGENT_CUBIN_DEFER must be 0..6"
#endif
158 changes: 104 additions & 54 deletions shim/core/cubinqueue.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,19 @@ CubinQueue::CubinQueue(const CubinQueueLimits &limits) : limits_(limits) {}

CubinQueue::~CubinQueue() {
std::lock_guard<std::mutex> g(mu_);
for (Entry &e : pending_) free(e.bytes);
pending_.clear();
pending_bytes_ = 0;
for (Entry &e : q_) free(e.bytes);
q_.clear();
queued_bytes_ = 0;
}

bool CubinQueue::capture(const CubinView &view, CubinCrcFn crc, CubinCapturedFn on_captured,
void *ctx) {
bool CubinQueue::capture(const CubinView &view, uint64_t module_id, uint64_t load_ns) {
// Everything before the memcpy is a decision NOT to copy - a bound, or a
// missing input. Nothing here defers the copy, and nothing here may: see
// cubinqueue.h and core/cubin_defer_test.cc.
if (view.empty() || crc == nullptr) return false;
if (view.empty()) return false;
const size_t len = view.size();
if (len > limits_.max_cubin_bytes) {
// Refused whole, never truncated. A truncated cubin parses into a
Expand All @@ -47,67 +49,110 @@ bool CubinQueue::capture(const CubinView &view, CubinCrcFn crc, CubinCapturedFn
return false;
}

// Over the copy, never over the view: a CRC and the bytes it names must
// come from one read of one buffer.
const uint64_t key = crc(copy, len);
if (key == 0) {
// Zero is the ABI's "no module". Sending it would produce a module
// record that joins to nothing, with every other counter green.
free(copy);
crc_failed_.fetch_add(1, std::memory_order_relaxed);
return false;
}

// One lock from the intern check to the push, so two threads loading the
// same module cannot both decide they are the first and fire two records
// for one CRC. on_captured runs inside it and is documented as forbidden
// from re-entering the queue; it is a probe fire, which is a nop when no
// consumer is attached and a ~1-2us uprobe trap when one is.
// And that is all the callback does. The CRC is NOT taken here: on the
// vendor side it takes the CUPTI guard, and this thread may be inside
// cuLaunchKernel holding a driver lock the guard's holder is waiting
// for (issue #211). settle() computes it, over this same copy, on a
// thread that holds nothing the driver needs.
std::unique_lock<std::mutex> g(mu_);

if (interned_.count(key) != 0) {
if (pending_.size() >= limits_.max_pending_entries ||
pending_bytes_ + len > limits_.max_pending_bytes) {
// More modules inside one settle tick than the pending bounds hold.
// Unlike a full offer queue this loses the record as well as the
// bytes, because the CRC the record carries is never computed --
// which is why it has a counter of its own.
g.unlock();
free(copy);
reload_skipped_.fetch_add(1, std::memory_order_relaxed);
pending_full_.fetch_add(1, std::memory_order_relaxed);
return false;
}
Entry e;
e.crc = 0;
e.module_id = module_id;
e.load_ns = load_ns;
e.bytes = copy;
e.len = len;
pending_.push_back(e);
pending_bytes_ += len;
return true;
}

// Fired here, before the push, because this is the last moment the copy
// is owned by nobody else: after the push the drain thread may free it,
// and gpu_module_load_v1.bytes_ptr would then name freed memory. The
// record stays accurate exactly because it is emitted here.
if (on_captured != nullptr) on_captured(ctx, key, copy, len);

if (q_.size() >= limits_.max_entries || queued_bytes_ + len > limits_.max_queued_bytes) {
// The OFFER is dropped; the copy has already served its purpose of
// being taken in time. The module goes unresolvable rather than the
// application going slow, and this counter is the size of that.
//
// Deliberately NOT interned: a later re-load of the same module gets
// another chance at a queue that may since have drained.
g.unlock();
free(copy);
queue_full_.fetch_add(1, std::memory_order_relaxed);
return false;
size_t CubinQueue::settle(CubinCrcFn crc, CubinCapturedFn on_captured, void *ctx) {
if (crc == nullptr) return 0;
size_t budget;
{
std::lock_guard<std::mutex> g(mu_);
budget = pending_.size();
}
size_t done = 0;
for (; done < budget; done++) {
Entry e;
{
std::lock_guard<std::mutex> g(mu_);
if (pending_.empty()) break;
e = pending_.front();
pending_.pop_front();
pending_bytes_ -= e.len;
}

// Interning is bounded like everything else. Past the bound we stop
// remembering rather than stop capturing: the consequence of forgetting
// is a duplicate offer, which the consumer answers with a counted no-op
// (CubinsDuplicate), and the consequence of not capturing is a module
// nothing can resolve. The cheaper mistake is the one we make.
if (interned_.size() < limits_.max_interned) interned_.insert(key);
// Over the copy, never over the view: a CRC and the bytes it names
// must come from one read of one buffer. The mutex is NOT held: this
// is a vendor call behind the CUPTI guard, and a capture() blocked on
// mu_ behind it is the #211 cycle again with one more lock in it.
e.crc = crc(e.bytes, e.len);
if (e.crc == 0) {
// Zero is the ABI's "no module". Sending it would produce a
// module record that joins to nothing, with every other counter
// green.
free(e.bytes);
crc_failed_.fetch_add(1, std::memory_order_relaxed);
continue;
}

Entry e;
e.crc = key;
e.bytes = copy;
e.len = len;
q_.push_back(e);
queued_bytes_ += len;
g.unlock();
// Only settle() touches interned_, and only one thread settles, so
// two loads of one module can no longer both decide they are first:
// they are settled one after the other on the same thread.
if (interned_.count(e.crc) != 0) {
free(e.bytes);
reload_skipped_.fetch_add(1, std::memory_order_relaxed);
continue;
}

captured_.fetch_add(1, std::memory_order_relaxed);
return true;
// Fired here, before the push, because this is the last moment the
// copy is owned by nobody else: after the push drain() may free it,
// and gpu_module_load_v1.bytes_ptr would then name freed memory.
if (on_captured != nullptr)
on_captured(ctx, e.module_id, e.load_ns, e.crc, e.bytes, e.len);

std::unique_lock<std::mutex> g(mu_);
if (q_.size() >= limits_.max_entries || queued_bytes_ + e.len > limits_.max_queued_bytes) {
// The OFFER is dropped; the copy has already served its purpose
// of being taken in time, and the record above has told the
// consumer the module exists. The module goes unresolvable
// rather than the application going slow, and this counter is
// the size of that.
//
// Deliberately NOT interned: a later re-load of the same module
// gets another chance at a queue that may since have drained.
g.unlock();
free(e.bytes);
queue_full_.fetch_add(1, std::memory_order_relaxed);
continue;
}
q_.push_back(e);
queued_bytes_ += e.len;
g.unlock();

// Interning is bounded like everything else. Past the bound we stop
// remembering rather than stop capturing: the consequence of
// forgetting is a duplicate offer, which the consumer answers with a
// counted no-op (CubinsDuplicate), and the consequence of not
// capturing is a module nothing can resolve. The cheaper mistake is
// the one we make.
if (interned_.size() < limits_.max_interned) interned_.insert(e.crc);
captured_.fetch_add(1, std::memory_order_relaxed);
}
return done;
}

size_t CubinQueue::drain(CubinOfferFn offer, unsigned timeout_ms) {
Expand Down Expand Up @@ -154,4 +199,9 @@ size_t CubinQueue::depth() const {
return q_.size();
}

size_t CubinQueue::pending() const {
std::lock_guard<std::mutex> g(mu_);
return pending_.size();
}

} // namespace perfagent
Loading
Loading