Skip to content

refactor the rbtree / interval tree in virtio thunk and fix issue for virtio driver - #12469

Merged
HongleiHuang-amd merged 1 commit into
developfrom
users/honghuan/rbtree-fix
Oct 9, 2026
Merged

HongleiHuang-amd merged 1 commit into
developfrom
users/honghuan/rbtree-fix

Conversation

@HongleiHuang-amd

@HongleiHuang-amd HongleiHuang-amd commented Sep 29, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

Refactor rbtree/interval tree with internal implementation

Technical Details

refactor the rbtree in virtio thunkloader

Issue Tracking

JIRA ID : SWCSD-12742

Test Plan

rocr
hiptest

Test Result

all pass

@therock-pr-bot

therock-pr-bot Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

✅ All Policy Checks Passed

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ⚠️ Warning Error: Source/code files changed without an accompanying unit test.
Expected: add at least one test file named like test_<name>.py / test_<name>.cpp (or <name>_test.*).
Current: code file(s) changed: projects/rocr-runtime/libhsakmt/src/virtio/hsakmt_interval_tree.c, projects/rocr-runtime/libhsakmt/src/virtio/hsakmt_interval_tree.h, projects/rocr-runtime/libhsakmt/src/virtio/hsakmt_virtio_memory.c; no test file found
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —

🎉 All policy checks passed!

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

This comment was marked as resolved.

@dayatsin-amd dayatsin-amd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems this patches is bringing in some other changes other than the RB Tree refactor.

Can you please move the other changes to a separate PR.

@HongleiHuang-amd
HongleiHuang-amd force-pushed the users/honghuan/rbtree-fix branch from 8afff35 to 765593a Compare October 1, 2026 00:19
@HongleiHuang-amd

Copy link
Copy Markdown
Contributor Author

It seems this patches is bringing in some other changes other than the RB Tree refactor.

Can you please move the other changes to a separate PR.

Got it, only keep the license scan fix commit now.

@HongleiHuang-amd
HongleiHuang-amd force-pushed the users/honghuan/rbtree-fix branch from 18ddeac to 0e0f851 Compare October 1, 2026 00:29
@ROCm ROCm deleted a comment from therock-pr-bot Bot Oct 1, 2026
@HongleiHuang-amd
HongleiHuang-amd force-pushed the users/honghuan/rbtree-fix branch from 0e0f851 to fba8eea Compare October 1, 2026 14:03

@cfreeamd cfreeamd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 AI-assisted pre-review (for @cfreeamd)

[Required] PR title refactor the rbtree / interval tree in virtio thunk and fix issue for virtio driver does not follow the required type(component): summary format that becomes the permanent squash-merge commit. Suggested: refactor(hsakmt): replace custom augmented interval tree with plain rbtree scan in virtio thunk.

Other files reviewed: hsakmt_interval_tree.c, hsakmt_interval_tree.h, hsakmt_virtio_memory.c. The algorithm is correct — interval_tree_scan starting from the BST minimum and terminating when node->start > last is a valid O(n) scan on a tree ordered by start. The O(n) complexity is explicitly documented in the header comment.

@HongleiHuang-amd
HongleiHuang-amd force-pushed the users/honghuan/rbtree-fix branch 2 times, most recently from dac749d to d3849d7 Compare October 8, 2026 03:13
Copilot AI requested a review from a team as a code owner October 8, 2026 06:09
@github-actions github-actions Bot added the github actions Pull requests that update GitHub Actions code label Oct 8, 2026
@therock-pr-bot

therock-pr-bot Bot commented Oct 8, 2026

Copy link
Copy Markdown

🎉 All checks passed! This PR is ready for review.

…sed scan

hsakmt_interval_tree.c/.h were a structural port of the Linux kernel's
augmented rbtree (lib/rbtree.c, rbtree_augmented.h, interval_tree_generic.h),
GPL-2.0 source carrying an MIT header (Palamida SWCSD-15612 finding).

Keep the file names and public API (interval_tree_t, interval_tree_node_t,
hsakmt_interval_tree_insert/remove/iter_first/iter_next) unchanged to avoid
a wide-reaching rename and to keep the door open for a real augmented
interval tree later; only the internal implementation changes.

The new implementation is built on the existing BSD-2-Clause nginx-derived
rbtree.c already used for bo_rbt. Ranges are ordered by start address and
overlap queries do a bounded ascending scan instead of maintaining an
augmented subtree-max invariant. This is O(n) worst case instead of
O(log n), an acceptable trade-off given the small number of userptr/SVM
ranges tracked per process.

Signed-off-by: Honglei Huang <honghuan@amd.com>
@HongleiHuang-amd
HongleiHuang-amd force-pushed the users/honghuan/rbtree-fix branch from dac749d to 980c0b9 Compare October 8, 2026 08:11
@HongleiHuang-amd
HongleiHuang-amd merged commit 63802ea into develop Oct 9, 2026
167 of 168 checks passed
@HongleiHuang-amd
HongleiHuang-amd deleted the users/honghuan/rbtree-fix branch October 9, 2026 01:19
systems-assistant Bot pushed a commit to ROCm/ROCR-Runtime that referenced this pull request Oct 9, 2026
 rbtree-based scan

hsakmt_interval_tree.c/.h were a structural port of the Linux kernel's
augmented rbtree (lib/rbtree.c, rbtree_augmented.h, interval_tree_generic.h),
GPL-2.0 source carrying an MIT header (Palamida SWCSD-15612 finding).

Keep the file names and public API (interval_tree_t, interval_tree_node_t,
hsakmt_interval_tree_insert/remove/iter_first/iter_next) unchanged to avoid
a wide-reaching rename and to keep the door open for a real augmented
interval tree later; only the internal implementation changes.

The new implementation is built on the existing BSD-2-Clause nginx-derived
rbtree.c already used for bo_rbt. Ranges are ordered by start address and
overlap queries do a bounded ascending scan instead of maintaining an
augmented subtree-max invariant. This is O(n) worst case instead of
O(log n), an acceptable trade-off given the small number of userptr/SVM
ranges tracked per process.

Signed-off-by: Honglei Huang <honghuan@amd.com>
[rocm-systems] ROCm/rocm-systems#12469 (commit 63802ea)
@systems-assistant

Copy link
Copy Markdown
Contributor

TheRock Submodule Bump Activity

Newest first

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

github actions Pull requests that update GitHub Actions code organization: ROCm project: rocr-runtime

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants