Skip to content

feat(uffd,fc): wire balloon free-page-reporting#2541

Open
ValentaTomas wants to merge 6 commits intofeat/uffd-remove-events-matrixfrom
feat/uffd-fc-free-page-reporting-integration
Open

feat(uffd,fc): wire balloon free-page-reporting#2541
ValentaTomas wants to merge 6 commits intofeat/uffd-remove-events-matrixfrom
feat/uffd-fc-free-page-reporting-integration

Conversation

@ValentaTomas
Copy link
Copy Markdown
Member

@ValentaTomas ValentaTomas commented May 3, 2026

Wires Firecracker balloon free-page-reporting through the template build into pkg/sandbox/fc. In the orchestrator path FPR turns on only when fcInfo.HasFreePageReporting() (FC v1.14+) AND the free-page-reporting LaunchDarkly flag are both true; cmd/create-build defaults to the FC-version-derived value and accepts an explicit --free-page-reporting override.

Depends on #2545#2520 — without the REMOVE-event handling from #2520, FC's madvise(MADV_DONTNEED) on balloon deflate would race in-flight pagefault workers.

@cursor
Copy link
Copy Markdown

cursor Bot commented May 3, 2026

PR Summary

Medium Risk
Touches VM lifecycle and snapshot/diff metadata paths (Firecracker API config + UFFD dirty/empty tracking), so mis-gating by version/flag or incorrect bitmap handling could lead to resume corruption or runtime failures. Rollout risk is mitigated by version checks (v1.14+) and a feature flag, but the new defaults/overrides should be validated across upgrade/downgrade scenarios.

Overview
This PR plumbs a new FreePageReporting setting from template creation/build inputs into sandbox runtime so Firecracker can enable balloon free-page-reporting (defaulting on for FC v1.14+ and gated by a LaunchDarkly flag, with an explicit cmd/create-build --free-page-reporting override), and updates snapshot diff generation to treat UFFD REMOVE’d pages as Empty so freed guest pages restore as zero rather than leaking stale data; key things to double-check are the FC-version parsing/compare (suffix handling), behavior when the version check fails (silent fallback), and that the new dirty∩empty masking doesn’t accidentally drop real dirty pages under unexpected FC/host behavior.

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

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 d472bdf. Configure here.

Comment thread packages/shared/pkg/fcversion/sandbox_features.go
@ValentaTomas ValentaTomas force-pushed the feat/uffd-fc-free-page-reporting-integration branch from 0a2570e to 99eec35 Compare May 3, 2026 02:21
@ValentaTomas ValentaTomas force-pushed the feat/uffd-fc-free-page-reporting-integration branch from 0fd4caf to d30e0ef Compare May 3, 2026 04:52
@ValentaTomas ValentaTomas changed the title feat(uffd): wire free-page-reporting through template build to FC balloon feat(uffd,fc): wire balloon free-page-reporting (auto for FC v1.14+) May 3, 2026
@ValentaTomas ValentaTomas force-pushed the feat/uffd-fc-free-page-reporting-integration branch 3 times, most recently from bf23242 to 6d4b734 Compare May 3, 2026 05:28
@ValentaTomas ValentaTomas force-pushed the feat/uffd-remove-events-matrix branch from 68037d2 to ceec7d6 Compare May 3, 2026 07:10
@ValentaTomas ValentaTomas force-pushed the feat/uffd-fc-free-page-reporting-integration branch from d38614d to c777e5c Compare May 3, 2026 07:18
@ValentaTomas ValentaTomas force-pushed the feat/uffd-remove-events-matrix branch from ceec7d6 to 9ce0941 Compare May 3, 2026 07:36
@ValentaTomas ValentaTomas force-pushed the feat/uffd-fc-free-page-reporting-integration branch from c777e5c to 6faaefa Compare May 3, 2026 07:39
@ValentaTomas ValentaTomas marked this pull request as ready for review May 3, 2026 08:04
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6faaefab87

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread packages/orchestrator/pkg/sandbox/uffd/uffd.go
Comment thread packages/orchestrator/pkg/sandbox/uffd/uffd.go
@ValentaTomas ValentaTomas force-pushed the feat/uffd-remove-events-matrix branch from 9ce0941 to 5896b7b Compare May 3, 2026 08:43
@ValentaTomas ValentaTomas force-pushed the feat/uffd-fc-free-page-reporting-integration branch from 6faaefa to 4bb8be8 Compare May 3, 2026 08:43
@ValentaTomas ValentaTomas changed the title feat(uffd,fc): wire balloon free-page-reporting (auto for FC v1.14+) feat(uffd,fc): wire balloon free-page-reporting May 3, 2026
@ValentaTomas ValentaTomas force-pushed the feat/uffd-fc-free-page-reporting-integration branch 2 times, most recently from 07ab37d to 4951164 Compare May 3, 2026 09:39
@ValentaTomas ValentaTomas force-pushed the feat/uffd-remove-events-matrix branch from 345f7e9 to db978d4 Compare May 3, 2026 10:22
…loon

Adds the FC-side integration plumbing for free page reporting on top of
the UFFD REMOVE-event handling in #2520:

- template-manager proto: optional bool freePageReporting (field 17).
- TemplateConfig + sandbox.Config gain a FreePageReporting bool that
  flows from template create → build phases (base/steps/finalize) →
  sandbox factory → fc.Process.Create.
- fc.apiClient.enableFreePageReporting calls PUT /balloon with
  free_page_reporting=true after entropy setup and before VM start.
- fcversion.HasFreePageReporting gates rollout to FC v1.14+.
- Adds free-page-reporting LaunchDarkly feature flag.
- create-build CLI: --free-page-reporting flag, defaults to enabled
  when FC version supports it.
- smoketest: opportunistically enables FPR when the FC version under
  test supports it.

UFFD-side changes (REMOVE handling, page tracker, race tests, fix)
remain in #2520; this PR is purely the production rollout path.
…rchestrator

HasFreePageReporting() was added to fcversion.Info but had zero callers
in the production path. Mirror the HasHugePages() pattern: let the
orchestrator derive the value from the FC version (authoritative),
gated by the FreePageReportingFlag LaunchDarkly flag (default false).

Also emit an env.free_page_reporting span attribute alongside the
existing env.huge_pages one.
Read the Removed bitmap from PageTracker and emit it as DiffMetadata.Empty
so REMOVE'd pages become uuid.Nil mappings in the snapshot header (read as
zero on resume). Defensively AndNot the empty set out of dirty: settle drains
make these disjoint in practice (Removed pages have no PTE, WP-async only
sees present pages with WP cleared), but if the invariant ever breaks the
guest's last intent for a Removed page is "free, read zero on restore" — so
empty must win, not stale dirty content.
@ValentaTomas ValentaTomas force-pushed the feat/uffd-fc-free-page-reporting-integration branch from 4951164 to 2f6f62a Compare May 3, 2026 10:28
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