Skip to content

refactor(uffd): store pageSize on struct#2419

Merged
ValentaTomas merged 3 commits intomainfrom
refactor/uffd-pagesize-on-struct
Apr 18, 2026
Merged

refactor(uffd): store pageSize on struct#2419
ValentaTomas merged 3 commits intomainfrom
refactor/uffd-pagesize-on-struct

Conversation

@ValentaTomas
Copy link
Copy Markdown
Member

@ValentaTomas ValentaTomas commented Apr 16, 2026

Summary

  • Store pageSize on the Userfaultfd struct instead of retrieving it from mapping functions on every fault.
  • Drop the pagesize return value from Mapping.GetOffset and Mapping.GetHostVirtAddr; callers now read u.pageSize.
  • Drop the pagesize parameter from faultPage.
  • Update mapping offset tests for the new signatures.

All regions in a mapping are already validated to share the same page size in NewUserfaultfdFromFd, so a single stored value is sufficient.

Pure refactor, no behavior change.

Split out of this PR

Test plan

  • go build ./... passes
  • go vet ./pkg/sandbox/uffd/... clean
  • golangci-lint run ./pkg/sandbox/uffd/... clean
  • Existing mapping tests updated

@cursor
Copy link
Copy Markdown

cursor bot commented Apr 16, 2026

PR Summary

Low Risk
Low risk refactor that primarily changes function signatures and threads a cached pageSize through the userfaultfd fault/prefault paths. Main risk is missed call-site updates leading to incorrect page sizing or compile-time breakage.

Overview
Stores a single pageSize on Userfaultfd (validated once at construction) and refactors the UFFD faulting/prefaulting path to use it, removing per-call page-size plumbing. As part of this, Mapping.GetOffset and Mapping.GetHostVirtAddr drop their page-size return values, faultPage no longer accepts a pagesize parameter, and the mapping offset tests are updated to the new signatures.

Reviewed by Cursor Bugbot for commit be402d1. Bugbot is set up for automated code reviews on this repo. Configure here.

Comment thread packages/orchestrator/pkg/sandbox/uffd/userfaultfd/invoke.go Outdated
Comment thread packages/orchestrator/pkg/sandbox/uffd/userfaultfd/prefault.go Outdated
Comment thread packages/orchestrator/pkg/sandbox/uffd/userfaultfd/prefault.go Outdated
- Store pageSize on Userfaultfd struct instead of retrieving it from
  mapping functions on every fault
- Drop pagesize return value from GetOffset and GetHostVirtAddr;
  callers now read u.pageSize (already validated to be uniform across
  regions in NewUserfaultfdFromFd)
- Drop pagesize parameter from faultPage
- Update mapping offset tests for the new signatures

Pure refactor, no behavior change.
@ValentaTomas ValentaTomas force-pushed the refactor/uffd-pagesize-on-struct branch from 563aee3 to 1a86243 Compare April 18, 2026 07:50
@ValentaTomas ValentaTomas changed the title refactor(uffd): store pageSize on struct and extract prefault to own file refactor(uffd): store pageSize on struct Apr 18, 2026
@ValentaTomas ValentaTomas enabled auto-merge (squash) April 18, 2026 07:52
Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 754ef36. Configure here.

GetOffset no longer returns pagesize, so the expectedPagesize field
and its per-case values in TestMapping_GetOffset were dead.
@ValentaTomas ValentaTomas merged commit d434ccd into main Apr 18, 2026
45 checks passed
@ValentaTomas ValentaTomas deleted the refactor/uffd-pagesize-on-struct branch April 18, 2026 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants