Skip to content

[KV Offload][FS] Opt-in per-block CRC32C checksums - #57912

Open
CHIPMUNK-T0T wants to merge 5 commits into
vllm-project:mainfrom
CHIPMUNK-T0T:feat/kv-offload-fs-block-checksum
Open

CHIPMUNK-T0T wants to merge 5 commits into
vllm-project:mainfrom
CHIPMUNK-T0T:feat/kv-offload-fs-block-checksum

Conversation

@CHIPMUNK-T0T

@CHIPMUNK-T0T CHIPMUNK-T0T commented Sep 21, 2026 •

Copy link
Copy Markdown
Contributor

Purpose

Implements item 1 of #54363. The FS tier catches a truncated block (short read) but not one whose bytes changed at the same size, e.g. bit rot; such a block is loaded as KV silently.

New tier option checksum_blocks (default false), using the xattr approach preferred in the #54363 discussion:

  • Store: the block's CRC32C is set as xattr user.vllm.kv_crc32c on the temp file, before the rename.

  • Load: verified after the read. A checksum verification failure removes the file and fails the load with EBADMSG through the existing num_succeeded partial-keep path, with checksum_failed set on the exception. The tier reports it as JobResult.checksum_failed, and TieringMetricsTracker counts it in vllm:kv_offload_tiering_checksum_failures under the indexed tier label (e.g. 1:fs). An EBADMSG from the filesystem itself (as ext4/xfs report metadata checksum errors) is not flagged, so it fails the load without removing the file or being counted.

  • Backward compatible: a block with no recorded checksum still loads, so the option can be enabled on an existing cache. For the same reason FileMapper.fields is not bumped, unlike the RFC text: the file contents do not change, so there is no cold start.

  • xattrs unavailable at root_dir: the tier fails at startup (see the design decision below).

  • Implemented in both csrc/fs_io.cpp (SSE4.2, table fallback) and the Python fallback in fs/io.py. With the option off, no CRC or xattr call is made and the metric is not registered.

batch_store_block / batch_load_block in fs_io_C gain a trailing checksum argument, so the extension must be rebuilt with this change.

Limitation: hardware CRC32C is x86-64 only. Other architectures, including ARM, use the table implementation (same value, slower); ARM hardware CRC can follow separately.

No other open PR references #54363, and searches for checksum kv offload, crc32c and xattr find none in this area.

Design decision to confirm

#54363 did not settle what checksum mode should do when xattrs are unavailable: fail, or disable itself with a warning. I chose to fail at startup, because a silent fallback hides that blocks are no longer verified. Is that acceptable?

Test Plan

.venv/bin/python -m pytest tests/v1/kv_offload/ -q
pre-commit run --from-ref upstream/main --to-ref HEAD

New tests in test_fs_tier.py, on both I/O paths where applicable: same recorded value on both paths, C and Python CRC32C agree for 1-12 bytes (C 8-byte words and byte tail), corrupt data / a short, long or wrong-value record, legacy block still loads, xattr set before the rename, startup failure without xattrs, metric registered only when enabled, a filesystem EBADMSG is not counted as a checksum failure. In test_metrics.py, the tracker counts a checksum failure under the indexed tier label.

Test Result

  • tests/v1/kv_offload/: 805 passed, 2 skipped (test_fs_tier.py: 74 passed, 29 new).
  • pre-commit: passed.

Cost when enabled

i7-14700F, ext4 on NVMe, O_DIRECT, fs_io_C, one thread, median per block:

Block Store off → on (µs) Load off → on (µs) Device writes/block off / on Device reads/block off / on
256 KiB 84.2 → 107.3 180.1 → 214.2 2.10 / 2.12 2.00 / 2.00
1 MiB 207.2 → 300.4 275.8 → 393.5 8.16 / 8.13 8.00 / 8.00
4 MiB 679.3 → 1043.3 709.8 → 1090.1 32.35 / 32.39 32.00 / 32.00

Device I/O counts are effectively unchanged in this ext4/NVMe setup (the 4-byte value lives in the inode) and the xattr calls take under 1 µs. The added latency is the CRC32C pass, about 12 GB/s here.

Notes

  • No behavior change by default: checksum_blocks is off unless explicitly enabled, so no model evals were run. When enabled, a checksum failure turns a corrupted cache hit into a miss and recompute instead of returning the corrupted KV block.

  • AI assistance (Claude) was used. I have reviewed every changed line and ran the tests above.


Essential Elements of an Effective PR Description Checklist
  • The purpose of the PR, such as "Fix some issue (link existing issues this PR will resolve)".
  • The test plan, such as providing test command.
  • The test results, such as pasting the results comparison before and after, or e2e results
  • (Optional) The necessary documentation update, such as updating supported_models.md and examples for a new model.

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Claude Code Review

This pull request is from a fork — automated review is disabled. A repository maintainer can comment @claude review to run a one-time review.

@mergify

mergify Bot commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

Documentation preview: https://vllm--57912.org.readthedocs.build/en/57912/

@mergify mergify Bot added the documentation Improvements or additions to documentation label Sep 21, 2026
Comment thread vllm/v1/kv_offload/tiering/fs/manager.py Outdated
@CHIPMUNK-T0T

Copy link
Copy Markdown
Contributor Author

@varun-sundar-rabindranath On the IOPS question from #54363: I measured the xattr implementation in this PR on ext4 / NVMe (i7-14700F, O_DIRECT, fs_io_C).

No additional device I/O per block on this setup. Reads/block were identical with checksums off/on, and writes/block differed only by noise (e.g. 32.35 vs 32.39 at 4 MiB), measured from /sys/class/block/<dev>/stat. filefrag -x reports the 4-byte xattr as inline in the inode. fsetxattr / fgetxattr measured 0.9 / 0.4 µs.

At the tier's default concurrency (16 threads), 4 GiB per pass, median of 5:

Block Store off → on (GB/s) Load off → on (GB/s)
256 KiB 5.90 → 5.75 6.43 → 6.46
1 MiB 5.89 → 5.90 7.18 → 7.19
4 MiB 5.94 → 6.06 7.17 → 7.16
16 MiB 6.10 → 6.10 7.10 → 7.09

So on this local ext4 setup, the xattr does not add observable device I/O and checksum mode does not materially reduce throughput at default concurrency. A filesystem that stores xattrs externally, or a network filesystem, may behave differently.

@varun-sundar-rabindranath

Copy link
Copy Markdown
Contributor

Hi @CHIPMUNK-T0T , thank you for running the performance numbers. QQ :
about

Block 	Store off → on (GB/s) 	Load off → on (GB/s)
256 KiB 	5.90 → 5.75 	6.43 → 6.46
1 MiB 	5.89 → 5.90 	7.18 → 7.19
4 MiB 	5.94 → 6.06 	7.17 → 7.16
16 MiB 	6.10 → 6.10 	7.10 → 7.09

Can you also share the time it takes for the thread-pool to finish the workload with checksum on/off.

Given that the CRC calculation expands the store / load time by 65%, is this reflected in the thread-pool e2e time ? Just want to understand where this time is absorbed.

Also, it'd be nice to add a file-size vs crc32-comupation-time table.

Thanks 🙌

@CHIPMUNK-T0T

Copy link
Copy Markdown
Contributor Author

@varun-sundar-rabindranath

I re-measured through FileSystemTierManager itself using the default pool (16 read / 16 write threads). Each benchmark job contains 16 blocks, N jobs are submitted at once, and I measured wall-clock time until the last job completed. The setup is otherwise the same as before: i7-14700F, ext4 / NVMe, O_DIRECT, fs_io_C, with checksum off/on runs interleaved.

For the same 4 GiB workload as my earlier table (4 MiB blocks × 64 jobs), the pool finished in 716.5 → 717.2 ms for store and 603.5 → 607.1 ms for load. With several jobs in flight, the pool e2e time does not show a consistent regression from checksumming, while process CPU time increases by about 1.4–3.0x. This is consistent with I/O becoming the limiting factor at higher concurrency, with CRC work overlapping with I/O across pool threads.

With only one job in flight, that overlap is not available at the job level: one pool task processes its blocks serially, so the checksum cost is directly visible in job latency. At 1 MiB and above, I measured roughly +34–89%.

So the checksum cost is reflected in single-job latency, but not in pool e2e time when several jobs are in flight; there it is absorbed as additional CPU time on the pool threads.

Thread-pool e2e time, checksum off → on
Block Concurrent jobs Store e2e (ms) Load e2e (ms) Process CPU time (user+sys), store+load (ms)
256 KiB 1 2.0 → 2.5 3.5 → 4.1 2 → 3
256 KiB 4 3.2 → 3.2 4.4 → 4.6 7 → 11
256 KiB 16 10.9 → 10.9 10.8 → 10.3 25 → 41
256 KiB 64 47.1 → 46.9 39.0 → 38.6 103 → 143
1 MiB 1 4.1 → 6.0 6.4 → 8.6 4 → 8
1 MiB 4 10.8 → 11.0 11.3 → 13.3 20 → 42
1 MiB 16 45.4 → 47.3 40.6 → 40.8 139 → 302
1 MiB 64 176.9 → 180.4 154.4 → 154.4 752 → 1257
4 MiB 1 11.6 → 19.7 13.7 → 26.0 7 → 28
4 MiB 4 46.6 → 44.7 41.3 → 42.1 62 → 134
4 MiB 16 174.3 → 175.3 151.3 → 152.1 309 → 888
4 MiB 64 716.5 → 717.2 603.5 → 607.1 1914 → 3816
16 MiB 1 83.1 → 128.7 42.6 → 66.4 47 → 100
16 MiB 4 269.5 → 251.4 150.9 → 163.8 203 → 489
16 MiB 16 882.3 → 862.6 604.4 → 605.1 937 → 2823
16 MiB 64 4917.5 → 4905.2 2400.3 → 2412.0 6456 → 12253

Medians of 10 runs. The short one-job measurements have relatively high run-to-run variance, and 16 MiB store times also vary with the SSD's write state. Device reads/writes per block were effectively unchanged between checksum off/on in every row.

Block size vs CRC32C computation time
Block (= file) size SSE4.2 Table fallback
64 KiB 5.4 µs 101 µs
256 KiB 19.8 µs 403 µs
1 MiB 79 µs 1.6 ms
4 MiB 319 µs 6.4 ms
16 MiB 1.27 ms 25.8 ms
Throughput 13.2 GB/s 0.65 GB/s

Both columns run the CRC32C code from fs_io.cpp on the same P-core, over data not resident in the CPU cache. The C table fallback is used on non-x86 builds, or on x86 without SSE4.2.

Thanks!

@CHIPMUNK-T0T
CHIPMUNK-T0T force-pushed the feat/kv-offload-fs-block-checksum branch from 0132ee7 to 000f561 Compare September 23, 2026 08:02
Comment thread csrc/fs_io.cpp
Comment thread tests/v1/kv_offload/tiering/test_fs_tier.py
Comment thread tests/v1/kv_offload/tiering/test_fs_tier.py
Comment thread vllm/v1/kv_offload/tiering/fs/manager.py
Comment thread vllm/v1/kv_offload/tiering/fs/manager.py
@varun-sundar-rabindranath

Copy link
Copy Markdown
Contributor

Thanks @CHIPMUNK-T0T . I made a pass the PR generally looks good to me. Left a few high level comments.

cc @zdtsw @Srinivasoo7 @dannyharnik PTAL 🙌

@Srinivasoo7

Copy link
Copy Markdown
Contributor

Thanks @CHIPMUNK-T0T . I made a pass the PR generally looks good to me. Left a few high level comments.

cc @zdtsw @Srinivasoo7 @dannyharnik PTAL 🙌

Hi @CHIPMUNK-T0T thanks for the PR.

LGTM!
From the documentation standpoint, we might have to capture the behavior for existing blocks without checksum metadata.

cc @varun-sundar-rabindranath @orozery

@mergify

mergify Bot commented Oct 15, 2026

Copy link
Copy Markdown
Contributor

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @CHIPMUNK-T0T.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify Bot added the needs-rebase label Oct 15, 2026
@CHIPMUNK-T0T
CHIPMUNK-T0T force-pushed the feat/kv-offload-fs-block-checksum branch from aed1b73 to 1306e76 Compare October 15, 2026 14:09
@mergify mergify Bot removed the needs-rebase label Oct 15, 2026
@CHIPMUNK-T0T

CHIPMUNK-T0T commented Oct 15, 2026 •

Copy link
Copy Markdown
Contributor Author

@Srinivasoo7
Thanks, added this to the docs. Blocks already in the cache when the option is enabled are not rewritten, so only blocks stored afterwards are verified.
1306e76

Signed-off-by: Chipmunk <101038159+CHIPMUNK-T0T@users.noreply.github.com>
Signed-off-by: Chipmunk <101038159+CHIPMUNK-T0T@users.noreply.github.com>
… them

Signed-off-by: Chipmunk <101038159+CHIPMUNK-T0T@users.noreply.github.com>
Signed-off-by: Chipmunk <101038159+CHIPMUNK-T0T@users.noreply.github.com>
…troactively

Signed-off-by: Chipmunk <101038159+CHIPMUNK-T0T@users.noreply.github.com>
@mergify

mergify Bot commented Oct 29, 2026

Copy link
Copy Markdown
Contributor

This pull request has merge conflicts that must be resolved before it can be
merged. Please rebase the PR, @CHIPMUNK-T0T.

https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/working-with-forks/syncing-a-fork

@mergify mergify Bot added needs-rebase nixl NIXL KV connector / multi-tiered KV offloading (KVCR) labels Oct 29, 2026

This branch has not been deployed

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

Labels

documentation Improvements or additions to documentation needs-rebase nixl NIXL KV connector / multi-tiered KV offloading (KVCR)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants