libbdm: handle failed speculative read-ahead correctly

iop/fs/libbdm/src/bd_cache.c

@@ -14,6 +14,7 @@ struct bd_cache
     struct block_device *bd;
     int weight[BLOCK_COUNT];
     u64 sector[BLOCK_COUNT];
+    u16 count[BLOCK_COUNT];
     u8 cache[BLOCK_COUNT][SECTORS_PER_BLOCK*512];
 #ifdef DEBUG
     u32 sectors_read;
@@ -23,18 +24,18 @@ struct bd_cache
 };
 
 /* cache overlaps with requested area ? */
-static int _overlaps(u64 csector, u64 sector, u16 count)
+static int _overlaps(u64 csector, u16 ccount, u64 sector, u16 count)
 {
-    if ((sector < (csector + SECTORS_PER_BLOCK)) && ((sector + count) > csector))
+    if (ccount != 0 && (sector < (csector + ccount)) && ((sector + count) > csector))
         return 1;
     else
         return 0;
 }
 
 /* cache contains requested area ? */
-static int _contains(u64 csector, u64 sector, u16 count)
+static int _contains(u64 csector, u16 ccount, u64 sector, u16 count)
 {
-    if ((sector >= csector) && ((sector + count) <= (csector + SECTORS_PER_BLOCK)))
+    if (ccount != 0 && (sector >= csector) && ((sector + count) <= (csector + ccount)))
         return 1;
     else
         return 0;
@@ -45,9 +46,10 @@ static void _invalidate(struct bd_cache *c, u64 sector, u16 count)
     int blkidx;
 
     for (blkidx = 0; blkidx < BLOCK_COUNT; blkidx++) {
-        if (_overlaps(c->sector[blkidx], sector, count)) {
+        if (_overlaps(c->sector[blkidx], c->count[blkidx], sector, count)) {
             // Invalidate cache entry
             c->sector[blkidx] = 0xffffffffffffffff;
+            c->count[blkidx] = 0;
         }
     }
 }
@@ -71,7 +73,7 @@ static int _read(struct block_device *bd, u64 sector, void *buffer, u16 count)
     // Do a cached read
     int blkidx;
     for (blkidx = 0; blkidx < BLOCK_COUNT; blkidx++) {
-        if (_contains(c->sector[blkidx], sector, count)) {
+        if (_contains(c->sector[blkidx], c->count[blkidx], sector, count)) {
 #ifdef DEBUG
             c->sectors_cache += count;
             //M_DEBUG("- CACHE HIT[%d] [block %d] [devread %ds, hit-ratio %d%%]\n", sector, blkidx, c->sectors_dev, (c->sectors_cache * 100) / c->sectors_read);
@@ -113,9 +115,23 @@ static int _read(struct block_device *bd, u64 sector, void *buffer, u16 count)
     //M_DEBUG("- CACHE READ[%d] -> [block %d] [devread %ds, hit-ratio %d%%]\n", sector, blkidx_best, c->sectors_dev, (c->sectors_cache * 100) / c->sectors_read);
 #endif
 
-    // Fill the block
-    c->bd->read(c->bd, sector, c->cache[blkidx_best], SECTORS_PER_BLOCK);
+    // Fill the block with read-ahead. Some devices reject a speculative
+    // multi-sector read even though the requested sectors themselves are
+    // valid (for example when the read-ahead crosses the end of the media).
+    // Fall back to exactly the requested range in that case.
+    u16 fill_count = SECTORS_PER_BLOCK;
+    int result = c->bd->read(c->bd, sector, c->cache[blkidx_best], fill_count);
+    if (result != fill_count) {
+        fill_count = count;
+        result = c->bd->read(c->bd, sector, c->cache[blkidx_best], fill_count);
+        if (result != fill_count) {
+            c->sector[blkidx_best] = 0xffffffffffffffff;
+            c->count[blkidx_best] = 0;
+            return result < 0 ? result : -1;
+        }
+    }
     c->sector[blkidx_best] = sector;
+    c->count[blkidx_best] = fill_count;
 
     // Read from cache
     u64 offset = (sector - c->sector[blkidx_best]) * 512;
@@ -169,6 +185,7 @@ struct block_device *bd_cache_create(struct block_device *bd)
     for (blkidx = 0; blkidx < BLOCK_COUNT; blkidx++) {
         c->weight[blkidx] = 0;
         c->sector[blkidx] = 0xffffffffffffffff;
+        c->count[blkidx] = 0;
     }
 #ifdef DEBUG
     c->sectors_read = 0;

iop/fs/libbdm/tests/Makefile

@@ -0,0 +1,19 @@
+CC ?= cc
+CFLAGS ?= -std=c99 -Wall -Wextra -Werror
+
+TARGET = test_bd_cache
+SOURCE = test_bd_cache.c
+BD_CACHE_SOURCE = ../src/bd_cache.c
+
+.PHONY: all test clean
+
+all: $(TARGET)
+
+$(TARGET): $(SOURCE) $(BD_CACHE_SOURCE)
+	$(CC) $(CFLAGS) -Iinclude $(SOURCE) $(BD_CACHE_SOURCE) -o $@
+
+test: $(TARGET)
+	./$(TARGET)
+
+clean:
+	rm -f $(TARGET)

iop/fs/libbdm/tests/include/bd_cache.h

@@ -0,0 +1,30 @@
+#ifndef __BDM_CACHE_H__
+#define __BDM_CACHE_H__
+
+#include <stdint.h>
+
+typedef uint64_t u64;
+typedef uint16_t u16;
+typedef uint8_t u8;
+
+struct block_device
+{
+    void *priv;
+    char *name;
+    char *path;
+    unsigned int devNr;
+    unsigned int parNr;
+    unsigned int parId;
+    unsigned int sectorSize;
+    u64 sectorOffset;
+    u64 sectorCount;
+    int (*read)(struct block_device *, u64, void *, u16);
+    int (*write)(struct block_device *, u64, const void *, u16);
+    void (*flush)(struct block_device *);
+    int (*stop)(struct block_device *);
+};
+
+struct block_device *bd_cache_create(struct block_device *bd);
+void bd_cache_destroy(struct block_device *cbd);
+
+#endif

iop/fs/libbdm/tests/include/module_debug.h

@@ -0,0 +1,7 @@
+#ifndef _MODULE_DEBUG_H
+#define _MODULE_DEBUG_H
+
+#define DEBUG_U64_2XU32(value)
+#define M_DEBUG(...)
+
+#endif

iop/fs/libbdm/tests/include/sysmem.h

@@ -0,0 +1,10 @@
+#ifndef __SYSMEM_H__
+#define __SYSMEM_H__
+
+#include <stdlib.h>
+
+#define ALLOC_FIRST 0
+#define AllocSysMemory(mode, size, address) malloc(size)
+#define FreeSysMemory(ptr) free(ptr)
+
+#endif

iop/fs/libbdm/tests/test_bd_cache.c

@@ -0,0 +1,81 @@
+#include <assert.h>
+#include <stdio.h>
+#include <string.h>
+
+#include <bd_cache.h>
+
+static int calls;
+static int mode;
+
+static int read_device(struct block_device *bd, u64 sector, void *buffer, u16 count)
+{
+    unsigned int i;
+
+    (void)bd;
+    ++calls;
+
+    /* Poison failed transfers to catch accidentally caching incomplete data. */
+    memset(buffer, 0xee, count * 512);
+
+    if (mode == 1)
+        return -7;
+    if (mode == 2)
+        return count - 1;
+    if (sector + count > 100)
+        return -7;
+
+    for (i = 0; i < count; ++i)
+        memset((u8 *)buffer + i * 512, (u8)(sector + i), 512);
+
+    return count;
+}
+
+static int write_device(struct block_device *bd, u64 sector, const void *buffer, u16 count)
+{
+    (void)bd;
+    (void)sector;
+    (void)buffer;
+    return count;
+}
+
+int main(void)
+{
+    struct block_device raw = {
+        .sectorSize = 512,
+        .sectorCount = 100,
+        .read = read_device,
+        .write = write_device,
+    };
+    struct block_device *cache = bd_cache_create(&raw);
+    u8 data[4096];
+
+    assert(cache != NULL);
+
+    assert(cache->read(cache, 10, data, 1) == 1 && calls == 1);
+    assert(cache->read(cache, 11, data, 1) == 1 && calls == 1 && data[0] == 11);
+
+    /* Read-ahead past the end of media must fall back to the requested range. */
+    assert(cache->read(cache, 99, data, 1) == 1 && calls == 3 && data[0] == 99);
+    assert(cache->read(cache, 99, data, 1) == 1 && calls == 3);
+    assert(cache->read(cache, 100, data, 1) < 0 && calls == 5);
+
+    /* Failed and short fills must neither reach the caller nor become hits. */
+    mode = 1;
+    assert(cache->read(cache, 30, data, 1) == -7 && calls == 7);
+    mode = 2;
+    assert(cache->read(cache, 30, data, 1) < 0 && calls == 9);
+    mode = 0;
+    assert(cache->read(cache, 30, data, 1) == 1 && calls == 10 && data[0] == 30);
+
+    /* Writes invalidate overlapping cached data. */
+    assert(cache->write(cache, 30, data, 1) == 1);
+    assert(cache->read(cache, 30, data, 1) == 1 && calls == 11);
+
+    /* Full cache-block requests bypass the cache. */
+    assert(cache->read(cache, 40, data, 8) == 8 && calls == 12 && data[4095] == 47);
+
+    bd_cache_destroy(cache);
+
+    puts("PASS: hits, end-of-media fallback, errors, short fills, invalidation, direct reads");
+    return 0;
+}