[GH-ISSUE #4871] [Bug]: MemFabric TENT registers adjacent buffers individually, potentially causing overlapping memory registrations #1237

New issue

Closed

opened 2026-10-29 21:24:17 +00:00 by bsakaguchi · 2 comments

bsakaguchi commented

2026-10-29 21:24:17 +00:00

Owner

Originally created by @jamesjxliu on GitHub (Oct 23, 2026).
Original GitHub issue: https://github.com/kvcache-ai/Mooncake/issues/4871

Bug Report

The vector overload of MemFabricTransport::addMemoryBuffer() currently loops over the buffers and calls the single-buffer registration method for each one.
MemFabric aligns memory ranges to NPU page boundaries before registering them. Two adjacent, non-overlapping buffers can therefore produce overlapping aligned ranges when registered separately. With device_urma, the subsequent memory registration may fail with an “overlaps existing MR” error.

Before submitting...

Ensure you searched for relevant issues and read the [documentation]

Originally created by @jamesjxliu on GitHub (Oct 23, 2026). Original GitHub issue: https://github.com/kvcache-ai/Mooncake/issues/4871 ### Bug Report The vector overload of MemFabricTransport::addMemoryBuffer() currently loops over the buffers and calls the single-buffer registration method for each one. MemFabric aligns memory ranges to NPU page boundaries before registering them. Two adjacent, non-overlapping buffers can therefore produce overlapping aligned ranges when registered separately. With device_urma, the subsequent memory registration may fail with an “overlaps existing MR” error. ### Before submitting... - [x] Ensure you searched for relevant issues and read the [documentation]

bsakaguchi

2026-10-29 21:24:17 +00:00

closed this issue
added the
bug
label

bsakaguchi commented

2026-10-29 21:24:18 +00:00

Author

Owner

@github-actions[bot] commented on GitHub (Oct 23, 2026):

Thanks for opening this issue, @jamesjxliu!

Field	Value
Issue	#4871
GitHub user ID	`20507750`
Reporter	@jamesjxliu

A maintainer will triage this when possible. To help us respond faster, please include:

Mooncake version or commit SHA
Environment (OS, CUDA/driver, RDMA stack if relevant)
Steps to reproduce and expected vs. actual behavior

Useful links: Documentation · Contributing guide

This message was posted automatically by the issue bot.

@github-actions[bot] commented on GitHub (Oct 23, 2026): Thanks for opening this issue, @jamesjxliu! | Field | Value | |-------|-------| | **Issue** | #4871 | | **GitHub user ID** | `20507750` | | **Reporter** | @jamesjxliu | A maintainer will triage this when possible. To help us respond faster, please include: - Mooncake version or commit SHA - Environment (OS, CUDA/driver, RDMA stack if relevant) - Steps to reproduce and expected vs. actual behavior Useful links: [Documentation](https://kvcache-ai.github.io/Mooncake/) · [Contributing guide](https://github.com/kvcache-ai/Mooncake/blob/main/CONTRIBUTING.md) > This message was posted automatically by the issue bot.

bsakaguchi commented

2026-10-29 21:24:18 +00:00

Author

Owner

@he-yufeng commented on GitHub (Oct 23, 2026):

Confirmed the mechanism on current main. MemFabricTransport::addMemoryBuffer(std::vector<BufferDesc>&) is a plain loop over the single-buffer overload, and that overload hands each range straight to MemFabricUnderApi::SmemTransRegisterMem, which is a dlsym of the vendor smem_trans_register_mem. So the outward page rounding you describe happens below this repo's visibility and every vector element becomes its own MR.

The overlap condition is narrower than "adjacent", which is worth pinning for whoever picks this up: two buffers only collide after rounding when the shared boundary is not itself page aligned. [0, 100) and [100, 200) with a 4 KiB page both round to [0, 4096) and the second registration fails; [0, 4096) and [4096, 8192) stay disjoint. Callers that hand over slab pieces at arbitrary offsets hit it, callers that hand over page-aligned segment splits do not.

On the fix shape, merging contiguous ranges before registration is the obvious move, but removeMemoryBuffer deregisters by address and there is no refcount today, so a merged registration needs one: the merged range stays registered until its last member is removed, and a per-desc remove must not tear down a range other descs still sit in. That is the part I would want a regression around, not the coalescing loop itself. If the vendor side could take a list and register one MR per aligned range instead, that would be the cleaner contract and would keep this file's per-desc semantics intact.

@he-yufeng commented on GitHub (Oct 23, 2026): Confirmed the mechanism on current main. `MemFabricTransport::addMemoryBuffer(std::vector<BufferDesc>&)` is a plain loop over the single-buffer overload, and that overload hands each range straight to `MemFabricUnderApi::SmemTransRegisterMem`, which is a dlsym of the vendor `smem_trans_register_mem`. So the outward page rounding you describe happens below this repo's visibility and every vector element becomes its own MR. The overlap condition is narrower than "adjacent", which is worth pinning for whoever picks this up: two buffers only collide after rounding when the shared boundary is not itself page aligned. `[0, 100)` and `[100, 200)` with a 4 KiB page both round to `[0, 4096)` and the second registration fails; `[0, 4096)` and `[4096, 8192)` stay disjoint. Callers that hand over slab pieces at arbitrary offsets hit it, callers that hand over page-aligned segment splits do not. On the fix shape, merging contiguous ranges before registration is the obvious move, but `removeMemoryBuffer` deregisters by address and there is no refcount today, so a merged registration needs one: the merged range stays registered until its last member is removed, and a per-desc remove must not tear down a range other descs still sit in. That is the part I would want a regression around, not the coalescing loop itself. If the vendor side could take a list and register one MR per aligned range instead, that would be the cleaner contract and would keep this file's per-desc semantics intact.

bsakaguchi referenced this issue

2026-10-29 21:32:45 +00:00

[PR #1237] [MERGED] [Doc] add more mooncake store APIs doc #2095