[Bugfix] Fail fast with a clear error when CPU offload region exceeds available space

[Alex-ai-future]

[Bugfix] Fail fast when CPU KV offload exceeds /dev/shm

Summary

Fixes #46949.

Oversized CPU KV offload regions currently fail in
mmap.madvise(MADV_POPULATE_WRITE) with an opaque
OSError: [Errno 14] Bad address. This PR checks /dev/shm capacity first
and reports the requested and available sizes with actionable configuration
guidance.

After #50094, the default CPUOffloadingSpec also uses
SharedOffloadRegion on CUDA/ROCm, so both configuration paths are named:
--kv-offloading-size and cpu_bytes_to_use in
kv_connector_extra_config.

Changes

• Run the existing check_shm_free_space helper only after winning O_EXCL,
  so joiners do not perform a redundant capacity check.
• Unlink and close the creator fd if validation or ftruncate fails.
• Preserve the helper's actionable insufficient-space error unchanged.
• Add focused tests for clear errors and creator cleanup.

Duplicate check

#47073 and #46959 address the same issue but remain open with different,
stalled implementations. This PR is based on current main, reuses vLLM's
existing check_shm_free_space helper, and follows @orozery's requested
creator-path placement.

Tests

.venv/bin/python -m pytest \
  tests/v1/kv_offload/cpu/test_shared_offload_region.py -v \
  -k 'wait_for_file_size or insufficient_space or ftruncate_failure'
.venv/bin/ruff check \
  vllm/v1/kv_offload/cpu/shared_offload_region.py \
  tests/v1/kv_offload/cpu/test_shared_offload_region.py
.venv/bin/ruff format --check \
  vllm/v1/kv_offload/cpu/shared_offload_region.py \
  tests/v1/kv_offload/cpu/test_shared_offload_region.py
pre-commit run mypy-3.12 --files \
  vllm/v1/kv_offload/cpu/shared_offload_region.py \
  tests/v1/kv_offload/cpu/test_shared_offload_region.py \
  --hook-stage manual

CUDA host validation

The failure path was also verified on a CUDA host by requesting one page more
than the available /dev/shm capacity:

.venv/bin/python - <<'PY'
import mmap
import os
import shutil
import uuid

from vllm.v1.kv_offload.cpu.shared_offload_region import SharedOffloadRegion

page_size = mmap.PAGESIZE
free_bytes = shutil.disk_usage("/dev/shm").free
required_bytes = ((free_bytes // page_size) + 1) * page_size
engine_id = f"shm_capacity_test_{uuid.uuid4().hex}"
mmap_path = f"/dev/shm/vllm_offload_{engine_id}.mmap"

try:
    SharedOffloadRegion(
        engine_id=engine_id,
        num_blocks=1,
        rank=0,
        kv_bytes_per_block=required_bytes,
        cpu_page_size=page_size,
    )
except RuntimeError as exc:
    message = str(exc)
    assert "Insufficient space in /dev/shm" in message
    assert not os.path.exists(mmap_path)
    print("PASS: capacity error and cleanup verified")
    print(message)
else:
    raise AssertionError("Expected insufficient /dev/shm capacity error")
PY

PASS: capacity error and cleanup verified
Insufficient space in /dev/shm: 31744 MiB required, 31744 MiB free. Increase
/dev/shm (e.g. --shm-size or --ipc=host).

Model evaluation is not applicable; this changes startup validation and
cleanup only.

AI assistance

AI assistance was used for implementation, tests, and this description. The
submitting human reviewed every changed line and ran the tests above.

[claude]

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.

[Alex-ai-future]

@orozery PTAL

[orozery]

Thanks @Alex-ai-future !
Please disregard my previous posts (accidently posted by my Claude agent).

[orozery]

I don't understand why we need this check

[Alex-ai-future]

This judgment is for semantic consistency. Check should be a behavior on the creator path. Furthermore, I think it can be placed if check is after os.open(self.mmap_path, os.O_CREAT | os.O_EXCL | os.O_RDWR, 0o600)
do you agree that?

[orozery]

So something like that, right?

try:
    self.fd = os.open(self.mmap_path, os.O_CREAT | os.O_EXCL | os.O_RDWR, 0o600)
    try:
        check_shm_free_space(self.total_size_bytes)
        os.ftruncate(self.fd, self.total_size_bytes)
    except (RuntimeError, OSError) as e:
        os.unlink(self.mmap_path)
        raise
    self._creator = True
except FileExistsError:
    ...

This looks good to me.

[orozery]

Claude:
Need a try/except OSError around ftruncate with os.unlink(self.mmap_path) before re-raising — joiners then get immediate FileNotFoundError instead of spinning.

[Alex-ai-future]

Is it possible to add mmap.mmap .mmap_obj.madvise torch.frombuffer to be added to error capture?
like this

try:
      self.fd = os.open(...)
      os.ftruncate(...)
      self._creator = True
      logger.info(...)

      self.mmap_obj = mmap.mmap(...)                  
      self.mmap_obj.madvise(...)                     
      self._base = torch.frombuffer(...)              

  except FileExistsError:                            # joiner path
      self.fd = os.open(path, O_RDWR)
      _wait_for_file_size(...)
      logger.info(...)

  except OSError:                                     
      if self.fd is not None:
          os.close(self.fd)
      os.unlink(self.mmap_path)                       
      raise

[orozery]

Claude's answer:

No — keep the except OSError tightly around ftruncate only. Reasons:

1. The unlink is specifically to protect joiners from hanging. Joiners hang because they see a file that exists but never reaches the expected size. That can only happen if ftruncate fails (file stays at 0 bytes) or succeeds but the creator dies before MADV_POPULATE_WRITE completes. But MADV_POPULATE_WRITE failure is a different class of error — the file is the right size on disk, just not faulted in. Joiners won't hang because _wait_for_file_size checks fstat.st_size, not whether pages are populated.
2. mmap.mmap failure after a successful ftruncate doesn't strand joiners either — the file is already the right size, joiners can open and mmap it fine. Unlinking it here would actually break joiners that already opened it.
3. Mixing the joiner path (FileExistsError) and the error-cleanup path (OSError) in the same try block is fragile. OSError is a parent of FileExistsError — if the ordering of except clauses is wrong, or if future code adds an os.open call that raises OSError for a different reason, you'd accidentally catch the wrong thing. Keeping them in separate, narrow try blocks makes the intent explicit.

[orozery]

Let's remove this test

[orozery]

Claude:
just monkeypatch check_shm_free_space to raise RuntimeError, assert the constructor propagates it. One mock, same coverage of the actual contract.

[Alex-ai-future]

@orozery It looks a little more complicated.

[orozery]

check_shm_free_space already gives an actionable message.
I think we should drop the exception mutation.

[orozery]

Let's remove this test

[orozery]

/ci run

[github-actions]

✅ Triggered Buildkite CI #82506 for commit 67659870163d.

[orozery]

Thanks @Alex-ai-future !