Skip to content

libbdm: handle failed speculative read-ahead correctly - #935

Merged
uyjulian merged 1 commit into
ps2dev:masterfrom
fjtrujy:bdm-cache-read-ahead-fix
Oct 6, 2026
Merged

uyjulian merged 1 commit into
ps2dev:masterfrom
fjtrujy:bdm-cache-read-ahead-fix

Conversation

@fjtrujy

@fjtrujy fjtrujy commented Oct 6, 2026

Copy link
Copy Markdown
Member

Summary

Fix the libbdm block cache so a failed or short speculative read-ahead does not cause a valid read request to fail or leave incomplete data marked as a valid cache entry.

The BDM cache currently fills a cache block by requesting 8 sectors even when the caller requested fewer sectors. This read-ahead is an optimization and therefore should not change the result of the original read.

Some block devices can reject the larger speculative request even though the range originally requested by the caller is valid. A simple example is a read near the end of a device:

  • the caller requests 1 valid sector;
  • libbdm attempts to read 8 sectors to populate its cache;
  • the 8-sector request crosses the end of the device and fails;
  • the original 1-sector request could still have succeeded.

Previously, the cache did not validate the result of the read-ahead before treating the cache block as populated. This could make the operation appear to succeed using incomplete or invalid cache contents, and could also allow that data to be returned by subsequent cache hits.

Changes

  • Check the result of speculative cache fills.
  • If an 8-sector read-ahead fails or is short, retry using exactly the range requested by the caller.
  • Propagate the error if the requested range itself cannot be read.
  • Track the number of valid sectors in each cache entry instead of assuming that every entry always contains 8 valid sectors.
  • Invalidate an entry after a failed or short fill so incomplete data cannot become a later cache hit.
  • Update overlap/containment checks to use the actual valid sector count.
  • Add a host-side C regression test covering:
    • normal cache hits;
    • read-ahead crossing the end of the device;
    • failed reads;
    • short reads;
    • invalidation after writes;
    • recovery after a failed fill;
    • direct reads.

Why this belongs in PS2SDK

This is a correctness issue in libbdm itself rather than in any particular filesystem, USB driver, or application.

The cache performs the larger read internally as an optimization. Callers of the BDM API do not request that additional range and should not have to account for whether the cache decides to perform read-ahead.

Likewise, individual BDM block-device drivers should be allowed to reject a request that exceeds the valid range of their device. They should not need special handling for a cache implementation detail.

Handling the fallback in libbdm keeps the expected abstraction:

  1. the application or filesystem requests a valid range;
  2. libbdm may speculatively read additional sectors for caching;
  3. if that speculation is not possible, libbdm falls back to the actual
    requested range;
  4. only data confirmed to have been read successfully is exposed as cached
    data.

This makes the behavior consistent for every block device using the libbdm
cache.

Real-world example

This was encountered while using NJEMU on PS2 with its data stored on a USB mass-storage device.

NJEMU performs many small/random reads from large cache files. BDM's 8-sector (4 KiB) read-ahead is useful for this workload, so disabling the BDM cache or read-ahead globally would lose a useful optimization.

However, a small valid read near a device boundary can cause the speculative 8-sector fill to extend beyond the available sectors. The underlying block device is allowed to reject that larger request even though the application's
original read is valid.

With this change, the common case still gets the existing 4 KiB read-ahead, while boundary and error cases fall back to the exact requested range.

This is also applicable to other BDM-backed storage devices and applications;
NJEMU is only the workload that exposed the issue.

Testing

A host-side C regression test has been added which compiles the actual bd_cache.c against a small fault-injecting block-device implementation.

The test runs on the host machine, not on the PS2 IOP. This makes the cache logic deterministic and easy to exercise in CI without requiring PS2 hardware or PCSX2.

It verifies:

  • normal cache fills and hits;
  • fallback when read-ahead crosses the end of the media;
  • propagation of genuine read errors;
  • rejection and invalidation of short fills;
  • successful recovery after a failed fill;
  • write invalidation;
  • direct reads.

The test can be run with:

make -C iop/fs/libbdm/tests test

It requires only a standard C99 host compiler.

Retry the requested range when speculative read-ahead fails. Track valid sector counts and invalidate failed fills so incomplete data cannot be returned as a cache hit.

Add a host regression test covering read-ahead fallback, failed and short fills, cache hits, write invalidation and direct reads.

Replace BDM cache regression test with C
@uyjulian

uyjulian commented Oct 6, 2026

Copy link
Copy Markdown
Member

Changes lgtm

@uyjulian
uyjulian merged commit 18ffdef into ps2dev:master Oct 6, 2026
5 checks passed
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