fix(agc): find a view's region when the cached address space dissolves after UploadPrepare

UploadFinish dissolves the cached address space when an import was
retired after the space was built. The space's base regions are then
merged with the build's own regions, which since readsOverBase may
overlap them. owner() searched the merged list for the region with the
greatest begin, a base region of one mapping, so a read-only V# merged
into an own region that began earlier was bound through it again:

  guest buffer view 0x1f0390c1c4+0x10c1cc exceeds its GPU owner
  0x1f03900000-0x1f03910000 (buffer 0, direct 1, mirror 0)

The dissolve now notes when the merged list overlaps, and owner() then
looks back for a region that holds the whole range. The test's third
build retires the imports between UploadPrepare and UploadFinish.
This commit is contained in:
Yunus Shaikh committed 2026-10-14 02:38:14 -07:00
1 parent d4709592e7
commit c54854b2bd
3 files changed
+21 -6

No files matched your search

@@ -389,6 +389,7 @@ private:
// Whether `regions` is in ascending address order (true right after AcquireRegistered, whose
// regions follow the registry's order), so AddSnapshot can search instead of scanning.
bool regionsSorted = false;
bool regionsOverlap = false;
std::vector<std::pair<std::uint64_t, std::uint64_t>> writes;
std::vector<std::pair<std::uint64_t, std::uint64_t>> readsOverBase;
std::vector<std::pair<std::uint64_t, std::vector<std::byte>>> heapReferences;
@@ -2580,6 +2580,11 @@ void GuestBufferMemory::dissolveSpace(bool resolve) {
// Both sorted and disjoint (UploadPrepare merged this build's regions; the space's are
// registered ranges), so the merge keeps the order Descriptor searches.
std::merge(space->base.begin(), space->base.end(), std::make_move_iterator(regions.begin()), std::make_move_iterator(regions.end()), std::back_inserter(merged), [](const Region& left, const Region& right) { return left.begin < right.begin; });
std::uint64_t reached = 0;
for (const auto& region : merged) {
regionsOverlap = regionsOverlap || region.begin < reached;
reached = std::max(reached, region.end);
}
} else {
merged.assign(space->base.begin(), space->base.end());
merged.insert(merged.end(), std::make_move_iterator(regions.begin()), std::make_move_iterator(regions.end()));
@@ -2597,12 +2602,19 @@ void GuestBufferMemory::dissolveSpace(bool resolve) {
}
const GuestBufferMemory::Region* GuestBufferMemory::owner(std::uint64_t address, std::uint64_t end) const {
const auto candidate = [address](const std::vector<Region>& list) -> const Region* {
const auto candidate = [address, end](const std::vector<Region>& list, bool overlapping) -> const Region* {
const auto found = std::upper_bound(list.begin(), list.end(), address, [](std::uint64_t value, const Region& region) { return value < region.begin; });
return found == list.begin() ? nullptr : &*std::prev(found);
if (found == list.begin()) return nullptr;
if (overlapping) {
for (auto it = found; it != list.begin();) {
--it;
if (end <= it->end) return &*it;
}
}
return &*std::prev(found);
};
const auto* own = candidate(regions);
const auto* shared = space != nullptr ? candidate(space->base) : nullptr;
const auto* own = candidate(regions, regionsOverlap);
const auto* shared = space != nullptr ? candidate(space->base, false) : nullptr;
const auto holds = [end](const Region* region) { return region != nullptr && end <= region->end; };
if (holds(own) != holds(shared)) return holds(own) ? own : shared;
if (own == nullptr || (shared != nullptr && shared->begin > own->begin)) return shared;
@@ -581,12 +581,14 @@ void spaceReadsAcrossImportsMergedTests(const Context& context, const BdaTestAcc
const std::uint64_t lowerBytes = 512;
const std::uint64_t upper = second + 128;
const std::uint64_t upperBytes = third + 256 - upper;
for (int build = 0; build < 2; ++build) {
for (int build = 0; build < 3; ++build) {
GuestBufferMemory leased(importing);
leased.AcquireRegistered();
leased.AddReadable(lower, lowerBytes);
leased.AddReadable(upper, upperBytes);
leased.Upload(true);
leased.UploadPrepare(true);
if (build == 2) ClearHostImports(importing.device);
leased.UploadFinish(true);
for (const auto& [address, bytes] : {std::pair{lower, lowerBytes}, std::pair{upper, upperBytes}}) {
std::uint32_t adjustment = 0;
const auto view = leased.Descriptor(address, bytes, adjustment);