From e539f7e0cdde403ddb61a12d7e38c725f82c156e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20DUCOM?= Date: Wed, 30 Sep 2026 23:14:15 +0200 Subject: [PATCH 01/18] test(blob): a thin cluster reads zeroes where no write landed (0044, tests only) Patch 0044 starts as tests only, and the unit-tests stage is expected to fail on blob_ut: - blob_thin_prov_new_cluster_reads_zeroes: every free cluster is filled with 0xAA, as a device whose unmap does not zero leaves them. A write of one io_unit goes into a new cluster, and the rest of the cluster must read zeroes. Upstream returns the 0xAA. - blob_thin_prov_full_cluster_write_is_not_cleared_first: a write that covers the whole cluster costs the payload and the metadata page(s), and nothing more. It passes before the fix too: it guards the fix's cost. - blob_thin_prov_rw, blob_thin_prov_write_count_io and blob_thin_prov_rle count the cleared cluster in their write accounting. Locally (debug, arm64): blob_ut fails 4 tests in each of its 5 suites. The fix follows in the next commit. --- ...blob-thin-cluster-cleared-before-use.patch | 207 ++++++++++++++++++ 1 file changed, 207 insertions(+) create mode 100644 patches/0044-blob-thin-cluster-cleared-before-use.patch diff --git a/patches/0044-blob-thin-cluster-cleared-before-use.patch b/patches/0044-blob-thin-cluster-cleared-before-use.patch new file mode 100644 index 0000000..d980cb0 --- /dev/null +++ b/patches/0044-blob-thin-cluster-cleared-before-use.patch @@ -0,0 +1,207 @@ +diff --git a/test/unit/lib/blob/blob.c/blob_ut.c b/test/unit/lib/blob/blob.c/blob_ut.c +index c168e08..3bb3f5c 100644 +--- a/test/unit/lib/blob/blob.c/blob_ut.c ++++ b/test/unit/lib/blob/blob.c/blob_ut.c +@@ -4628,8 +4628,9 @@ blob_thin_prov_rw(void) + CU_ASSERT(free_clusters - 1 == spdk_bs_free_cluster_count(bs)); + CU_ASSERT(spdk_blob_get_num_allocated_clusters(blob) == 1); + /* For thin-provisioned blob we need to write 20 io_units plus one page metadata and +- * read 0 bytes */ +- expected_bytes = 20 * io_unit_size + spdk_bs_get_page_size(bs); ++ * read 0 bytes. Each thread allocated a cluster and cleared it before inserting it, ++ * the one that lost the race included. */ ++ expected_bytes = 20 * io_unit_size + spdk_bs_get_page_size(bs) + 2 * spdk_bs_get_cluster_size(bs); + if (g_use_extent_table) { + /* Add one more page for EXTENT_PAGE write */ + expected_bytes += spdk_bs_get_page_size(bs); +@@ -4731,17 +4732,18 @@ blob_thin_prov_write_count_io(void) + CU_ASSERT(free_clusters - (2 * i + 1) == spdk_bs_free_cluster_count(bs)); + + CU_ASSERT(g_dev_read_bytes == read_bytes); ++ /* The write covers one io_unit of a new cluster: the cluster is cleared first. */ + if (!g_use_extent_table) { + /* For legacy metadata, we should have written the io_unit for + * the write I/O, plus the blob's primary metadata page + */ +- expected_bytes = io_unit_size + spdk_bs_get_page_size(bs); ++ expected_bytes = CLUSTER_SZ + io_unit_size + spdk_bs_get_page_size(bs); + } else { + /* For extent table metadata, we should have written the io_unit for + * the write I/O, plus 2 metadata pages - the extent page and the + * blob's primary metadata page + */ +- expected_bytes = io_unit_size + 2 * spdk_bs_get_page_size(bs); ++ expected_bytes = CLUSTER_SZ + io_unit_size + 2 * spdk_bs_get_page_size(bs); + } + CU_ASSERT((g_dev_write_bytes - write_bytes) == expected_bytes); + +@@ -4774,8 +4776,9 @@ blob_thin_prov_write_count_io(void) + /* + * For legacy metadata, we should have written the I/O and the primary metadata page. + * For extent table metadata, we should have written the I/O and the extent metadata page. ++ * Both clear the new cluster first. + */ +- expected_bytes = io_unit_size + spdk_bs_get_page_size(bs); ++ expected_bytes = CLUSTER_SZ + io_unit_size + spdk_bs_get_page_size(bs); + CU_ASSERT((g_dev_write_bytes - write_bytes) == expected_bytes); + + /* Send unmap aligned to the whole cluster - should free it up */ +@@ -4809,6 +4812,138 @@ blob_thin_prov_write_count_io(void) + g_bs = NULL; + } + ++/* ++ * Fills every free cluster of the device with a pattern: what a device whose unmap does not ++ * zero keeps in the clusters of blobs deleted earlier. ++ */ ++static void ++ut_fill_free_clusters(struct spdk_blob_store *bs, uint8_t pattern) ++{ ++ uint32_t i; ++ ++ for (i = 0; i < bs->total_clusters; i++) { ++ if (!spdk_bit_pool_is_allocated(bs->used_clusters, i)) { ++ memset(&g_dev_buffer[bs_cluster_to_lba(bs, i) * bs->dev->blocklen], pattern, ++ bs->cluster_sz); ++ } ++ } ++} ++ ++/* ++ * A cluster allocated by a write that covers part of it is cleared first: the rest of it ++ * reads zeroes, not what the device held there, which is another blob's data where the ++ * device's unmap does not zero (and on each leg of a mirror, something different). ++ */ ++static void ++blob_thin_prov_new_cluster_reads_zeroes(void) ++{ ++ struct spdk_blob_store *bs = g_bs; ++ struct spdk_blob *blob; ++ struct spdk_io_channel *channel; ++ struct spdk_blob_opts opts; ++ uint64_t io_unit_size = spdk_bs_get_io_unit_size(bs); ++ uint64_t cluster_size = spdk_bs_get_cluster_size(bs); ++ uint64_t io_units_per_cluster = cluster_size / io_unit_size; ++ uint8_t payload_write[BLOCKLEN]; ++ uint8_t *payload_read; ++ ++ channel = spdk_bs_alloc_io_channel(bs); ++ SPDK_CU_ASSERT_FATAL(channel != NULL); ++ ++ ut_fill_free_clusters(bs, 0xAA); ++ ++ ut_spdk_blob_opts_init(&opts); ++ opts.thin_provision = true; ++ opts.num_clusters = 1; ++ blob = ut_blob_create_and_open(bs, &opts); ++ CU_ASSERT(spdk_blob_get_num_allocated_clusters(blob) == 0); ++ ++ /* One io_unit at the start of the cluster: the rest of it is never written. */ ++ memset(payload_write, 0xE5, sizeof(payload_write)); ++ spdk_blob_io_write(blob, channel, payload_write, 0, 1, blob_op_complete, NULL); ++ poll_threads(); ++ CU_ASSERT(g_bserrno == 0); ++ CU_ASSERT(spdk_blob_get_num_allocated_clusters(blob) == 1); ++ ++ payload_read = calloc(1, cluster_size); ++ SPDK_CU_ASSERT_FATAL(payload_read != NULL); ++ spdk_blob_io_read(blob, channel, payload_read, 0, io_units_per_cluster, blob_op_complete, NULL); ++ poll_threads(); ++ CU_ASSERT(g_bserrno == 0); ++ CU_ASSERT(memcmp(payload_read, payload_write, io_unit_size) == 0); ++ CU_ASSERT(spdk_mem_all_zero(payload_read + io_unit_size, cluster_size - io_unit_size)); ++ ++ free(payload_read); ++ ut_blob_close_and_delete(bs, blob); ++ spdk_bs_free_io_channel(channel); ++ poll_threads(); ++ g_blob = NULL; ++ g_blobid = 0; ++} ++ ++/* ++ * A write that covers the whole new cluster leaves nothing of the device's old content, so the ++ * cluster is not cleared first: a rebuild or a copy writes whole clusters, and clearing each of ++ * them would double what it writes. ++ */ ++static void ++blob_thin_prov_full_cluster_write_is_not_cleared_first(void) ++{ ++ struct spdk_blob_store *bs = g_bs; ++ struct spdk_blob *blob; ++ struct spdk_io_channel *channel; ++ struct spdk_blob_opts opts; ++ uint64_t io_unit_size = spdk_bs_get_io_unit_size(bs); ++ uint64_t cluster_size = spdk_bs_get_cluster_size(bs); ++ uint64_t io_units_per_cluster = cluster_size / io_unit_size; ++ uint64_t write_bytes; ++ uint64_t expected_bytes; ++ uint8_t *payload_write; ++ uint8_t *payload_read; ++ ++ channel = spdk_bs_alloc_io_channel(bs); ++ SPDK_CU_ASSERT_FATAL(channel != NULL); ++ ++ ut_fill_free_clusters(bs, 0xAA); ++ ++ ut_spdk_blob_opts_init(&opts); ++ opts.thin_provision = true; ++ opts.num_clusters = 1; ++ blob = ut_blob_create_and_open(bs, &opts); ++ ++ payload_write = malloc(cluster_size); ++ payload_read = calloc(1, cluster_size); ++ SPDK_CU_ASSERT_FATAL(payload_write != NULL && payload_read != NULL); ++ memset(payload_write, 0xE5, cluster_size); ++ ++ write_bytes = g_dev_write_bytes; ++ spdk_blob_io_write(blob, channel, payload_write, 0, io_units_per_cluster, blob_op_complete, NULL); ++ poll_threads(); ++ CU_ASSERT(g_bserrno == 0); ++ CU_ASSERT(spdk_blob_get_num_allocated_clusters(blob) == 1); ++ ++ /* The payload and the blob's metadata page; no cleared cluster before them. */ ++ expected_bytes = cluster_size + spdk_bs_get_page_size(bs); ++ if (g_use_extent_table) { ++ /* Add one more page for EXTENT_PAGE write */ ++ expected_bytes += spdk_bs_get_page_size(bs); ++ } ++ CU_ASSERT(g_dev_write_bytes - write_bytes == expected_bytes); ++ ++ spdk_blob_io_read(blob, channel, payload_read, 0, io_units_per_cluster, blob_op_complete, NULL); ++ poll_threads(); ++ CU_ASSERT(g_bserrno == 0); ++ CU_ASSERT(memcmp(payload_read, payload_write, cluster_size) == 0); ++ ++ free(payload_write); ++ free(payload_read); ++ ut_blob_close_and_delete(bs, blob); ++ spdk_bs_free_io_channel(channel); ++ poll_threads(); ++ g_blob = NULL; ++ g_blobid = 0; ++} ++ + static void + blob_thin_prov_unmap_cluster(void) + { +@@ -5124,8 +5259,8 @@ blob_thin_prov_rle(void) + CU_ASSERT(g_bserrno == 0); + CU_ASSERT(free_clusters - 1 == spdk_bs_free_cluster_count(bs)); + /* For thin-provisioned blob we need to write 10 pages plus one page metadata and +- * read 0 bytes */ +- expected_bytes = 10 * io_unit_size + spdk_bs_get_page_size(bs); ++ * read 0 bytes, and clear the new cluster before the write lands in it. */ ++ expected_bytes = 10 * io_unit_size + spdk_bs_get_page_size(bs) + spdk_bs_get_cluster_size(bs); + if (g_use_extent_table) { + /* Add one more page for EXTENT_PAGE write */ + expected_bytes += spdk_bs_get_page_size(bs); +@@ -10364,6 +10499,8 @@ main(int argc, char **argv) + CU_ADD_TEST(suite_bs, blob_insert_cluster_msg_test); + CU_ADD_TEST(suite_bs, blob_thin_prov_rw); + CU_ADD_TEST(suite, blob_thin_prov_write_count_io); ++ CU_ADD_TEST(suite_bs, blob_thin_prov_new_cluster_reads_zeroes); ++ CU_ADD_TEST(suite_bs, blob_thin_prov_full_cluster_write_is_not_cleared_first); + CU_ADD_TEST(suite, blob_thin_prov_unmap_cluster); + CU_ADD_TEST(suite_bs, blob_thin_prov_rle); + CU_ADD_TEST(suite_bs, blob_thin_prov_rw_iov); From e94d8627ea70237be0e2a22e6f5307f283cfa8c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20DUCOM?= Date: Wed, 30 Sep 2026 23:15:21 +0200 Subject: [PATCH 02/18] fix(blob): a thin cluster is cleared before use (0044) A cluster that a write allocates in a blob backed by zeroes (a thin blob with no parent, or a clone over a range its snapshot never allocated) was inserted as the device left it. The write filled its own range, and the rest of the cluster returned what was stored there before. The free path counts on unmap to leave zeroes, which a SATA SSD does not guarantee and a bdev without unmap support never provides. When the new cluster is not copied from a parent, bs_allocate_and_copy_cluster now clears it with write_zeroes before inserting it. A write (or write_zeroes) that covers the whole cluster skips the clear, so a rebuild or a copy does not write twice. A clear that fails returns the cluster to the pool. The inflate path goes through the same branch. Locally (debug, arm64): blob_ut passes, 495 tests. --- ...blob-thin-cluster-cleared-before-use.patch | 66 +++++++++++++++++++ patches/README.md | 3 +- 2 files changed, 68 insertions(+), 1 deletion(-) diff --git a/patches/0044-blob-thin-cluster-cleared-before-use.patch b/patches/0044-blob-thin-cluster-cleared-before-use.patch index d980cb0..d8e702b 100644 --- a/patches/0044-blob-thin-cluster-cleared-before-use.patch +++ b/patches/0044-blob-thin-cluster-cleared-before-use.patch @@ -1,3 +1,69 @@ +diff --git a/lib/blob/blobstore.c b/lib/blob/blobstore.c +index f774f99..4d656c9 100644 +--- a/lib/blob/blobstore.c ++++ b/lib/blob/blobstore.c +@@ -2827,6 +2827,40 @@ blob_copy(struct spdk_blob_copy_cluster_ctx *ctx, spdk_bs_user_op_t *op, uint64_ + blob_write_copy_cpl, ctx); + } + ++/* ++ * The write that triggered the allocation overwrites the whole cluster: no byte of what the ++ * device held there survives it, so clearing the cluster first would write it twice (a rebuild ++ * or a copy writes whole clusters). ++ */ ++static bool ++blob_op_covers_cluster(struct spdk_blob *blob, spdk_bs_user_op_t *op, uint64_t cluster_start_io_unit) ++{ ++ struct spdk_bs_user_op_args *args = &((struct spdk_bs_request_set *)op)->u.user_op; ++ ++ if (args->type != SPDK_BLOB_WRITE && args->type != SPDK_BLOB_WRITEV && ++ args->type != SPDK_BLOB_WRITE_ZEROES) { ++ return false; ++ } ++ ++ return args->offset <= cluster_start_io_unit && ++ args->offset + args->length >= cluster_start_io_unit + bs_io_units_per_cluster(blob); ++} ++ ++static void ++blob_zero_fill_cpl(spdk_bs_sequence_t *seq, void *cb_arg, int bserrno) ++{ ++ struct spdk_blob_copy_cluster_ctx *ctx = cb_arg; ++ ++ if (bserrno) { ++ /* The cluster was never handed to the blob: it goes back to the pool. */ ++ blob_insert_cluster_revert(ctx); ++ bs_sequence_finish(seq, bserrno); ++ return; ++ } ++ ++ blob_write_copy_cpl(seq, cb_arg, 0); ++} ++ + static void + bs_allocate_and_copy_cluster(struct spdk_blob *blob, + struct spdk_io_channel *_ch, +@@ -2937,9 +2971,19 @@ bs_allocate_and_copy_cluster(struct spdk_blob *blob, + blob_write_copy, ctx); + } + +- } else { ++ } else if (blob_op_covers_cluster(blob, op, cluster_start_io_unit)) { + blob_insert_cluster_on_md_thread(ctx->blob, cluster_number, ctx->new_cluster, + ctx->new_extent_page, ctx->new_cluster_page, blob_insert_cluster_cpl, ctx); ++ } else { ++ /* ++ * A cluster whose backing reads as zeroes is handed out as the device left it: the part ++ * the triggering write does not cover would read whatever an earlier blob wrote there, ++ * wherever the device's unmap does not zero (the free path counts on it to). That leaks ++ * one blob's data into another, and makes the legs of a mirror differ in bytes nobody ++ * wrote. The cluster is cleared before the blob sees it. ++ */ ++ bs_sequence_write_zeroes_dev(ctx->seq, bs_cluster_to_lba(blob->bs, ctx->new_cluster), ++ bs_cluster_to_lba(blob->bs, 1), blob_zero_fill_cpl, ctx); + } + } + diff --git a/test/unit/lib/blob/blob.c/blob_ut.c b/test/unit/lib/blob/blob.c/blob_ut.c index c168e08..3bb3f5c 100644 --- a/test/unit/lib/blob/blob.c/blob_ut.c diff --git a/patches/README.md b/patches/README.md index eb82251..1e590d8 100644 --- a/patches/README.md +++ b/patches/README.md @@ -4,7 +4,7 @@ Out-of-tree patches applied on top of upstream SPDK during the container build ( ## Application order -Patches are applied in **lexicographic order of filename** (`0001` … `0041`) — the Dockerfile globs `patches/*.patch` and `git apply`s each. The numeric prefix IS the contract; do not rely on any other ordering. Order matters: +Patches are applied in **lexicographic order of filename** (`0001` … `0044`) — the Dockerfile globs `patches/*.patch` and `git apply`s each. The numeric prefix IS the contract; do not rely on any other ordering. Order matters: | # | Patch | Touches | Depends on | |--:|:------|:--------|:-----------| @@ -49,6 +49,7 @@ Patches are applied in **lexicographic order of filename** (`0001` … `0041`) | 0039 | an armed delayed reconnect owns the continuation — the path returns instead of re-driving a failed disconnect, ending a mutual recursion that overflows the reactor stack | module/bdev/nvme (bdev_nvme.c) | 0032 | | 0040 | the delete-stop only MARKS — 0035's stop drove the window machinery from a foreign thread, unquiescing under in-flight requests; it now sets STOPPING with `-ECANCELED` and each resting state concludes | module/bdev/raid (bdev_raid.c) | 0035, 0039 | | 0041 | lvol → tier band usage provider — when an lvolstore loads on a `bdev_tier` composite the lvol layer registers a per-band fill-accounting provider, gated on the tier grain being a whole multiple of the lvolstore cluster (else no provider, honest unknown), and **unregisters it at teardown INITIATION** (before `spdk_lvs_unload`/`destroy`, whose synchronous store free would otherwise race a `get_bands` poll — UAF); `bdev_tier_get_bands` then fills `used_blocks` by counting allocated clusters of the blobstore's `used_clusters` pool in the band's LBA range (`spdk_bs_count_allocated_clusters_in_lba_range`, new in lib/blob — exclusive cluster-aligned bounds, one masked popcount per 64 clusters via `spdk_bit_pool_count_allocated_in_range` / `spdk_bit_array_count_set_in_range`, new in lib/util) | lib/blob, lib/util (ranged popcount), bdev_lvol; `#include`s `vbdev_tier.h` (requires the tier module, like 0005) | 0004 (bit-pool accessors), 0005 (lvol `-I` tier CFLAGS), tier module | +| 0044 | a thin cluster is cleared before use — a cluster allocated by a write (or write_zeroes, or an inflate) that covers only part of it is zeroed before the blob sees it. Its unwritten part returned what the device held there: another blob's data wherever the device's unmap does not zero (the free path counts on unmap to), and different bytes on each leg of a mirror, which a verify of the mirror reads as a content divergence. A write that covers the whole cluster skips the clear, so a rebuild or a copy does not write twice. `blob_ut` pins both, and its thin-provisioning I/O accounting now counts the cleared cluster | lib/blob (blobstore.c), test/unit/lib/blob | — | 0005 `#include`s `vbdev_tier.h` and adds `-I module/bdev/tier` to the lvol module CFLAGS via its own Makefile hunk; the Dockerfile injects the module dirs before applying patches (copy-before-apply ordering matters). From d42771cd7cdd494bdc4b67452314722d273cfc9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20DUCOM?= Date: Thu, 1 Oct 2026 04:58:02 +0200 Subject: [PATCH 03/18] test(raid): a seeded rebuild copies its ranges, and only under the windows they need (0045, tests only) Three cases for bdev_raid_ut, red on the series as it stands: - two seed ranges inside one window: 12 blocks to copy under one lock; the window is copied whole (128 blocks); - one range starting mid-window across three 4-block windows: 9 blocks under three locks, the first at the range; the windows are copied whole (12); - one range at the end of the raid: 4 blocks under one lock, at the range; the clean windows before it are each locked and skipped. The stubbed range quiesce counts the windows the copy phase locks. --- ...aid-seeded-rebuild-copies-its-ranges.patch | 157 ++++++++++++++++++ 1 file changed, 157 insertions(+) create mode 100644 patches/0045-raid-seeded-rebuild-copies-its-ranges.patch diff --git a/patches/0045-raid-seeded-rebuild-copies-its-ranges.patch b/patches/0045-raid-seeded-rebuild-copies-its-ranges.patch new file mode 100644 index 0000000..1a78986 --- /dev/null +++ b/patches/0045-raid-seeded-rebuild-copies-its-ranges.patch @@ -0,0 +1,157 @@ +diff --git a/test/unit/lib/bdev/raid/bdev_raid.c/bdev_raid_ut.c b/test/unit/lib/bdev/raid/bdev_raid.c/bdev_raid_ut.c +index 9efc3c3..f5a439b 100644 +--- a/test/unit/lib/bdev/raid/bdev_raid.c/bdev_raid_ut.c ++++ b/test/unit/lib/bdev/raid/bdev_raid.c/bdev_raid_ut.c +@@ -667,11 +667,23 @@ spdk_bdev_unquiesce(struct spdk_bdev *bdev, struct spdk_bdev_module *module, + return 0; + } + ++/* Evariops 0045: the windows the copy phase of a process locks, the verify ++ * phase's aside, and the lowest of them. */ ++static uint32_t g_copy_windows_locked; ++static uint64_t g_lowest_copy_window = UINT64_MAX; ++ + int + spdk_bdev_quiesce_range(struct spdk_bdev *bdev, struct spdk_bdev_module *module, + uint64_t offset, uint64_t length, + spdk_bdev_quiesce_cb cb_fn, void *cb_arg) + { ++ struct raid_bdev *raid_bdev = SPDK_CONTAINEROF(bdev, struct raid_bdev, bdev); ++ ++ if (raid_bdev->process != NULL && raid_bdev->process->verify == NULL) { ++ g_copy_windows_locked++; ++ g_lowest_copy_window = spdk_min(g_lowest_copy_window, offset); ++ } ++ + if (cb_fn) { + cb_fn(cb_arg, 0); + } +@@ -1547,6 +1559,118 @@ test_raid_process(void) + reset_globals(); + } + ++/* Evariops 0045: a seeded rebuild copies its seed ranges and nothing else. A ++ * window that intersected a range used to be copied whole: scattered writes ++ * across a volume dirty every window, and the "seeded" pass copied the volume. ++ * Runs the rebuild with the given window and returns the blocks copied. */ ++static uint64_t ++run_seeded_rebuild(uint32_t window_size_kb, const struct raid_bdev_seed_range *seed, uint32_t num_ranges) ++{ ++ struct rpc_bdev_raid_create req; ++ struct rpc_bdev_raid_delete destroy_req; ++ struct raid_bdev *pbdev; ++ struct spdk_bdev *base_bdev; ++ struct spdk_thread *process_thread; ++ struct raid_bdev_seed_range *ranges; ++ struct spdk_raid_bdev_opts opts = {}; ++ uint64_t num_blocks_processed = 0; ++ ++ set_globals(); ++ CU_ASSERT(raid_bdev_init() == 0); ++ opts.process_window_size_kb = window_size_kb; ++ CU_ASSERT(raid_bdev_set_opts(&opts) == 0); ++ g_copy_windows_locked = 0; ++ g_lowest_copy_window = UINT64_MAX; ++ ++ create_raid_bdev_create_req(&req, "raid1", 0, true, 0, false); ++ TAILQ_FOREACH(base_bdev, &g_bdev_list, internal.link) { ++ base_bdev->blockcnt = 128; ++ } ++ rpc_bdev_raid_create(NULL, NULL); ++ CU_ASSERT(g_rpc_err == 0); ++ free_test_req(&req); ++ ++ TAILQ_FOREACH(pbdev, &g_raid_bdev_list, global_link) { ++ if (strcmp(pbdev->bdev.name, "raid1") == 0) { ++ break; ++ } ++ } ++ SPDK_CU_ASSERT_FATAL(pbdev != NULL); ++ pbdev->module_private = &num_blocks_processed; ++ pbdev->min_base_bdevs_operational = 0; ++ ++ ranges = calloc(num_ranges, sizeof(*ranges)); ++ SPDK_CU_ASSERT_FATAL(ranges != NULL); ++ memcpy(ranges, seed, num_ranges * sizeof(*ranges)); ++ pbdev->base_bdev_info[0].write_only_pending = true; ++ CU_ASSERT(raid_bdev_start_seeded_rebuild(&pbdev->base_bdev_info[0], ranges, num_ranges, NULL) == 0); ++ poll_app_thread(); ++ SPDK_CU_ASSERT_FATAL(pbdev->process != NULL); ++ ++ process_thread = g_latest_thread; ++ while (spdk_thread_poll(process_thread, 0, 0) > 0) { ++ poll_app_thread(); ++ } ++ CU_ASSERT(pbdev->process == NULL); ++ poll_app_thread(); ++ ++ create_raid_bdev_delete_req(&destroy_req, "raid1", 0); ++ rpc_bdev_raid_delete(NULL, NULL); ++ CU_ASSERT(g_rpc_err == 0); ++ ++ /* The window size is global: put the default back for the next tests. */ ++ opts.process_window_size_kb = 1024; ++ CU_ASSERT(raid_bdev_set_opts(&opts) == 0); ++ raid_bdev_exit(); ++ base_bdevs_cleanup(); ++ reset_globals(); ++ ++ return num_blocks_processed; ++} ++ ++static void ++test_raid_process_seeded_copies_only_its_ranges(void) ++{ ++ /* One 1 MiB window holds the whole 128-block raid: both ranges fall in it, ++ * and are copied under that one window's lock. */ ++ const struct raid_bdev_seed_range seed[] = { ++ { .offset_blocks = 10, .num_blocks = 4 }, ++ { .offset_blocks = 40, .num_blocks = 8 }, ++ }; ++ ++ CU_ASSERT(run_seeded_rebuild(1024, seed, 2) == 12); ++ CU_ASSERT(g_copy_windows_locked == 1); ++} ++ ++static void ++test_raid_process_seeded_range_across_windows(void) ++{ ++ /* 16 KiB windows of 4 blocks: [6, 15) starts mid-window and spans three, ++ * the first of them at the range. */ ++ const struct raid_bdev_seed_range seed[] = { ++ { .offset_blocks = 6, .num_blocks = 9 }, ++ }; ++ ++ CU_ASSERT(run_seeded_rebuild(16, seed, 1) == 9); ++ CU_ASSERT(g_copy_windows_locked == 3); ++ CU_ASSERT(g_lowest_copy_window == 6); ++} ++ ++static void ++test_raid_process_seeded_skips_a_clean_span_without_a_window(void) ++{ ++ /* A clean span needs no copy, hence no lock: the 25 windows before the ++ * one range used to be locked and fast-skipped one after the other — on a ++ * 1 TiB volume, 262144 window cycles for an empty delta. */ ++ const struct raid_bdev_seed_range seed[] = { ++ { .offset_blocks = 100, .num_blocks = 4 }, ++ }; ++ ++ CU_ASSERT(run_seeded_rebuild(16, seed, 1) == 4); ++ CU_ASSERT(g_copy_windows_locked == 1); ++ CU_ASSERT(g_lowest_copy_window == 100); ++} ++ + static void + test_raid_process_with_qos(void) + { +@@ -1953,6 +2077,9 @@ main(int argc, char **argv) + CU_ADD_TEST(suite, test_raid_level_conversions); + CU_ADD_TEST(suite, test_raid_io_split); + CU_ADD_TEST(suite, test_raid_process); ++ CU_ADD_TEST(suite, test_raid_process_seeded_copies_only_its_ranges); ++ CU_ADD_TEST(suite, test_raid_process_seeded_range_across_windows); ++ CU_ADD_TEST(suite, test_raid_process_seeded_skips_a_clean_span_without_a_window); + CU_ADD_TEST(suite, test_raid_process_with_qos); + + spdk_thread_lib_init(test_new_thread_fn, 0); From 604f3ef3bc826f637641b96afba329d7e0f3c8ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20DUCOM?= Date: Thu, 1 Oct 2026 04:58:30 +0200 Subject: [PATCH 04/18] fix(raid): a seeded rebuild copies its ranges, and only under the windows they need (0045) A window that a seed range touched was copied whole, and every clean window was locked and skipped in turn. Measured on a three-node cluster under a random-write load: 5 GiB re-copied for 395 MiB of dirty chunks (4794 ranges), 110 s. Copying only the ranges, one window per range part, still took 100 s: the cost is the window cycle (quiesce, copy, channel update, unquiesce), not the bytes. - A seeded process jumps over what its ranges leave clean, with no window. Only a copy needs a window's quiesce (a write landing between the read of the source and the write of the target), and the seeded target is a member of every channel for writes: the routing by process offset sends a write to it on either side. - A window copies every range part it holds, under its one lock, and ends where everything before it is copied or clean. Out of requests, it ends at the first block not submitted and the next window resumes there. The verify phase and the copied-bytes accounting are unchanged. bdev_raid_ut: the three cases of the previous commit pass, and every raid suite stays green (bdev_raid 20, bdev_raid_sb 9, concat 3, raid0 8, raid1 5, raid5f 8). --- ...aid-seeded-rebuild-copies-its-ranges.patch | 99 +++++++++++++++++++ patches/README.md | 1 + 2 files changed, 100 insertions(+) diff --git a/patches/0045-raid-seeded-rebuild-copies-its-ranges.patch b/patches/0045-raid-seeded-rebuild-copies-its-ranges.patch index 1a78986..b919eb4 100644 --- a/patches/0045-raid-seeded-rebuild-copies-its-ranges.patch +++ b/patches/0045-raid-seeded-rebuild-copies-its-ranges.patch @@ -1,3 +1,102 @@ +diff --git a/module/bdev/raid/bdev_raid.c b/module/bdev/raid/bdev_raid.c +index 5064454..583707e 100644 +--- a/module/bdev/raid/bdev_raid.c ++++ b/module/bdev/raid/bdev_raid.c +@@ -3332,6 +3332,71 @@ raid_bdev_process_window_seeded_clean(struct raid_bdev_process *process) + return true; + } + ++/* Evariops 0045: the copy of a seeded window — every seed range part the window ++ * holds, all under its one lock. The window then ends where everything before it ++ * is either copied or clean; out of requests, it ends at the first block not ++ * submitted, and the next window resumes there. Copying the window whole as soon ++ * as one range touched it made scattered writes copy the whole volume (5 GiB for ++ * 395 MiB of dirty chunks), and one window per range part made them pay a lock ++ * cycle per part (100 s for the same 395 MiB). Process thread only. */ ++static void ++raid_bdev_process_window_seeded_copy(struct raid_bdev_process *process, uint64_t offset_end) ++{ ++ const uint64_t offset = process->window_offset; ++ uint64_t covered = offset_end; ++ uint32_t idx; ++ int ret; ++ ++ for (idx = process->seed_range_idx; idx < process->num_seed_ranges; idx++) { ++ const struct raid_bdev_seed_range *range = &process->seed_ranges[idx]; ++ uint64_t start = spdk_max(range->offset_blocks, offset); ++ const uint64_t end = spdk_min(range->offset_blocks + range->num_blocks, offset_end); ++ ++ if (start >= offset_end) { ++ break; ++ } ++ while (start < end) { ++ ret = raid_bdev_submit_process_request(process, start, end - start); ++ if (ret <= 0) { ++ covered = start; ++ goto submitted; ++ } ++ process->window_remaining += ret; ++ start += ret; ++ } ++ } ++ ++submitted: ++ if (process->window_remaining > 0) { ++ process->window_size = covered - offset; ++ } else { ++ raid_bdev_process_finish(process, process->window_status); ++ } ++} ++ ++/* Evariops 0045: a seeded process jumps, with no window, over what its ranges ++ * leave clean. Only a copy needs a window's quiesce (a write landing between the ++ * read of the source and the write of the target); the seeded target is a member ++ * of every channel for writes, so the routing by process.offset sends a write to ++ * it on either side of the offset. Without the jump, every clean window was ++ * locked and fast-skipped in turn: 262144 cycles on a 1 TiB volume, whatever its ++ * delta. Process thread, no window locked. */ ++static void ++raid_bdev_process_seeded_skip_clean(struct raid_bdev_process *process) ++{ ++ while (process->seed_range_idx < process->num_seed_ranges) { ++ const struct raid_bdev_seed_range *range = &process->seed_ranges[process->seed_range_idx]; ++ ++ if (range->offset_blocks + range->num_blocks <= process->window_offset) { ++ process->seed_range_idx++; ++ continue; ++ } ++ process->window_offset = spdk_max(process->window_offset, range->offset_blocks); ++ return; ++ } ++ process->window_offset = process->raid_bdev->bdev.blockcnt; ++} ++ + static void + _raid_bdev_process_thread_run(struct raid_bdev_process *process) + { +@@ -3351,6 +3416,11 @@ _raid_bdev_process_thread_run(struct raid_bdev_process *process) + return; + } + ++ if (spdk_unlikely(process->seeded)) { ++ raid_bdev_process_window_seeded_copy(process, offset_end); ++ return; ++ } ++ + while (offset < offset_end) { + ret = raid_bdev_submit_process_request(process, offset, offset_end - offset); + if (ret <= 0) { +@@ -3907,6 +3977,10 @@ raid_bdev_process_thread_run(struct raid_bdev_process *process) + return; + } + ++ if (spdk_unlikely(process->seeded)) { ++ raid_bdev_process_seeded_skip_clean(process); ++ } ++ + if (process->window_offset == raid_bdev->bdev.blockcnt) { + SPDK_DEBUGLOG(bdev_raid, "process completed on %s\n", raid_bdev->bdev.name); + /* Evariops 0018: run the verify phase before concluding. The diff --git a/test/unit/lib/bdev/raid/bdev_raid.c/bdev_raid_ut.c b/test/unit/lib/bdev/raid/bdev_raid.c/bdev_raid_ut.c index 9efc3c3..f5a439b 100644 --- a/test/unit/lib/bdev/raid/bdev_raid.c/bdev_raid_ut.c diff --git a/patches/README.md b/patches/README.md index 1e590d8..69551c6 100644 --- a/patches/README.md +++ b/patches/README.md @@ -50,6 +50,7 @@ Patches are applied in **lexicographic order of filename** (`0001` … `0044`) | 0040 | the delete-stop only MARKS — 0035's stop drove the window machinery from a foreign thread, unquiescing under in-flight requests; it now sets STOPPING with `-ECANCELED` and each resting state concludes | module/bdev/raid (bdev_raid.c) | 0035, 0039 | | 0041 | lvol → tier band usage provider — when an lvolstore loads on a `bdev_tier` composite the lvol layer registers a per-band fill-accounting provider, gated on the tier grain being a whole multiple of the lvolstore cluster (else no provider, honest unknown), and **unregisters it at teardown INITIATION** (before `spdk_lvs_unload`/`destroy`, whose synchronous store free would otherwise race a `get_bands` poll — UAF); `bdev_tier_get_bands` then fills `used_blocks` by counting allocated clusters of the blobstore's `used_clusters` pool in the band's LBA range (`spdk_bs_count_allocated_clusters_in_lba_range`, new in lib/blob — exclusive cluster-aligned bounds, one masked popcount per 64 clusters via `spdk_bit_pool_count_allocated_in_range` / `spdk_bit_array_count_set_in_range`, new in lib/util) | lib/blob, lib/util (ranged popcount), bdev_lvol; `#include`s `vbdev_tier.h` (requires the tier module, like 0005) | 0004 (bit-pool accessors), 0005 (lvol `-I` tier CFLAGS), tier module | | 0044 | a thin cluster is cleared before use — a cluster allocated by a write (or write_zeroes, or an inflate) that covers only part of it is zeroed before the blob sees it. Its unwritten part returned what the device held there: another blob's data wherever the device's unmap does not zero (the free path counts on unmap to), and different bytes on each leg of a mirror, which a verify of the mirror reads as a content divergence. A write that covers the whole cluster skips the clear, so a rebuild or a copy does not write twice. `blob_ut` pins both, and its thin-provisioning I/O accounting now counts the cleared cluster | lib/blob (blobstore.c), test/unit/lib/blob | — | +| 0045 | a seeded rebuild copies its seed ranges, and only under the windows they need — a seeded process jumps over what its ranges leave clean with no window (only a copy needs a window's quiesce, and the seeded target takes writes on either side of the process offset), and a window copies every range part it holds under its one lock. A window that a range touched used to be copied whole (scattered writes: 5 GiB re-copied for 395 MiB of dirty chunks), and every clean window was locked and skipped in turn (262144 cycles on a 1 TiB volume, whatever its delta). `bdev_raid_ut` pins the blocks copied and the windows locked | bdev_raid, test/unit/lib/bdev/raid | 0013 | 0005 `#include`s `vbdev_tier.h` and adds `-I module/bdev/tier` to the lvol module CFLAGS via its own Makefile hunk; the Dockerfile injects the module dirs before applying patches (copy-before-apply ordering matters). From 42b0e9c467cd29dbf0eedcec3b29878503583e47 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20DUCOM?= Date: Thu, 1 Oct 2026 05:20:53 +0200 Subject: [PATCH 05/18] feat(raid): a seeded rebuild reads its ranges from the CBT epoch (0046) A JSON-RPC request carries ~200 ranges (SPDK_JSONRPC_MAX_VALUES = 1024 parsed values, a 32 KiB receive buffer), so a caller had to fold a larger delta to fit, and folding a scattered delta recopies every clean block in between: measured on a three-node cluster under random writes, 4485 dirty ranges (368 MiB) folded to 128 recopied 3.2 GiB of clean blocks, and the seeded rebuild of a 5 GiB volume took as long as a full one (95-110 s). bdev_raid_start_seeded_rebuild_from_epoch {name, base_bdev, cbt_bdev, epoch_id, expected_incarnation, rebuild_token} seeds the same rebuild with the dirty ranges of a frozen epoch, read in-process: the cbt module answers vbdev_cbt_query_epoch_ranges (the ranges bdev_cbt_epoch_get_dirty_ranges walks, up to the caller's cap; more is -E2BIG, since a truncated delta is a wrong one, and the caller rebuilds in full). The two RPCs share their checks (raid, incarnation, running process, write_only member, range bounds, token) and their audit. bdev_raid_ut fakes the query: 64 one-block ranges a block apart copy their 64 blocks, and a delta the query refuses starts nothing. Every raid suite stays green (bdev_raid 22, bdev_raid_sb 9, concat 3, raid0 8, raid1 5, raid5f 8). --- module/bdev/cbt/vbdev_cbt.c | 47 ++ module/bdev/cbt/vbdev_cbt_query.h | 23 + ...uild-reads-its-ranges-from-the-epoch.patch | 522 ++++++++++++++++++ patches/README.md | 1 + 4 files changed, 593 insertions(+) create mode 100644 patches/0046-raid-seeded-rebuild-reads-its-ranges-from-the-epoch.patch diff --git a/module/bdev/cbt/vbdev_cbt.c b/module/bdev/cbt/vbdev_cbt.c index 13f0adb..b4221f2 100644 --- a/module/bdev/cbt/vbdev_cbt.c +++ b/module/bdev/cbt/vbdev_cbt.c @@ -573,6 +573,53 @@ vbdev_cbt_query_latest_epoch(const char *bdev_name, struct vbdev_cbt_epoch_facts return 0; } +/* Cross-module query — the delta of an epoch for the raid module's seeded + * rebuild (see vbdev_cbt_query.h): the ranges bdev_cbt_epoch_get_dirty_ranges + * walks, in the query's own type, refused whole when they do not all fit. */ +int +vbdev_cbt_query_epoch_ranges(const char *bdev_name, const char *epoch_id, uint32_t max, + struct vbdev_cbt_range **out, uint32_t *count) +{ + struct cbt_dirty_range *ranges = NULL; + struct vbdev_cbt_range *copy = NULL; + uint64_t dirty_chunks, total_chunks; + uint32_t num_ranges = 0, chunk_size_kb, i; + bool truncated = false; + int rc; + + assert(spdk_get_thread() == spdk_thread_get_app_thread()); + + *out = NULL; + *count = 0; + + rc = bdev_cbt_epoch_get_dirty_ranges(bdev_name, epoch_id, max, &ranges, &num_ranges, + &dirty_chunks, &total_chunks, &chunk_size_kb, &truncated); + if (rc != 0) { + return rc; + } + if (truncated) { + free(ranges); + return -E2BIG; + } + + if (num_ranges > 0) { + copy = calloc(num_ranges, sizeof(*copy)); + if (copy == NULL) { + free(ranges); + return -ENOMEM; + } + for (i = 0; i < num_ranges; i++) { + copy[i].offset_blocks = ranges[i].offset_blocks; + copy[i].num_blocks = ranges[i].length_blocks; + } + } + free(ranges); + + *out = copy; + *count = num_ranges; + return 0; +} + static int vbdev_cbt_dump_info_json(void *ctx, struct spdk_json_write_ctx *w) { diff --git a/module/bdev/cbt/vbdev_cbt_query.h b/module/bdev/cbt/vbdev_cbt_query.h index cfd29ec..0053d7c 100644 --- a/module/bdev/cbt/vbdev_cbt_query.h +++ b/module/bdev/cbt/vbdev_cbt_query.h @@ -45,4 +45,27 @@ int vbdev_cbt_query_latest_epoch(const char *bdev_name, struct vbdev_cbt_epoch_f */ int vbdev_cbt_auto_epoch_open(const char *bdev_name, const char *stale_backend_id); +/* One dirty range of an epoch, in blocks of the cbt bdev: the raid's own, since + * a cbt bdev passes its LBAs through. */ +struct vbdev_cbt_range { + uint64_t offset_blocks; + uint64_t num_blocks; +}; + +/** + * The dirty ranges of a frozen (or rebuilding) epoch of the cbt bdev named + * \c bdev_name, sorted and non-overlapping, for the raid module to seed a + * rebuild with in-process. A JSON-RPC request carries ~200 ranges at most + * (1024 parsed values, a 32 KiB receive buffer), and folding a scattered delta + * down to that many recopies every clean block in between. + * + * \param max the most ranges the caller takes. A delta with more is -E2BIG: + * truncated, it would not be a smaller delta but a wrong one. + * \return 0 with \c *out allocated (the caller frees it; NULL when \c *count is + * 0); -ENODEV, -ENOENT or -EINVAL as bdev_cbt_epoch_get_dirty_ranges; + * -E2BIG; -ENOMEM. App thread only. + */ +int vbdev_cbt_query_epoch_ranges(const char *bdev_name, const char *epoch_id, uint32_t max, + struct vbdev_cbt_range **out, uint32_t *count); + #endif /* SPDK_VBDEV_CBT_QUERY_H */ diff --git a/patches/0046-raid-seeded-rebuild-reads-its-ranges-from-the-epoch.patch b/patches/0046-raid-seeded-rebuild-reads-its-ranges-from-the-epoch.patch new file mode 100644 index 0000000..f7213c7 --- /dev/null +++ b/patches/0046-raid-seeded-rebuild-reads-its-ranges-from-the-epoch.patch @@ -0,0 +1,522 @@ +diff --git a/module/bdev/raid/bdev_raid_rpc.c b/module/bdev/raid/bdev_raid_rpc.c +index 6ded0ca..d4127d2 100644 +--- a/module/bdev/raid/bdev_raid_rpc.c ++++ b/module/bdev/raid/bdev_raid_rpc.c +@@ -8,6 +8,8 @@ + #include "bdev_raid.h" + #include "bdev_raid_outcomes.h" + #include "bdev_raid_envelopes.h" ++/* Evariops 0046: the seed of a rebuild read from the CBT epoch in-process. */ ++#include "../cbt/vbdev_cbt_query.h" + #include "spdk/util.h" + #include "spdk/string.h" + #include "spdk/log.h" +@@ -670,9 +672,9 @@ SPDK_RPC_REGISTER("bdev_raid_add_base_bdev", rpc_bdev_raid_add_base_bdev, SPDK_R + * progress shows up as the raid's process in bdev_raid_get_bdevs, and + * completion makes the member read-eligible and CONFIGURED in the superblock. */ + +-/* The rebuild engine works in 1 MiB windows, so finer ranges buy nothing: the +- * caller coalesces its dirty set down to this many ranges. Same cap as +- * bdev_raid_rebuild_ranges. */ ++/* A request decodes this many ranges at most; the JSON-RPC server parses ~200 ++ * anyway. A larger delta goes through bdev_raid_start_seeded_rebuild_from_epoch, ++ * which reads it in-process. Same cap as bdev_raid_rebuild_ranges. */ + #define RAID_SEEDED_REBUILD_MAX_RANGES 4096u + + struct rpc_bdev_raid_start_seeded_rebuild { +@@ -759,51 +761,40 @@ seed_range_cmp(const void *a, const void *b) + return 0; + } + +-static void +-rpc_bdev_raid_start_seeded_rebuild(struct spdk_jsonrpc_request *request, +- const struct spdk_json_val *params) ++/* The raid and its write_only-attached member a seeded rebuild targets, or ++ * false with the request answered: unknown raid, stale incarnation, a process ++ * already running, a member that is not write_only-attached. */ ++static bool ++seeded_rebuild_target(struct spdk_jsonrpc_request *request, const char *name, ++ const char *base_bdev, const char *expected_incarnation, ++ struct raid_bdev **raid_out, struct raid_base_bdev_info **base_out) + { +- struct rpc_bdev_raid_start_seeded_rebuild req = {}; + struct raid_bdev *raid_bdev; + struct raid_base_bdev_info *base_info = NULL, *iter; +- struct raid_bdev_seed_range *ranges; +- struct spdk_json_write_ctx *w; +- uint64_t blockcnt; +- char detail[192]; +- uint32_t i; +- int rc; +- +- if (spdk_json_decode_object(params, rpc_bdev_raid_start_seeded_rebuild_decoders, +- SPDK_COUNTOF(rpc_bdev_raid_start_seeded_rebuild_decoders), +- &req)) { +- spdk_jsonrpc_send_error_response(request, SPDK_JSONRPC_ERROR_PARSE_ERROR, +- "spdk_json_decode_object failed"); +- goto cleanup; +- } + +- raid_bdev = raid_bdev_find_by_name(req.name); ++ raid_bdev = raid_bdev_find_by_name(name); + if (raid_bdev == NULL) { + spdk_jsonrpc_send_error_response_fmt(request, -ENODEV, "raid bdev %s is not found in config", +- req.name); +- goto cleanup; ++ name); ++ return false; + } + +- if (raid_bdev_check_incarnation(raid_bdev, req.expected_incarnation) != 0) { ++ if (raid_bdev_check_incarnation(raid_bdev, expected_incarnation) != 0) { + spdk_jsonrpc_send_error_response_fmt(request, -ESTALE, + "stale incarnation for raid bdev %s", +- req.name); +- goto cleanup; ++ name); ++ return false; + } + + if (raid_bdev->process != NULL) { + spdk_jsonrpc_send_error_response_fmt(request, -EBUSY, + "raid bdev %s already has a background process", +- req.name); +- goto cleanup; ++ name); ++ return false; + } + + RAID_FOR_EACH_BASE_BDEV(raid_bdev, iter) { +- if (iter->name != NULL && strcmp(iter->name, req.base_bdev) == 0) { ++ if (iter->name != NULL && strcmp(iter->name, base_bdev) == 0) { + base_info = iter; + break; + } +@@ -811,24 +802,41 @@ rpc_bdev_raid_start_seeded_rebuild(struct spdk_jsonrpc_request *request, + if (base_info == NULL) { + spdk_jsonrpc_send_error_response_fmt(request, -ENODEV, + "base bdev %s is not a member of raid bdev %s", +- req.base_bdev, req.name); +- goto cleanup; ++ base_bdev, name); ++ return false; + } + + if (!base_info->write_only_pending) { + spdk_jsonrpc_send_error_response_fmt(request, -EINVAL, + "base bdev %s is not write_only-attached", +- req.base_bdev); +- goto cleanup; ++ base_bdev); ++ return false; + } + +- /* Sort, then validate bounds and non-overlap (the engine's fast-skip +- * cursor depends on both). Touching ranges are fine. */ +- qsort(req.ranges, req.num_ranges, sizeof(*req.ranges), seed_range_cmp); ++ *raid_out = raid_bdev; ++ *base_out = base_info; ++ return true; ++} + +- blockcnt = raid_bdev->bdev.blockcnt; +- for (i = 0; i < req.num_ranges; i++) { +- const struct raid_bdev_seed_range *range = &req.ranges[i]; ++/* Validates sorted ranges against the raid, audits the RPC, starts the seeded ++ * rebuild and answers the request. Takes ownership of ranges, freed on every ++ * path the rebuild does not take them. */ ++static void ++seeded_rebuild_start(struct spdk_jsonrpc_request *request, const char *method, ++ struct raid_bdev *raid_bdev, struct raid_base_bdev_info *base_info, ++ struct raid_bdev_seed_range *ranges, uint32_t num_ranges, ++ const char *rebuild_token, const char *audit_extra) ++{ ++ struct spdk_json_write_ctx *w; ++ uint64_t blockcnt = raid_bdev->bdev.blockcnt; ++ char detail[256]; ++ uint32_t i; ++ int rc; ++ ++ /* The engine's cursor depends on sorted, non-overlapping ranges; touching ++ * ranges are fine. */ ++ for (i = 0; i < num_ranges; i++) { ++ const struct raid_bdev_seed_range *range = &ranges[i]; + + if (range->num_blocks == 0 || range->offset_blocks >= blockcnt || + range->num_blocks > blockcnt - range->offset_blocks) { +@@ -837,55 +845,85 @@ rpc_bdev_raid_start_seeded_rebuild(struct spdk_jsonrpc_request *request, + "exceeds the raid size (%" PRIu64 " blocks)", + i, range->offset_blocks, range->num_blocks, + blockcnt); +- goto cleanup; ++ goto err; + } +- if (i > 0 && req.ranges[i - 1].offset_blocks + req.ranges[i - 1].num_blocks > ++ if (i > 0 && ranges[i - 1].offset_blocks + ranges[i - 1].num_blocks > + range->offset_blocks) { + spdk_jsonrpc_send_error_response_fmt(request, SPDK_JSONRPC_ERROR_INVALID_PARAMS, + "ranges %u and %u overlap", i - 1, i); +- goto cleanup; ++ goto err; + } + } + +- if (req.rebuild_token != NULL && +- strnlen(req.rebuild_token, RAID_REBUILD_TOKEN_MAX) == RAID_REBUILD_TOKEN_MAX) { ++ if (rebuild_token != NULL && ++ strnlen(rebuild_token, RAID_REBUILD_TOKEN_MAX) == RAID_REBUILD_TOKEN_MAX) { + spdk_jsonrpc_send_error_response_fmt(request, -ENAMETOOLONG, + "rebuild_token exceeds %d characters", + RAID_REBUILD_TOKEN_MAX - 1); +- goto cleanup; ++ goto err; + } + + /* Evariops 0011: audit every RPC that writes to the array — this one arms + * a background writer. */ +- snprintf(detail, sizeof(detail), "raid=%s base=%s ranges=%zu", +- req.name, req.base_bdev, req.num_ranges); +- spdk_jsonrpc_request_audit(request, "bdev_raid_start_seeded_rebuild", detail); ++ snprintf(detail, sizeof(detail), "raid=%s base=%s ranges=%u%s", ++ raid_bdev->bdev.name, base_info->name, num_ranges, audit_extra); ++ spdk_jsonrpc_request_audit(request, method, detail); + + /* An empty set is legal: nothing was dirty, the backfill skips the whole + * device, and the completion still promotes the member. */ +- if (req.num_ranges == 0) { +- free(req.ranges); ++ if (num_ranges == 0) { ++ free(ranges); + ranges = NULL; +- } else { +- ranges = req.ranges; + } +- req.ranges = NULL; + +- rc = raid_bdev_start_seeded_rebuild(base_info, ranges, (uint32_t)req.num_ranges, +- req.rebuild_token); ++ rc = raid_bdev_start_seeded_rebuild(base_info, ranges, num_ranges, rebuild_token); + if (rc != 0) { +- free(ranges); + spdk_jsonrpc_send_error_response_fmt(request, rc, + "Failed to start seeded rebuild on %s/%s: %s", +- req.name, req.base_bdev, spdk_strerror(-rc)); +- goto cleanup; ++ raid_bdev->bdev.name, base_info->name, ++ spdk_strerror(-rc)); ++ goto err; + } + + w = spdk_jsonrpc_begin_result(request); + spdk_json_write_object_begin(w); +- spdk_json_write_named_string_fmt(w, "rebuild_id", "%s:%s", req.name, req.base_bdev); ++ spdk_json_write_named_string_fmt(w, "rebuild_id", "%s:%s", raid_bdev->bdev.name, ++ base_info->name); + spdk_json_write_object_end(w); + spdk_jsonrpc_end_result(request, w); ++ return; ++ ++err: ++ free(ranges); ++} ++ ++static void ++rpc_bdev_raid_start_seeded_rebuild(struct spdk_jsonrpc_request *request, ++ const struct spdk_json_val *params) ++{ ++ struct rpc_bdev_raid_start_seeded_rebuild req = {}; ++ struct raid_bdev *raid_bdev; ++ struct raid_base_bdev_info *base_info; ++ struct raid_bdev_seed_range *ranges; ++ ++ if (spdk_json_decode_object(params, rpc_bdev_raid_start_seeded_rebuild_decoders, ++ SPDK_COUNTOF(rpc_bdev_raid_start_seeded_rebuild_decoders), ++ &req)) { ++ spdk_jsonrpc_send_error_response(request, SPDK_JSONRPC_ERROR_PARSE_ERROR, ++ "spdk_json_decode_object failed"); ++ goto cleanup; ++ } ++ ++ if (!seeded_rebuild_target(request, req.name, req.base_bdev, req.expected_incarnation, ++ &raid_bdev, &base_info)) { ++ goto cleanup; ++ } ++ ++ qsort(req.ranges, req.num_ranges, sizeof(*req.ranges), seed_range_cmp); ++ ranges = req.ranges; ++ req.ranges = NULL; ++ seeded_rebuild_start(request, "bdev_raid_start_seeded_rebuild", raid_bdev, base_info, ++ ranges, (uint32_t)req.num_ranges, req.rebuild_token, ""); + + cleanup: + free_rpc_bdev_raid_start_seeded_rebuild(&req); +@@ -893,6 +931,107 @@ cleanup: + SPDK_RPC_REGISTER("bdev_raid_start_seeded_rebuild", rpc_bdev_raid_start_seeded_rebuild, + SPDK_RPC_RUNTIME) + ++/* Evariops 0046: bdev_raid_start_seeded_rebuild_from_epoch {name, base_bdev, ++ * cbt_bdev, epoch_id} — the seeded rebuild of bdev_raid_start_seeded_rebuild, ++ * seeded with the dirty ranges of a frozen CBT epoch read in-process. A request ++ * carries ~200 ranges at most (1024 parsed values, a 32 KiB receive buffer): ++ * folding a scattered delta down to that many recopied the clean blocks in ++ * between, 3.2 GiB of them for 4485 dirty ranges (368 MiB), measured. A delta of ++ * more ranges than this is refused (-E2BIG): the caller rebuilds in full. */ ++#define RAID_SEEDED_REBUILD_MAX_EPOCH_RANGES 65536u ++ ++struct rpc_bdev_raid_start_seeded_rebuild_from_epoch { ++ char *name; ++ char *base_bdev; ++ /* The cbt bdev over this raid, and the frozen epoch of the member's outage. */ ++ char *cbt_bdev; ++ char *epoch_id; ++ char *expected_incarnation; ++ char *rebuild_token; ++}; ++ ++static void ++free_rpc_bdev_raid_start_seeded_rebuild_from_epoch( ++ struct rpc_bdev_raid_start_seeded_rebuild_from_epoch *req) ++{ ++ free(req->name); ++ free(req->base_bdev); ++ free(req->cbt_bdev); ++ free(req->epoch_id); ++ free(req->expected_incarnation); ++ free(req->rebuild_token); ++} ++ ++static const struct spdk_json_object_decoder rpc_bdev_raid_start_seeded_rebuild_from_epoch_decoders[] = { ++ {"name", offsetof(struct rpc_bdev_raid_start_seeded_rebuild_from_epoch, name), spdk_json_decode_string}, ++ {"base_bdev", offsetof(struct rpc_bdev_raid_start_seeded_rebuild_from_epoch, base_bdev), spdk_json_decode_string}, ++ {"cbt_bdev", offsetof(struct rpc_bdev_raid_start_seeded_rebuild_from_epoch, cbt_bdev), spdk_json_decode_string}, ++ {"epoch_id", offsetof(struct rpc_bdev_raid_start_seeded_rebuild_from_epoch, epoch_id), spdk_json_decode_string}, ++ {"expected_incarnation", offsetof(struct rpc_bdev_raid_start_seeded_rebuild_from_epoch, expected_incarnation), spdk_json_decode_string, true}, ++ {"rebuild_token", offsetof(struct rpc_bdev_raid_start_seeded_rebuild_from_epoch, rebuild_token), spdk_json_decode_string, true}, ++}; ++ ++static void ++rpc_bdev_raid_start_seeded_rebuild_from_epoch(struct spdk_jsonrpc_request *request, ++ const struct spdk_json_val *params) ++{ ++ struct rpc_bdev_raid_start_seeded_rebuild_from_epoch req = {}; ++ struct raid_bdev *raid_bdev; ++ struct raid_base_bdev_info *base_info; ++ struct vbdev_cbt_range *epoch_ranges = NULL; ++ struct raid_bdev_seed_range *ranges = NULL; ++ uint32_t num_ranges = 0, i; ++ char audit_extra[96]; ++ int rc; ++ ++ if (spdk_json_decode_object(params, rpc_bdev_raid_start_seeded_rebuild_from_epoch_decoders, ++ SPDK_COUNTOF(rpc_bdev_raid_start_seeded_rebuild_from_epoch_decoders), ++ &req)) { ++ spdk_jsonrpc_send_error_response(request, SPDK_JSONRPC_ERROR_PARSE_ERROR, ++ "spdk_json_decode_object failed"); ++ goto cleanup; ++ } ++ ++ if (!seeded_rebuild_target(request, req.name, req.base_bdev, req.expected_incarnation, ++ &raid_bdev, &base_info)) { ++ goto cleanup; ++ } ++ ++ rc = vbdev_cbt_query_epoch_ranges(req.cbt_bdev, req.epoch_id, ++ RAID_SEEDED_REBUILD_MAX_EPOCH_RANGES, ++ &epoch_ranges, &num_ranges); ++ if (rc != 0) { ++ spdk_jsonrpc_send_error_response_fmt(request, rc, ++ "epoch %s of %s gives no seed for %s/%s: %s", ++ req.epoch_id, req.cbt_bdev, req.name, req.base_bdev, ++ spdk_strerror(-rc)); ++ goto cleanup; ++ } ++ ++ if (num_ranges > 0) { ++ ranges = calloc(num_ranges, sizeof(*ranges)); ++ if (ranges == NULL) { ++ free(epoch_ranges); ++ spdk_jsonrpc_send_error_response(request, -ENOMEM, spdk_strerror(ENOMEM)); ++ goto cleanup; ++ } ++ for (i = 0; i < num_ranges; i++) { ++ ranges[i].offset_blocks = epoch_ranges[i].offset_blocks; ++ ranges[i].num_blocks = epoch_ranges[i].num_blocks; ++ } ++ } ++ free(epoch_ranges); ++ ++ snprintf(audit_extra, sizeof(audit_extra), " epoch=%s", req.epoch_id); ++ seeded_rebuild_start(request, "bdev_raid_start_seeded_rebuild_from_epoch", raid_bdev, ++ base_info, ranges, num_ranges, req.rebuild_token, audit_extra); ++ ++cleanup: ++ free_rpc_bdev_raid_start_seeded_rebuild_from_epoch(&req); ++} ++SPDK_RPC_REGISTER("bdev_raid_start_seeded_rebuild_from_epoch", ++ rpc_bdev_raid_start_seeded_rebuild_from_epoch, SPDK_RPC_RUNTIME) ++ + struct rpc_bdev_raid_remove_base_bdev { + /* Base bdev name */ + char *name; +diff --git a/test/unit/lib/bdev/raid/bdev_raid.c/bdev_raid_ut.c b/test/unit/lib/bdev/raid/bdev_raid.c/bdev_raid_ut.c +index f5a439b..11b46f2 100644 +--- a/test/unit/lib/bdev/raid/bdev_raid.c/bdev_raid_ut.c ++++ b/test/unit/lib/bdev/raid/bdev_raid.c/bdev_raid_ut.c +@@ -246,6 +246,40 @@ DEFINE_STUB_V(raid_rebuild_outcome_set_verifying, (struct raid_rebuild_outcome * + * has CBT here (-ENODEV), which the raid passes over. */ + DEFINE_STUB(vbdev_cbt_auto_epoch_open, int, (const char *bdev_name, const char *stale_backend_id), + -ENODEV); ++ ++/* Evariops 0046: a seeded rebuild reads its ranges from a CBT epoch. The CBT ++ * module is not part of this suite: the epoch's delta is this fake's. */ ++static const struct raid_bdev_seed_range *g_epoch_seed; ++static uint32_t g_epoch_seed_count; ++static int g_epoch_query_rc; ++static char g_epoch_queried[64]; ++ ++int ++vbdev_cbt_query_epoch_ranges(const char *bdev_name, const char *epoch_id, uint32_t max, ++ struct vbdev_cbt_range **out, uint32_t *count) ++{ ++ uint32_t i; ++ ++ snprintf(g_epoch_queried, sizeof(g_epoch_queried), "%s/%s", bdev_name, epoch_id); ++ *out = NULL; ++ *count = 0; ++ if (g_epoch_query_rc != 0) { ++ return g_epoch_query_rc; ++ } ++ if (g_epoch_seed_count > max) { ++ return -E2BIG; ++ } ++ if (g_epoch_seed_count > 0) { ++ *out = calloc(g_epoch_seed_count, sizeof(**out)); ++ SPDK_CU_ASSERT_FATAL(*out != NULL); ++ for (i = 0; i < g_epoch_seed_count; i++) { ++ (*out)[i].offset_blocks = g_epoch_seed[i].offset_blocks; ++ (*out)[i].num_blocks = g_epoch_seed[i].num_blocks; ++ } ++ } ++ *count = g_epoch_seed_count; ++ return 0; ++} + /* Evariops 0021: the envelopes pace and admit the rebuilds. At their defaults here: + * no cap (0), and every rebuild admitted. */ + DEFINE_STUB(raid_envelope_active_mb_sec, uint32_t, (enum raid_envelope_class klass), 0); +@@ -1562,9 +1596,12 @@ test_raid_process(void) + /* Evariops 0045: a seeded rebuild copies its seed ranges and nothing else. A + * window that intersected a range used to be copied whole: scattered writes + * across a volume dirty every window, and the "seeded" pass copied the volume. +- * Runs the rebuild with the given window and returns the blocks copied. */ ++ * Runs the rebuild with the given window and returns the blocks copied — ++ * started directly, or (Evariops 0046) through the RPC that reads the seed from ++ * a CBT epoch, whose query answers query_rc. A refused start copies nothing. */ + static uint64_t +-run_seeded_rebuild(uint32_t window_size_kb, const struct raid_bdev_seed_range *seed, uint32_t num_ranges) ++seeded_rebuild_run(uint32_t window_size_kb, const struct raid_bdev_seed_range *seed, uint32_t num_ranges, ++ bool via_epoch, int query_rc) + { + struct rpc_bdev_raid_create req; + struct rpc_bdev_raid_delete destroy_req; +@@ -1599,20 +1636,43 @@ run_seeded_rebuild(uint32_t window_size_kb, const struct raid_bdev_seed_range *s + pbdev->module_private = &num_blocks_processed; + pbdev->min_base_bdevs_operational = 0; + +- ranges = calloc(num_ranges, sizeof(*ranges)); +- SPDK_CU_ASSERT_FATAL(ranges != NULL); +- memcpy(ranges, seed, num_ranges * sizeof(*ranges)); + pbdev->base_bdev_info[0].write_only_pending = true; +- CU_ASSERT(raid_bdev_start_seeded_rebuild(&pbdev->base_bdev_info[0], ranges, num_ranges, NULL) == 0); ++ if (via_epoch) { ++ struct rpc_bdev_raid_start_seeded_rebuild_from_epoch epoch_req = { ++ .name = strdup("raid1"), ++ .base_bdev = strdup(pbdev->base_bdev_info[0].name), ++ .cbt_bdev = strdup("cbt_raid1"), ++ .epoch_id = strdup("epoch-0046"), ++ }; ++ ++ g_epoch_seed = seed; ++ g_epoch_seed_count = num_ranges; ++ g_epoch_query_rc = query_rc; ++ g_json_decode_obj_create = 0; ++ g_rpc_req = &epoch_req; ++ g_rpc_req_size = sizeof(epoch_req); ++ rpc_bdev_raid_start_seeded_rebuild_from_epoch(NULL, NULL); ++ g_epoch_query_rc = 0; ++ CU_ASSERT(strcmp(g_epoch_queried, "cbt_raid1/epoch-0046") == 0); ++ CU_ASSERT(g_rpc_err == (query_rc != 0)); ++ } else { ++ ranges = calloc(num_ranges, sizeof(*ranges)); ++ SPDK_CU_ASSERT_FATAL(ranges != NULL); ++ memcpy(ranges, seed, num_ranges * sizeof(*ranges)); ++ CU_ASSERT(raid_bdev_start_seeded_rebuild(&pbdev->base_bdev_info[0], ranges, num_ranges, NULL) == 0); ++ } + poll_app_thread(); +- SPDK_CU_ASSERT_FATAL(pbdev->process != NULL); + +- process_thread = g_latest_thread; +- while (spdk_thread_poll(process_thread, 0, 0) > 0) { +- poll_app_thread(); ++ if (query_rc == 0) { ++ SPDK_CU_ASSERT_FATAL(pbdev->process != NULL); ++ process_thread = g_latest_thread; ++ while (spdk_thread_poll(process_thread, 0, 0) > 0) { ++ poll_app_thread(); ++ } + } + CU_ASSERT(pbdev->process == NULL); + poll_app_thread(); ++ g_rpc_err = 0; + + create_raid_bdev_delete_req(&destroy_req, "raid1", 0); + rpc_bdev_raid_delete(NULL, NULL); +@@ -1628,6 +1688,40 @@ run_seeded_rebuild(uint32_t window_size_kb, const struct raid_bdev_seed_range *s + return num_blocks_processed; + } + ++static uint64_t ++run_seeded_rebuild(uint32_t window_size_kb, const struct raid_bdev_seed_range *seed, uint32_t num_ranges) ++{ ++ return seeded_rebuild_run(window_size_kb, seed, num_ranges, false, 0); ++} ++ ++/* Evariops 0046: the epoch's ranges, read in-process, are the seed as they are — ++ * here 64 one-block ranges a block apart, each copied alone. A request carries ++ * ~200 ranges, and a scattered delta folded to fit one recopied the clean blocks ++ * in between. */ ++static void ++test_raid_seeded_rebuild_from_epoch_copies_every_range(void) ++{ ++ struct raid_bdev_seed_range seed[64]; ++ uint32_t i; ++ ++ for (i = 0; i < 64; i++) { ++ seed[i].offset_blocks = 2 * i; ++ seed[i].num_blocks = 1; ++ } ++ ++ CU_ASSERT(seeded_rebuild_run(16, seed, 64, true, 0) == 64); ++} ++ ++static void ++test_raid_seeded_rebuild_from_epoch_refuses_a_delta_it_cannot_take_whole(void) ++{ ++ const struct raid_bdev_seed_range seed[] = { ++ { .offset_blocks = 10, .num_blocks = 4 }, ++ }; ++ ++ CU_ASSERT(seeded_rebuild_run(16, seed, 1, true, -E2BIG) == 0); ++} ++ + static void + test_raid_process_seeded_copies_only_its_ranges(void) + { +@@ -2080,6 +2174,8 @@ main(int argc, char **argv) + CU_ADD_TEST(suite, test_raid_process_seeded_copies_only_its_ranges); + CU_ADD_TEST(suite, test_raid_process_seeded_range_across_windows); + CU_ADD_TEST(suite, test_raid_process_seeded_skips_a_clean_span_without_a_window); ++ CU_ADD_TEST(suite, test_raid_seeded_rebuild_from_epoch_copies_every_range); ++ CU_ADD_TEST(suite, test_raid_seeded_rebuild_from_epoch_refuses_a_delta_it_cannot_take_whole); + CU_ADD_TEST(suite, test_raid_process_with_qos); + + spdk_thread_lib_init(test_new_thread_fn, 0); diff --git a/patches/README.md b/patches/README.md index 69551c6..4e5d631 100644 --- a/patches/README.md +++ b/patches/README.md @@ -51,6 +51,7 @@ Patches are applied in **lexicographic order of filename** (`0001` … `0044`) | 0041 | lvol → tier band usage provider — when an lvolstore loads on a `bdev_tier` composite the lvol layer registers a per-band fill-accounting provider, gated on the tier grain being a whole multiple of the lvolstore cluster (else no provider, honest unknown), and **unregisters it at teardown INITIATION** (before `spdk_lvs_unload`/`destroy`, whose synchronous store free would otherwise race a `get_bands` poll — UAF); `bdev_tier_get_bands` then fills `used_blocks` by counting allocated clusters of the blobstore's `used_clusters` pool in the band's LBA range (`spdk_bs_count_allocated_clusters_in_lba_range`, new in lib/blob — exclusive cluster-aligned bounds, one masked popcount per 64 clusters via `spdk_bit_pool_count_allocated_in_range` / `spdk_bit_array_count_set_in_range`, new in lib/util) | lib/blob, lib/util (ranged popcount), bdev_lvol; `#include`s `vbdev_tier.h` (requires the tier module, like 0005) | 0004 (bit-pool accessors), 0005 (lvol `-I` tier CFLAGS), tier module | | 0044 | a thin cluster is cleared before use — a cluster allocated by a write (or write_zeroes, or an inflate) that covers only part of it is zeroed before the blob sees it. Its unwritten part returned what the device held there: another blob's data wherever the device's unmap does not zero (the free path counts on unmap to), and different bytes on each leg of a mirror, which a verify of the mirror reads as a content divergence. A write that covers the whole cluster skips the clear, so a rebuild or a copy does not write twice. `blob_ut` pins both, and its thin-provisioning I/O accounting now counts the cleared cluster | lib/blob (blobstore.c), test/unit/lib/blob | — | | 0045 | a seeded rebuild copies its seed ranges, and only under the windows they need — a seeded process jumps over what its ranges leave clean with no window (only a copy needs a window's quiesce, and the seeded target takes writes on either side of the process offset), and a window copies every range part it holds under its one lock. A window that a range touched used to be copied whole (scattered writes: 5 GiB re-copied for 395 MiB of dirty chunks), and every clean window was locked and skipped in turn (262144 cycles on a 1 TiB volume, whatever its delta). `bdev_raid_ut` pins the blocks copied and the windows locked | bdev_raid, test/unit/lib/bdev/raid | 0013 | +| 0046 | a seeded rebuild reads its ranges from the CBT epoch — `bdev_raid_start_seeded_rebuild_from_epoch {name, base_bdev, cbt_bdev, epoch_id}` seeds the rebuild of `bdev_raid_start_seeded_rebuild` with the dirty ranges of a frozen epoch, read in-process through `vbdev_cbt_query_epoch_ranges` (up to 65536; more is -E2BIG, the caller rebuilds in full). A request carries ~200 ranges (1024 parsed values, a 32 KiB receive buffer): a scattered delta folded to fit recopied every clean block in between, 3.2 GiB for 4485 dirty ranges (368 MiB), measured. Both RPCs share their checks. `bdev_raid_ut` fakes the query | bdev_raid (rpc), module/bdev/cbt (query), test/unit/lib/bdev/raid | 0013, 0045, cbt module | 0005 `#include`s `vbdev_tier.h` and adds `-I module/bdev/tier` to the lvol module CFLAGS via its own Makefile hunk; the Dockerfile injects the module dirs before applying patches (copy-before-apply ordering matters). From f82970b1fcb0be2135a89bee9232f4e36ec000bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20DUCOM?= Date: Thu, 1 Oct 2026 06:30:20 +0200 Subject: [PATCH 06/18] test(raid1): a rebuild copies its window in parts, in flight together (0047, tests only) Two cases, red on the series as it stands: - raid1_ut: a 64-block window offered as the generic layer offers it (the rest of the window to the next request) is copied by four requests of at most the part, every part read before any completes, the reads tiling the window and spread evenly over the members in sync, and each part's read completing writes that part to the target. raid1 takes the window whole in one request, and on three members the second one serves no read; - bdev_raid_ut: raid_bdev_process_request_max_part offers parts that are whole write units and all fit RAID_BDEV_PROCESS_MAX_QD requests, smaller than any window with room to split. It offers the window whole. raid1_ut now inits a process request's raid_io for real (its target names the raid) and records every I/O the module submits. --- ...1-rebuild-copies-its-window-in-parts.patch | 304 ++++++++++++++++++ patches/README.md | 3 +- 2 files changed, 306 insertions(+), 1 deletion(-) create mode 100644 patches/0047-raid1-rebuild-copies-its-window-in-parts.patch diff --git a/patches/0047-raid1-rebuild-copies-its-window-in-parts.patch b/patches/0047-raid1-rebuild-copies-its-window-in-parts.patch new file mode 100644 index 0000000..d469caf --- /dev/null +++ b/patches/0047-raid1-rebuild-copies-its-window-in-parts.patch @@ -0,0 +1,304 @@ +diff --git a/module/bdev/raid/bdev_raid.c b/module/bdev/raid/bdev_raid.c +index 583707e..d34fbf9 100644 +--- a/module/bdev/raid/bdev_raid.c ++++ b/module/bdev/raid/bdev_raid.c +@@ -3266,6 +3266,12 @@ raid_bdev_process_request_complete(struct raid_bdev_process_request *process_req + } + } + ++uint32_t ++raid_bdev_process_request_max_part(const struct raid_bdev_process_request *process_req) ++{ ++ return process_req->process->max_window_size; ++} ++ + static int + raid_bdev_submit_process_request(struct raid_bdev_process *process, uint64_t offset_blocks, + uint32_t num_blocks) +diff --git a/module/bdev/raid/bdev_raid.h b/module/bdev/raid/bdev_raid.h +index 280d61c..f584d19 100644 +--- a/module/bdev/raid/bdev_raid.h ++++ b/module/bdev/raid/bdev_raid.h +@@ -481,6 +481,10 @@ void *raid_bdev_channel_get_module_ctx(struct raid_bdev_io_channel *raid_ch); + struct raid_base_bdev_info *raid_bdev_channel_get_base_info(struct raid_bdev_io_channel *raid_ch, + struct spdk_bdev *base_bdev); + void raid_bdev_process_request_complete(struct raid_bdev_process_request *process_req, int status); ++/* Evariops 0047: the largest part of its window one process request should copy, ++ * for a module that copies any range (raid1). A module bound to a stripe (raid5f) ++ * ignores it. */ ++uint32_t raid_bdev_process_request_max_part(const struct raid_bdev_process_request *process_req); + void raid_bdev_io_init(struct raid_bdev_io *raid_io, struct raid_bdev_io_channel *raid_ch, + enum spdk_bdev_io_type type, uint64_t offset_blocks, + uint64_t num_blocks, struct iovec *iovs, int iovcnt, void *md_buf, +diff --git a/test/unit/lib/bdev/raid/bdev_raid.c/bdev_raid_ut.c b/test/unit/lib/bdev/raid/bdev_raid.c/bdev_raid_ut.c +index 11b46f2..a488d6d 100644 +--- a/test/unit/lib/bdev/raid/bdev_raid.c/bdev_raid_ut.c ++++ b/test/unit/lib/bdev/raid/bdev_raid.c/bdev_raid_ut.c +@@ -1765,6 +1765,35 @@ test_raid_process_seeded_skips_a_clean_span_without_a_window(void) + CU_ASSERT(g_lowest_copy_window == 100); + } + ++/* Evariops 0047: a window is offered to its requests in parts that all fit at once, ++ * so its copy is in flight together, and a window with room for it is split. The ++ * largest part is whole write units; the window's last part is what remains. */ ++static void ++test_raid_process_request_max_part(void) ++{ ++ struct raid_bdev raid_bdev = {}; ++ struct raid_bdev_process process = { .raid_bdev = &raid_bdev }; ++ struct raid_bdev_process_request process_req = { .process = &process }; ++ const uint64_t windows[] = { 8192, 2048, 1000, 17, 3, 1 }; ++ const uint32_t write_units[] = { 1, 8 }; ++ uint32_t w, u, part; ++ ++ for (u = 0; u < SPDK_COUNTOF(write_units); u++) { ++ raid_bdev.bdev.write_unit_size = write_units[u]; ++ for (w = 0; w < SPDK_COUNTOF(windows); w++) { ++ process.max_window_size = windows[w]; ++ part = raid_bdev_process_request_max_part(&process_req); ++ ++ CU_ASSERT(part > 0); ++ CU_ASSERT(part % write_units[u] == 0); ++ CU_ASSERT((uint64_t)part * RAID_BDEV_PROCESS_MAX_QD >= windows[w]); ++ if (windows[w] >= 2ULL * RAID_BDEV_PROCESS_MAX_QD * write_units[u]) { ++ CU_ASSERT(part < windows[w]); ++ } ++ } ++ } ++} ++ + static void + test_raid_process_with_qos(void) + { +@@ -2176,6 +2205,7 @@ main(int argc, char **argv) + CU_ADD_TEST(suite, test_raid_process_seeded_skips_a_clean_span_without_a_window); + CU_ADD_TEST(suite, test_raid_seeded_rebuild_from_epoch_copies_every_range); + CU_ADD_TEST(suite, test_raid_seeded_rebuild_from_epoch_refuses_a_delta_it_cannot_take_whole); ++ CU_ADD_TEST(suite, test_raid_process_request_max_part); + CU_ADD_TEST(suite, test_raid_process_with_qos); + + spdk_thread_lib_init(test_new_thread_fn, 0); +diff --git a/test/unit/lib/bdev/raid/raid1.c/raid1_ut.c b/test/unit/lib/bdev/raid/raid1.c/raid1_ut.c +index 9c3eb8f..88f4295 100644 +--- a/test/unit/lib/bdev/raid/raid1.c/raid1_ut.c ++++ b/test/unit/lib/bdev/raid/raid1.c/raid1_ut.c +@@ -23,11 +23,6 @@ DEFINE_STUB_V(raid_bdev_queue_io_wait, (struct raid_bdev_io *raid_io, struct spd + struct spdk_io_channel *ch, spdk_bdev_io_wait_cb cb_fn)); + DEFINE_STUB_V(raid_bdev_process_request_complete, (struct raid_bdev_process_request *process_req, + int status)); +-DEFINE_STUB_V(raid_bdev_io_init, (struct raid_bdev_io *raid_io, +- struct raid_bdev_io_channel *raid_ch, +- enum spdk_bdev_io_type type, uint64_t offset_blocks, +- uint64_t num_blocks, struct iovec *iovs, int iovcnt, void *md_buf, +- struct spdk_memory_domain *memory_domain, void *memory_domain_ctx)); + DEFINE_STUB(raid_bdev_remap_dix_reftag, int, (void *md_buf, uint64_t num_blocks, + struct spdk_bdev *bdev, uint32_t remapped_offset), -1); + DEFINE_STUB(spdk_bdev_notify_blockcnt_change, int, (struct spdk_bdev *bdev, uint64_t size), 0); +@@ -38,14 +33,41 @@ DEFINE_STUB(spdk_bdev_unmap_blocks, int, (struct spdk_bdev_desc *desc, struct sp + uint64_t offset_blocks, uint64_t num_blocks, spdk_bdev_io_completion_cb cb, + void *cb_arg), 0); + ++/* Evariops 0047: the I/O the module submitted, in order, so a test reads back which ++ * member served a read and where a write landed, and completes each in turn. */ ++struct ut_io { ++ bool write; ++ struct spdk_bdev_desc *desc; ++ uint64_t offset_blocks; ++ uint64_t num_blocks; ++ spdk_bdev_io_completion_cb cb; ++ void *cb_arg; ++}; ++static struct ut_io g_ios[32]; ++static int g_num_ios; ++ ++static void ++ut_record_io(bool write, struct spdk_bdev_desc *desc, uint64_t offset_blocks, uint64_t num_blocks, ++ spdk_bdev_io_completion_cb cb, void *cb_arg) ++{ ++ g_last_io_desc = desc; ++ g_last_io_cb = cb; ++ ++ if (g_num_ios < (int)SPDK_COUNTOF(g_ios)) { ++ g_ios[g_num_ios++] = (struct ut_io) { ++ .write = write, .desc = desc, .offset_blocks = offset_blocks, ++ .num_blocks = num_blocks, .cb = cb, .cb_arg = cb_arg, ++ }; ++ } ++} ++ + int + spdk_bdev_readv_blocks_ext(struct spdk_bdev_desc *desc, + struct spdk_io_channel *ch, + struct iovec *iov, int iovcnt, uint64_t offset_blocks, uint64_t num_blocks, + spdk_bdev_io_completion_cb cb, void *cb_arg, struct spdk_bdev_ext_io_opts *opts) + { +- g_last_io_desc = desc; +- g_last_io_cb = cb; ++ ut_record_io(false, desc, offset_blocks, num_blocks, cb, cb_arg); + + return 0; + } +@@ -56,12 +78,34 @@ spdk_bdev_writev_blocks_ext(struct spdk_bdev_desc *desc, + struct iovec *iov, int iovcnt, uint64_t offset_blocks, uint64_t num_blocks, + spdk_bdev_io_completion_cb cb, void *cb_arg, struct spdk_bdev_ext_io_opts *opts) + { +- g_last_io_desc = desc; +- g_last_io_cb = cb; ++ ut_record_io(true, desc, offset_blocks, num_blocks, cb, cb_arg); + + return 0; + } + ++/* raid1 inits a raid_io only for a process request, whose target names the raid: ++ * enough to init it for real, so the copy it submits runs through the module. */ ++void ++raid_bdev_io_init(struct raid_bdev_io *raid_io, struct raid_bdev_io_channel *raid_ch, ++ enum spdk_bdev_io_type type, uint64_t offset_blocks, ++ uint64_t num_blocks, struct iovec *iovs, int iovcnt, void *md_buf, ++ struct spdk_memory_domain *memory_domain, void *memory_domain_ctx) ++{ ++ struct raid_bdev_process_request *process_req = ++ SPDK_CONTAINEROF(raid_io, struct raid_bdev_process_request, raid_io); ++ ++ raid_test_bdev_io_init(raid_io, process_req->target->raid_bdev, raid_ch, type, ++ offset_blocks, num_blocks, iovs, iovcnt, md_buf); ++} ++ ++static uint32_t g_process_max_part = UINT32_MAX; ++ ++uint32_t ++raid_bdev_process_request_max_part(const struct raid_bdev_process_request *process_req) ++{ ++ return g_process_max_part; ++} ++ + void + raid_bdev_fail_base_bdev(struct raid_base_bdev_info *base_info) + { +@@ -274,6 +318,118 @@ test_raid1_read_balancing(void) + run_for_each_raid1_config(_test_raid1_read_balancing); + } + ++static int ++ut_member_of(struct raid_bdev *raid_bdev, struct spdk_bdev_desc *desc) ++{ ++ uint8_t i; ++ ++ for (i = 0; i < raid_bdev->num_base_bdevs; i++) { ++ if (raid_bdev->base_bdev_info[i].desc == desc) { ++ return i; ++ } ++ } ++ return -1; ++} ++ ++/* Evariops 0047: a rebuild window is copied in parts, all in flight together, and ++ * the parts' reads spread over the members in sync. Copied as ONE request, the ++ * window was read whole from the first member before any of it was written: read ++ * and write never overlapped, and on a 3-node bench every byte of a 5 GiB rebuild crossed ++ * the link from a remote member (36 MiB/s) while the nexus's local member idled. ++ * The target is the last member: the process channel has no read channel for it. */ ++static void ++_test_raid1_process_window_in_parts(struct raid_bdev *raid_bdev, struct raid_bdev_io_channel *raid_ch) ++{ ++ struct raid1_io_channel *raid1_ch = raid_bdev_channel_get_module_ctx(raid_ch); ++ struct raid_bdev_process_request reqs[4] = {}; ++ const uint8_t target = raid_bdev->num_base_bdevs - 1; ++ const uint64_t window = 64; ++ uint32_t served[8] = {}; ++ uint64_t offset = 0; ++ int num_reqs = 0, num_reads, i; ++ ++ if (raid_bdev->bdev.blockcnt < window) { ++ return; ++ } ++ ++ g_process_max_part = window / SPDK_COUNTOF(reqs); ++ g_num_ios = 0; ++ raid_ch->_base_channels[target] = NULL; ++ ++ /* As the generic layer offers it: the rest of the window, to the next request. */ ++ while (offset < window && num_reqs < (int)SPDK_COUNTOF(reqs)) { ++ struct raid_bdev_process_request *req = &reqs[num_reqs++]; ++ int ret; ++ ++ req->target = &raid_bdev->base_bdev_info[target]; ++ req->target_ch = (void *)1; ++ req->offset_blocks = offset; ++ req->num_blocks = window - offset; ++ req->iov.iov_len = req->num_blocks * raid_bdev->bdev.blocklen; ++ ++ ret = raid1_submit_process_request(req, raid_ch); ++ CU_ASSERT(ret > 0 && (uint32_t)ret <= g_process_max_part); ++ CU_ASSERT(req->num_blocks == (uint32_t)ret); ++ CU_ASSERT(req->iov.iov_len == (uint64_t)ret * raid_bdev->bdev.blocklen); ++ if (ret <= 0) { ++ break; ++ } ++ offset += ret; ++ } ++ CU_ASSERT(offset == window); ++ CU_ASSERT(num_reqs == (int)SPDK_COUNTOF(reqs)); ++ ++ /* Every part is read before any completes, each from a member in sync, and the ++ * reads tile the window; members in sync serve as many parts each. */ ++ num_reads = g_num_ios; ++ CU_ASSERT(num_reads == num_reqs); ++ offset = 0; ++ for (i = 0; i < num_reads; i++) { ++ int member = ut_member_of(raid_bdev, g_ios[i].desc); ++ ++ CU_ASSERT(!g_ios[i].write); ++ CU_ASSERT(member >= 0 && member != target); ++ CU_ASSERT(g_ios[i].offset_blocks == offset); ++ offset += g_ios[i].num_blocks; ++ if (member >= 0) { ++ served[member]++; ++ } ++ } ++ CU_ASSERT(offset == window); ++ for (i = 0; i < target; i++) { ++ CU_ASSERT(served[i] == (uint32_t)num_reqs / target); ++ } ++ ++ /* Each part's read completing writes that part to the target, and only it. */ ++ for (i = 0; i < num_reads; i++) { ++ struct spdk_bdev_io bdev_io = {}; ++ ++ g_ios[i].cb(&bdev_io, true, g_ios[i].cb_arg); ++ CU_ASSERT(g_num_ios == num_reads + i + 1); ++ CU_ASSERT(g_ios[num_reads + i].write); ++ CU_ASSERT(ut_member_of(raid_bdev, g_ios[num_reads + i].desc) == target); ++ CU_ASSERT(g_ios[num_reads + i].offset_blocks == g_ios[i].offset_blocks); ++ CU_ASSERT(g_ios[num_reads + i].num_blocks == g_ios[i].num_blocks); ++ } ++ for (i = num_reads; i < g_num_ios; i++) { ++ struct spdk_bdev_io bdev_io = {}; ++ ++ g_ios[i].cb(&bdev_io, true, g_ios[i].cb_arg); ++ } ++ for (i = 0; i < raid_bdev->num_base_bdevs; i++) { ++ CU_ASSERT(raid1_ch->read_blocks_outstanding[i] == 0); ++ } ++ ++ raid_ch->_base_channels[target] = (void *)1; ++ g_process_max_part = UINT32_MAX; ++} ++ ++static void ++test_raid1_process_window_in_parts(void) ++{ ++ run_for_each_raid1_config(_test_raid1_process_window_in_parts); ++} ++ + static void + _test_raid1_write_error(struct raid_bdev *raid_bdev, struct raid_bdev_io_channel *raid_ch) + { +@@ -572,6 +728,7 @@ main(int argc, char **argv) + suite = CU_add_suite("raid1", test_setup, test_cleanup); + CU_ADD_TEST(suite, test_raid1_start); + CU_ADD_TEST(suite, test_raid1_read_balancing); ++ CU_ADD_TEST(suite, test_raid1_process_window_in_parts); + CU_ADD_TEST(suite, test_raid1_write_error); + CU_ADD_TEST(suite, test_raid1_write_capacity_exceeded); + CU_ADD_TEST(suite, test_raid1_read_error); diff --git a/patches/README.md b/patches/README.md index 4e5d631..561f704 100644 --- a/patches/README.md +++ b/patches/README.md @@ -4,7 +4,7 @@ Out-of-tree patches applied on top of upstream SPDK during the container build ( ## Application order -Patches are applied in **lexicographic order of filename** (`0001` … `0044`) — the Dockerfile globs `patches/*.patch` and `git apply`s each. The numeric prefix IS the contract; do not rely on any other ordering. Order matters: +Patches are applied in **lexicographic order of filename** (`0001` … `0047`) — the Dockerfile globs `patches/*.patch` and `git apply`s each. The numeric prefix IS the contract; do not rely on any other ordering. Order matters: | # | Patch | Touches | Depends on | |--:|:------|:--------|:-----------| @@ -52,6 +52,7 @@ Patches are applied in **lexicographic order of filename** (`0001` … `0044`) | 0044 | a thin cluster is cleared before use — a cluster allocated by a write (or write_zeroes, or an inflate) that covers only part of it is zeroed before the blob sees it. Its unwritten part returned what the device held there: another blob's data wherever the device's unmap does not zero (the free path counts on unmap to), and different bytes on each leg of a mirror, which a verify of the mirror reads as a content divergence. A write that covers the whole cluster skips the clear, so a rebuild or a copy does not write twice. `blob_ut` pins both, and its thin-provisioning I/O accounting now counts the cleared cluster | lib/blob (blobstore.c), test/unit/lib/blob | — | | 0045 | a seeded rebuild copies its seed ranges, and only under the windows they need — a seeded process jumps over what its ranges leave clean with no window (only a copy needs a window's quiesce, and the seeded target takes writes on either side of the process offset), and a window copies every range part it holds under its one lock. A window that a range touched used to be copied whole (scattered writes: 5 GiB re-copied for 395 MiB of dirty chunks), and every clean window was locked and skipped in turn (262144 cycles on a 1 TiB volume, whatever its delta). `bdev_raid_ut` pins the blocks copied and the windows locked | bdev_raid, test/unit/lib/bdev/raid | 0013 | | 0046 | a seeded rebuild reads its ranges from the CBT epoch — `bdev_raid_start_seeded_rebuild_from_epoch {name, base_bdev, cbt_bdev, epoch_id}` seeds the rebuild of `bdev_raid_start_seeded_rebuild` with the dirty ranges of a frozen epoch, read in-process through `vbdev_cbt_query_epoch_ranges` (up to 65536; more is -E2BIG, the caller rebuilds in full). A request carries ~200 ranges (1024 parsed values, a 32 KiB receive buffer): a scattered delta folded to fit recopied every clean block in between, 3.2 GiB for 4485 dirty ranges (368 MiB), measured. Both RPCs share their checks. `bdev_raid_ut` fakes the query | bdev_raid (rpc), module/bdev/cbt (query), test/unit/lib/bdev/raid | 0013, 0045, cbt module | +| 0047 | a raid1 rebuild copies its window in parts — `raid_bdev_process_request_max_part` offers a window to the process's requests in `RAID_BDEV_PROCESS_MAX_QD` parts (whole write units), and raid1 copies one part per request, so a window's parts are in flight together: each part's write starts as its own read completes, and the reads spread over the members in sync. Copied as one request, a window was read whole from one member, then written whole: read and write never overlapped, and the first member served every read — a 5 GiB rebuild ran at 36 MiB/s, all of it read across the link from a remote member while the nexus's local member idled. raid5f, bound to its stripe, ignores the part. `raid1_ut` pins the parts, their spread and their writes; `bdev_raid_ut` the part's bounds | bdev_raid, raid1, test/unit/lib/bdev/raid | 0013 | 0005 `#include`s `vbdev_tier.h` and adds `-I module/bdev/tier` to the lvol module CFLAGS via its own Makefile hunk; the Dockerfile injects the module dirs before applying patches (copy-before-apply ordering matters). From 3ec19eec2a94ecbb889eeeaace47e944e925d8a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20DUCOM?= Date: Thu, 1 Oct 2026 06:31:05 +0200 Subject: [PATCH 07/18] fix(raid1): a rebuild copies its window in parts, in flight together (0047) raid_bdev_process_request_max_part offers a window to the process's requests in RAID_BDEV_PROCESS_MAX_QD parts, whole write units, and raid1 copies one part per request. A window's parts are now in flight together: each part's write starts as its own read completes, and the reads spread over the members in sync by their outstanding blocks. Copied as one request, a window was read whole from one member, then written whole: read and write never overlapped, and the first member served every read. Measured on a 3-node bench, a full rebuild of a 5 GiB mirror3 under a verified write load ran at 36 MiB/s for 141 s, every byte read across the link from a remote member while the nexus's local member served none. raid5f, bound to its stripe, does not call it; the seeded copy already takes what the module returns and offers the rest to the next request. --- ...1-rebuild-copies-its-window-in-parts.patch | 33 +++++++++++++++++-- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/patches/0047-raid1-rebuild-copies-its-window-in-parts.patch b/patches/0047-raid1-rebuild-copies-its-window-in-parts.patch index d469caf..c0d04b6 100644 --- a/patches/0047-raid1-rebuild-copies-its-window-in-parts.patch +++ b/patches/0047-raid1-rebuild-copies-its-window-in-parts.patch @@ -1,15 +1,24 @@ diff --git a/module/bdev/raid/bdev_raid.c b/module/bdev/raid/bdev_raid.c -index 583707e..d34fbf9 100644 +index 583707e..da919f1 100644 --- a/module/bdev/raid/bdev_raid.c +++ b/module/bdev/raid/bdev_raid.c -@@ -3266,6 +3266,12 @@ raid_bdev_process_request_complete(struct raid_bdev_process_request *process_req +@@ -3266,6 +3266,21 @@ raid_bdev_process_request_complete(struct raid_bdev_process_request *process_req } } ++/* Evariops 0047: a window is offered to its requests in RAID_BDEV_PROCESS_MAX_QD ++ * parts, so all of them are in flight together: each part's write starts as its ++ * own read completes, and the reads spread over the members that serve them. A ++ * window copied as one request was read whole, then written whole — read and ++ * write never overlapped, and one member served every read. */ +uint32_t +raid_bdev_process_request_max_part(const struct raid_bdev_process_request *process_req) +{ -+ return process_req->process->max_window_size; ++ const struct raid_bdev_process *process = process_req->process; ++ const uint64_t write_unit = spdk_max(process->raid_bdev->bdev.write_unit_size, 1); ++ const uint64_t part = spdk_divide_round_up(process->max_window_size, RAID_BDEV_PROCESS_MAX_QD); ++ ++ return spdk_divide_round_up(part, write_unit) * write_unit; +} + static int @@ -30,6 +39,24 @@ index 280d61c..f584d19 100644 void raid_bdev_io_init(struct raid_bdev_io *raid_io, struct raid_bdev_io_channel *raid_ch, enum spdk_bdev_io_type type, uint64_t offset_blocks, uint64_t num_blocks, struct iovec *iovs, int iovcnt, void *md_buf, +diff --git a/module/bdev/raid/raid1.c b/module/bdev/raid/raid1.c +index f1d4f0f..51efb51 100644 +--- a/module/bdev/raid/raid1.c ++++ b/module/bdev/raid/raid1.c +@@ -590,6 +590,13 @@ raid1_submit_process_request(struct raid_bdev_process_request *process_req, + struct raid_bdev_io *raid_io = &process_req->raid_io; + int ret; + ++ /* Evariops 0047: one part of the window, not the window whole: the next ++ * requests take the rest, in flight beside this one. */ ++ process_req->num_blocks = spdk_min(process_req->num_blocks, ++ raid_bdev_process_request_max_part(process_req)); ++ process_req->iov.iov_len = (size_t)process_req->num_blocks * ++ process_req->target->raid_bdev->bdev.blocklen; ++ + raid_bdev_io_init(raid_io, raid_ch, SPDK_BDEV_IO_TYPE_READ, + process_req->offset_blocks, process_req->num_blocks, + &process_req->iov, 1, process_req->md_buf, NULL, NULL); diff --git a/test/unit/lib/bdev/raid/bdev_raid.c/bdev_raid_ut.c b/test/unit/lib/bdev/raid/bdev_raid.c/bdev_raid_ut.c index 11b46f2..a488d6d 100644 --- a/test/unit/lib/bdev/raid/bdev_raid.c/bdev_raid_ut.c From e961ea2d0368990984092ff0876bf8036f195eab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20DUCOM?= Date: Thu, 1 Oct 2026 07:49:40 +0200 Subject: [PATCH 08/18] test(raid1): a rebuild writes a zero part as write-zeroes (0048, tests only) raid1_ut, red on the series as it stands: four parts of a rebuild window, two read back holding data and two holding nothing but zeroes. With a target that takes write-zeroes, the data parts are written as data and the zero parts as write-zeroes; with a target that cannot, every part is written as data. The zero parts go out as data writes. The window-in-parts test (0047) now gives its requests real buffers: the copy reads a part back before choosing how to write it. The suite records the write-zeroes it is sent, and the write-zeroes support of the members is a knob of the suite. --- ...d-writes-a-zero-part-as-write-zeroes.patch | 156 ++++++++++++++++++ patches/README.md | 3 +- 2 files changed, 158 insertions(+), 1 deletion(-) create mode 100644 patches/0048-raid1-rebuild-writes-a-zero-part-as-write-zeroes.patch diff --git a/patches/0048-raid1-rebuild-writes-a-zero-part-as-write-zeroes.patch b/patches/0048-raid1-rebuild-writes-a-zero-part-as-write-zeroes.patch new file mode 100644 index 0000000..51d5d0d --- /dev/null +++ b/patches/0048-raid1-rebuild-writes-a-zero-part-as-write-zeroes.patch @@ -0,0 +1,156 @@ +diff --git a/test/unit/lib/bdev/raid/raid1.c/raid1_ut.c b/test/unit/lib/bdev/raid/raid1.c/raid1_ut.c +index 88f4295..cad171b 100644 +--- a/test/unit/lib/bdev/raid/raid1.c/raid1_ut.c ++++ b/test/unit/lib/bdev/raid/raid1.c/raid1_ut.c +@@ -37,6 +37,7 @@ DEFINE_STUB(spdk_bdev_unmap_blocks, int, (struct spdk_bdev_desc *desc, struct sp + * member served a read and where a write landed, and completes each in turn. */ + struct ut_io { + bool write; ++ bool zeroes; + struct spdk_bdev_desc *desc; + uint64_t offset_blocks; + uint64_t num_blocks; +@@ -83,6 +84,26 @@ spdk_bdev_writev_blocks_ext(struct spdk_bdev_desc *desc, + return 0; + } + ++/* Evariops 0048: whether the members take write-zeroes, and the write-zeroes they were sent. */ ++static bool g_write_zeroes_supported = true; ++ ++bool ++spdk_bdev_io_type_supported(struct spdk_bdev *bdev, enum spdk_bdev_io_type io_type) ++{ ++ return io_type == SPDK_BDEV_IO_TYPE_WRITE_ZEROES ? g_write_zeroes_supported : true; ++} ++ ++int ++spdk_bdev_write_zeroes_blocks(struct spdk_bdev_desc *desc, struct spdk_io_channel *ch, ++ uint64_t offset_blocks, uint64_t num_blocks, ++ spdk_bdev_io_completion_cb cb, void *cb_arg) ++{ ++ ut_record_io(true, desc, offset_blocks, num_blocks, cb, cb_arg); ++ g_ios[g_num_ios - 1].zeroes = true; ++ ++ return 0; ++} ++ + /* raid1 inits a raid_io only for a process request, whose target names the raid: + * enough to init it for real, so the copy it submits runs through the module. */ + void +@@ -361,6 +382,9 @@ _test_raid1_process_window_in_parts(struct raid_bdev *raid_bdev, struct raid_bde + struct raid_bdev_process_request *req = &reqs[num_reqs++]; + int ret; + ++ /* A real buffer: the copy reads it back before choosing how to write it (0048). */ ++ req->iov.iov_base = calloc(1, window * raid_bdev->bdev.blocklen); ++ SPDK_CU_ASSERT_FATAL(req->iov.iov_base != NULL); + req->target = &raid_bdev->base_bdev_info[target]; + req->target_ch = (void *)1; + req->offset_blocks = offset; +@@ -419,6 +443,9 @@ _test_raid1_process_window_in_parts(struct raid_bdev *raid_bdev, struct raid_bde + for (i = 0; i < raid_bdev->num_base_bdevs; i++) { + CU_ASSERT(raid1_ch->read_blocks_outstanding[i] == 0); + } ++ for (i = 0; i < num_reqs; i++) { ++ free(reqs[i].iov.iov_base); ++ } + + raid_ch->_base_channels[target] = (void *)1; + g_process_max_part = UINT32_MAX; +@@ -430,6 +457,88 @@ test_raid1_process_window_in_parts(void) + run_for_each_raid1_config(_test_raid1_process_window_in_parts); + } + ++/* Evariops 0048: a rebuild part the source read back as nothing but zeroes is written to the ++ * target as write-zeroes, so no payload crosses the link for it; a part holding data is written ++ * as data. A target that cannot take write-zeroes gets every part as data. */ ++static void ++_test_raid1_process_zero_part_written_as_zeroes(struct raid_bdev *raid_bdev, struct raid_bdev_io_channel *raid_ch) ++{ ++ struct raid_bdev_process_request reqs[4]; ++ const uint8_t target = raid_bdev->num_base_bdevs - 1; ++ const uint64_t window = 64; ++ const uint64_t part = window / SPDK_COUNTOF(reqs); ++ const size_t part_len = part * raid_bdev->bdev.blocklen; ++ int pass, i; ++ ++ if (raid_bdev->bdev.blockcnt < window) { ++ return; ++ } ++ ++ for (pass = 0; pass < 2; pass++) { ++ const bool supported = pass == 0; ++ ++ g_write_zeroes_supported = supported; ++ g_process_max_part = part; ++ g_num_ios = 0; ++ raid_ch->_base_channels[target] = NULL; ++ ++ for (i = 0; i < (int)SPDK_COUNTOF(reqs); i++) { ++ struct raid_bdev_process_request *req = &reqs[i]; ++ ++ memset(req, 0, sizeof(*req)); ++ req->iov.iov_base = calloc(1, window * raid_bdev->bdev.blocklen); ++ SPDK_CU_ASSERT_FATAL(req->iov.iov_base != NULL); ++ req->target = &raid_bdev->base_bdev_info[target]; ++ req->target_ch = (void *)1; ++ req->offset_blocks = i * part; ++ req->num_blocks = window - i * part; ++ req->iov.iov_len = req->num_blocks * raid_bdev->bdev.blocklen; ++ CU_ASSERT(raid1_submit_process_request(req, raid_ch) == (int)part); ++ } ++ CU_ASSERT(g_num_ios == (int)SPDK_COUNTOF(reqs)); ++ ++ /* Parts 0 and 2 read data back, parts 1 and 3 nothing but zeroes. */ ++ for (i = 0; i < (int)SPDK_COUNTOF(reqs); i++) { ++ struct spdk_bdev_io bdev_io = {}; ++ ++ if (i % 2 == 0) { ++ memset(reqs[i].iov.iov_base, 0xa5, part_len); ++ } ++ g_ios[i].cb(&bdev_io, true, g_ios[i].cb_arg); ++ } ++ ++ CU_ASSERT(g_num_ios == 2 * (int)SPDK_COUNTOF(reqs)); ++ for (i = 0; i < (int)SPDK_COUNTOF(reqs); i++) { ++ struct ut_io *written = &g_ios[SPDK_COUNTOF(reqs) + i]; ++ ++ CU_ASSERT(written->write); ++ CU_ASSERT(ut_member_of(raid_bdev, written->desc) == target); ++ CU_ASSERT(written->offset_blocks == i * part); ++ CU_ASSERT(written->num_blocks == part); ++ CU_ASSERT(written->zeroes == (supported && i % 2 == 1)); ++ } ++ ++ for (i = SPDK_COUNTOF(reqs); i < g_num_ios; i++) { ++ struct spdk_bdev_io bdev_io = {}; ++ ++ g_ios[i].cb(&bdev_io, true, g_ios[i].cb_arg); ++ } ++ for (i = 0; i < (int)SPDK_COUNTOF(reqs); i++) { ++ free(reqs[i].iov.iov_base); ++ } ++ raid_ch->_base_channels[target] = (void *)1; ++ } ++ ++ g_write_zeroes_supported = true; ++ g_process_max_part = UINT32_MAX; ++} ++ ++static void ++test_raid1_process_zero_part_written_as_zeroes(void) ++{ ++ run_for_each_raid1_config(_test_raid1_process_zero_part_written_as_zeroes); ++} ++ + static void + _test_raid1_write_error(struct raid_bdev *raid_bdev, struct raid_bdev_io_channel *raid_ch) + { +@@ -729,6 +838,7 @@ main(int argc, char **argv) + CU_ADD_TEST(suite, test_raid1_start); + CU_ADD_TEST(suite, test_raid1_read_balancing); + CU_ADD_TEST(suite, test_raid1_process_window_in_parts); ++ CU_ADD_TEST(suite, test_raid1_process_zero_part_written_as_zeroes); + CU_ADD_TEST(suite, test_raid1_write_error); + CU_ADD_TEST(suite, test_raid1_write_capacity_exceeded); + CU_ADD_TEST(suite, test_raid1_read_error); diff --git a/patches/README.md b/patches/README.md index 561f704..13772c3 100644 --- a/patches/README.md +++ b/patches/README.md @@ -4,7 +4,7 @@ Out-of-tree patches applied on top of upstream SPDK during the container build ( ## Application order -Patches are applied in **lexicographic order of filename** (`0001` … `0047`) — the Dockerfile globs `patches/*.patch` and `git apply`s each. The numeric prefix IS the contract; do not rely on any other ordering. Order matters: +Patches are applied in **lexicographic order of filename** (`0001` … `0048`) — the Dockerfile globs `patches/*.patch` and `git apply`s each. The numeric prefix IS the contract; do not rely on any other ordering. Order matters: | # | Patch | Touches | Depends on | |--:|:------|:--------|:-----------| @@ -53,6 +53,7 @@ Patches are applied in **lexicographic order of filename** (`0001` … `0047`) | 0045 | a seeded rebuild copies its seed ranges, and only under the windows they need — a seeded process jumps over what its ranges leave clean with no window (only a copy needs a window's quiesce, and the seeded target takes writes on either side of the process offset), and a window copies every range part it holds under its one lock. A window that a range touched used to be copied whole (scattered writes: 5 GiB re-copied for 395 MiB of dirty chunks), and every clean window was locked and skipped in turn (262144 cycles on a 1 TiB volume, whatever its delta). `bdev_raid_ut` pins the blocks copied and the windows locked | bdev_raid, test/unit/lib/bdev/raid | 0013 | | 0046 | a seeded rebuild reads its ranges from the CBT epoch — `bdev_raid_start_seeded_rebuild_from_epoch {name, base_bdev, cbt_bdev, epoch_id}` seeds the rebuild of `bdev_raid_start_seeded_rebuild` with the dirty ranges of a frozen epoch, read in-process through `vbdev_cbt_query_epoch_ranges` (up to 65536; more is -E2BIG, the caller rebuilds in full). A request carries ~200 ranges (1024 parsed values, a 32 KiB receive buffer): a scattered delta folded to fit recopied every clean block in between, 3.2 GiB for 4485 dirty ranges (368 MiB), measured. Both RPCs share their checks. `bdev_raid_ut` fakes the query | bdev_raid (rpc), module/bdev/cbt (query), test/unit/lib/bdev/raid | 0013, 0045, cbt module | | 0047 | a raid1 rebuild copies its window in parts — `raid_bdev_process_request_max_part` offers a window to the process's requests in `RAID_BDEV_PROCESS_MAX_QD` parts (whole write units), and raid1 copies one part per request, so a window's parts are in flight together: each part's write starts as its own read completes, and the reads spread over the members in sync. Copied as one request, a window was read whole from one member, then written whole: read and write never overlapped, and the first member served every read — a 5 GiB rebuild ran at 36 MiB/s, all of it read across the link from a remote member while the nexus's local member idled. raid5f, bound to its stripe, ignores the part. `raid1_ut` pins the parts, their spread and their writes; `bdev_raid_ut` the part's bounds | bdev_raid, raid1, test/unit/lib/bdev/raid | 0013 | +| 0048 | a raid1 rebuild writes a zero part as write-zeroes — a part the source read back as nothing but zeroes goes to the target as `write_zeroes` (`raid_bdev_write_zeroes_blocks`): the same blocks, read back the same, and no payload on the wire, so a full rebuild of a sparse volume no longer ships its never-written clusters across the link (about 3 of the 5 GiB of the rebuild measured for 0047). Not when the target cannot take write-zeroes (the bdev layer would send a zero buffer anyway), nor when the blocks carry metadata. `raid1_ut` pins both kinds of write, and a target without write-zeroes getting data | raid1, bdev_raid (header), test/unit/lib/bdev/raid | 0047 | 0005 `#include`s `vbdev_tier.h` and adds `-I module/bdev/tier` to the lvol module CFLAGS via its own Makefile hunk; the Dockerfile injects the module dirs before applying patches (copy-before-apply ordering matters). From 8ed1d2a872f7dcfdc56197458a8ece9d060529df Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20DUCOM?= Date: Thu, 1 Oct 2026 07:49:57 +0200 Subject: [PATCH 09/18] fix(raid1): a rebuild writes a zero part as write-zeroes (0048) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A part the source read back as nothing but zeroes goes to the target as write_zeroes (raid_bdev_write_zeroes_blocks, the data offset applied like the other wrappers): the same blocks, read back the same, and no payload on the wire. A full rebuild of a sparse volume no longer ships its never-written clusters across the link — measured on a 3-node bench, about 3 of the 5 GiB of a mirror3 full rebuild, all of it zero, all of it through the nexus node's egress, which 0047 had made the bound. Not when the target cannot take write-zeroes (the bdev layer would then send a zero buffer anyway), nor when the blocks carry metadata. The check reads the part already in memory, and a data part stops at its first nonzero word. --- ...d-writes-a-zero-part-as-write-zeroes.patch | 83 +++++++++++++++++++ 1 file changed, 83 insertions(+) diff --git a/patches/0048-raid1-rebuild-writes-a-zero-part-as-write-zeroes.patch b/patches/0048-raid1-rebuild-writes-a-zero-part-as-write-zeroes.patch index 51d5d0d..3b36cf4 100644 --- a/patches/0048-raid1-rebuild-writes-a-zero-part-as-write-zeroes.patch +++ b/patches/0048-raid1-rebuild-writes-a-zero-part-as-write-zeroes.patch @@ -1,3 +1,86 @@ +diff --git a/module/bdev/raid/bdev_raid.h b/module/bdev/raid/bdev_raid.h +index f584d19..947359c 100644 +--- a/module/bdev/raid/bdev_raid.h ++++ b/module/bdev/raid/bdev_raid.h +@@ -560,6 +560,18 @@ raid_bdev_unmap_blocks(struct raid_base_bdev_info *base_info, struct spdk_io_cha + num_blocks, cb, cb_arg); + } + ++/** ++ * Raid bdev I/O wrapper for spdk_bdev_write_zeroes_blocks function. ++ */ ++static inline int ++raid_bdev_write_zeroes_blocks(struct raid_base_bdev_info *base_info, struct spdk_io_channel *ch, ++ uint64_t offset_blocks, uint64_t num_blocks, ++ spdk_bdev_io_completion_cb cb, void *cb_arg) ++{ ++ return spdk_bdev_write_zeroes_blocks(base_info->desc, ch, base_info->data_offset + offset_blocks, ++ num_blocks, cb, cb_arg); ++} ++ + /** + * Raid bdev I/O read/write wrapper for spdk_bdev_flush_blocks function. + */ +diff --git a/module/bdev/raid/raid1.c b/module/bdev/raid/raid1.c +index 51efb51..388b9a9 100644 +--- a/module/bdev/raid/raid1.c ++++ b/module/bdev/raid/raid1.c +@@ -7,6 +7,7 @@ + + #include "spdk/likely.h" + #include "spdk/log.h" ++#include "spdk/string.h" /* Evariops 0048: spdk_mem_all_zero */ + #include "spdk/nvme_spec.h" /* Evariops 0009: CAPACITY_EXCEEDED discrimination */ + + struct raid1_info { +@@ -547,6 +548,24 @@ _raid1_process_submit_write(void *ctx) + raid1_process_submit_write(process_req); + } + ++/* Evariops 0048: a part the source read back as nothing but zeroes goes to the target as ++ * write-zeroes — the same blocks, read back the same, and no payload on the wire, so a full ++ * rebuild of a sparse volume no longer ships its never-written clusters across the link. Not ++ * when the target cannot take write-zeroes (the bdev layer would send a zero buffer anyway), nor ++ * when the blocks carry metadata. */ ++static bool ++raid1_process_part_is_zeroes(struct raid_bdev_process_request *process_req) ++{ ++ struct raid_base_bdev_info *target = process_req->target; ++ ++ if (target->raid_bdev->bdev.md_len != 0 || ++ !spdk_bdev_io_type_supported(spdk_bdev_desc_get_bdev(target->desc), SPDK_BDEV_IO_TYPE_WRITE_ZEROES)) { ++ return false; ++ } ++ ++ return spdk_mem_all_zero(process_req->iov.iov_base, process_req->iov.iov_len); ++} ++ + static void + raid1_process_submit_write(struct raid_bdev_process_request *process_req) + { +@@ -554,11 +573,17 @@ raid1_process_submit_write(struct raid_bdev_process_request *process_req) + struct spdk_bdev_ext_io_opts io_opts; + int ret; + +- raid1_init_ext_io_opts(&io_opts, raid_io); +- ret = raid_bdev_writev_blocks_ext(process_req->target, process_req->target_ch, +- raid_io->iovs, raid_io->iovcnt, +- raid_io->offset_blocks, raid_io->num_blocks, +- raid1_process_write_completed, process_req, &io_opts); ++ if (raid1_process_part_is_zeroes(process_req)) { ++ ret = raid_bdev_write_zeroes_blocks(process_req->target, process_req->target_ch, ++ raid_io->offset_blocks, raid_io->num_blocks, ++ raid1_process_write_completed, process_req); ++ } else { ++ raid1_init_ext_io_opts(&io_opts, raid_io); ++ ret = raid_bdev_writev_blocks_ext(process_req->target, process_req->target_ch, ++ raid_io->iovs, raid_io->iovcnt, ++ raid_io->offset_blocks, raid_io->num_blocks, ++ raid1_process_write_completed, process_req, &io_opts); ++ } + if (spdk_unlikely(ret != 0)) { + if (ret == -ENOMEM) { + raid_bdev_queue_io_wait(raid_io, spdk_bdev_desc_get_bdev(process_req->target->desc), diff --git a/test/unit/lib/bdev/raid/raid1.c/raid1_ut.c b/test/unit/lib/bdev/raid/raid1.c/raid1_ut.c index 88f4295..cad171b 100644 --- a/test/unit/lib/bdev/raid/raid1.c/raid1_ut.c From fe0903e66b997c9bc85448e206550f481951627a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20DUCOM?= Date: Thu, 1 Oct 2026 08:24:19 +0200 Subject: [PATCH 10/18] test(blob): write-zeroes leaves an unallocated thin cluster unallocated (0049, tests only) blob_ut, red on the series as it stands: write-zeroes over half of a cluster a thin blob has not allocated allocates the cluster and writes to the device; it is to allocate nothing, write nothing, and read back zeroes. The same test pins what stays: write-zeroes over an allocated cluster holding data zeroes what it covers, and on a clone, whose unallocated cluster reads its parent's data, it allocates the cluster and zeroes it. --- ...unallocated-thin-cluster-unallocated.patch | 119 ++++++++++++++++++ patches/README.md | 3 +- 2 files changed, 121 insertions(+), 1 deletion(-) create mode 100644 patches/0049-blob-write-zeroes-leaves-an-unallocated-thin-cluster-unallocated.patch diff --git a/patches/0049-blob-write-zeroes-leaves-an-unallocated-thin-cluster-unallocated.patch b/patches/0049-blob-write-zeroes-leaves-an-unallocated-thin-cluster-unallocated.patch new file mode 100644 index 0000000..c603d81 --- /dev/null +++ b/patches/0049-blob-write-zeroes-leaves-an-unallocated-thin-cluster-unallocated.patch @@ -0,0 +1,119 @@ +diff --git a/test/unit/lib/blob/blob.c/blob_ut.c b/test/unit/lib/blob/blob.c/blob_ut.c +index 3bb3f5c..cdf55b1 100644 +--- a/test/unit/lib/blob/blob.c/blob_ut.c ++++ b/test/unit/lib/blob/blob.c/blob_ut.c +@@ -4944,6 +4944,106 @@ blob_thin_prov_full_cluster_write_is_not_cleared_first(void) + g_blobid = 0; + } + ++/* ++ * Evariops 0049: write-zeroes on a cluster that a thin blob backed by zeroes has not allocated ++ * changes nothing it reads, so it allocates nothing and writes nothing. A full rebuild sends every ++ * never-written part of its source as write-zeroes (0048): allocating the cluster of each made the ++ * rebuilt replica fully allocated, and wrote zeroes across the target's disk. An allocated cluster ++ * is still zeroed, and a clone, which reads its unallocated clusters from its parent, still ++ * allocates and zeroes. ++ */ ++static void ++blob_thin_prov_write_zeroes_on_unallocated_cluster_allocates_nothing(void) ++{ ++ struct spdk_blob_store *bs = g_bs; ++ struct spdk_blob *blob, *clone; ++ struct spdk_io_channel *channel; ++ struct spdk_blob_opts opts; ++ uint64_t io_unit_size = spdk_bs_get_io_unit_size(bs); ++ uint64_t cluster_size = spdk_bs_get_cluster_size(bs); ++ uint64_t io_units_per_cluster = cluster_size / io_unit_size; ++ uint64_t half = io_units_per_cluster / 2; ++ uint64_t write_bytes; ++ spdk_blob_id snapshotid; ++ uint8_t *payload; ++ ++ channel = spdk_bs_alloc_io_channel(bs); ++ SPDK_CU_ASSERT_FATAL(channel != NULL); ++ payload = malloc(cluster_size); ++ SPDK_CU_ASSERT_FATAL(payload != NULL); ++ ++ ut_fill_free_clusters(bs, 0xAA); ++ ++ ut_spdk_blob_opts_init(&opts); ++ opts.thin_provision = true; ++ opts.num_clusters = 2; ++ blob = ut_blob_create_and_open(bs, &opts); ++ ++ /* Half of cluster 0, never written: nothing allocated, nothing written, zeroes read back. */ ++ write_bytes = g_dev_write_bytes; ++ spdk_blob_io_write_zeroes(blob, channel, 0, half, blob_op_complete, NULL); ++ poll_threads(); ++ CU_ASSERT(g_bserrno == 0); ++ CU_ASSERT(spdk_blob_get_num_allocated_clusters(blob) == 0); ++ CU_ASSERT(g_dev_write_bytes == write_bytes); ++ memset(payload, 0xFF, cluster_size); ++ spdk_blob_io_read(blob, channel, payload, 0, io_units_per_cluster, blob_op_complete, NULL); ++ poll_threads(); ++ CU_ASSERT(g_bserrno == 0); ++ CU_ASSERT(spdk_mem_all_zero(payload, cluster_size)); ++ ++ /* An allocated cluster holding data: write-zeroes still zeroes what it covers. */ ++ memset(payload, 0xE5, cluster_size); ++ spdk_blob_io_write(blob, channel, payload, io_units_per_cluster, io_units_per_cluster, blob_op_complete, NULL); ++ poll_threads(); ++ CU_ASSERT(g_bserrno == 0); ++ CU_ASSERT(spdk_blob_get_num_allocated_clusters(blob) == 1); ++ spdk_blob_io_write_zeroes(blob, channel, io_units_per_cluster, half, blob_op_complete, NULL); ++ poll_threads(); ++ CU_ASSERT(g_bserrno == 0); ++ spdk_blob_io_read(blob, channel, payload, io_units_per_cluster, io_units_per_cluster, blob_op_complete, NULL); ++ poll_threads(); ++ CU_ASSERT(g_bserrno == 0); ++ CU_ASSERT(spdk_mem_all_zero(payload, cluster_size / 2)); ++ CU_ASSERT(payload[cluster_size / 2] == 0xE5 && payload[cluster_size - 1] == 0xE5); ++ ++ /* A clone reads its unallocated cluster 1 from the snapshot, whose second half holds data: ++ * write-zeroes there allocates the cluster and zeroes it. */ ++ spdk_bs_create_snapshot(bs, spdk_blob_get_id(blob), NULL, blob_op_with_id_complete, NULL); ++ poll_threads(); ++ CU_ASSERT(g_bserrno == 0); ++ snapshotid = g_blobid; ++ spdk_bs_create_clone(bs, snapshotid, NULL, blob_op_with_id_complete, NULL); ++ poll_threads(); ++ CU_ASSERT(g_bserrno == 0); ++ spdk_bs_open_blob(bs, g_blobid, blob_op_with_handle_complete, NULL); ++ poll_threads(); ++ CU_ASSERT(g_bserrno == 0); ++ SPDK_CU_ASSERT_FATAL(g_blob != NULL); ++ clone = g_blob; ++ ++ spdk_blob_io_write_zeroes(clone, channel, io_units_per_cluster + half, half, blob_op_complete, NULL); ++ poll_threads(); ++ CU_ASSERT(g_bserrno == 0); ++ CU_ASSERT(spdk_blob_get_num_allocated_clusters(clone) == 1); ++ memset(payload, 0xFF, cluster_size); ++ spdk_blob_io_read(clone, channel, payload, io_units_per_cluster, io_units_per_cluster, blob_op_complete, NULL); ++ poll_threads(); ++ CU_ASSERT(g_bserrno == 0); ++ CU_ASSERT(spdk_mem_all_zero(payload, cluster_size)); ++ ++ free(payload); ++ ut_blob_close_and_delete(bs, clone); ++ ut_blob_close_and_delete(bs, blob); ++ spdk_bs_delete_blob(bs, snapshotid, blob_op_complete, NULL); ++ poll_threads(); ++ CU_ASSERT(g_bserrno == 0); ++ spdk_bs_free_io_channel(channel); ++ poll_threads(); ++ g_blob = NULL; ++ g_blobid = 0; ++} ++ + static void + blob_thin_prov_unmap_cluster(void) + { +@@ -10501,6 +10601,7 @@ main(int argc, char **argv) + CU_ADD_TEST(suite, blob_thin_prov_write_count_io); + CU_ADD_TEST(suite_bs, blob_thin_prov_new_cluster_reads_zeroes); + CU_ADD_TEST(suite_bs, blob_thin_prov_full_cluster_write_is_not_cleared_first); ++ CU_ADD_TEST(suite_bs, blob_thin_prov_write_zeroes_on_unallocated_cluster_allocates_nothing); + CU_ADD_TEST(suite, blob_thin_prov_unmap_cluster); + CU_ADD_TEST(suite_bs, blob_thin_prov_rle); + CU_ADD_TEST(suite_bs, blob_thin_prov_rw_iov); diff --git a/patches/README.md b/patches/README.md index 13772c3..619c3cb 100644 --- a/patches/README.md +++ b/patches/README.md @@ -4,7 +4,7 @@ Out-of-tree patches applied on top of upstream SPDK during the container build ( ## Application order -Patches are applied in **lexicographic order of filename** (`0001` … `0048`) — the Dockerfile globs `patches/*.patch` and `git apply`s each. The numeric prefix IS the contract; do not rely on any other ordering. Order matters: +Patches are applied in **lexicographic order of filename** (`0001` … `0049`) — the Dockerfile globs `patches/*.patch` and `git apply`s each. The numeric prefix IS the contract; do not rely on any other ordering. Order matters: | # | Patch | Touches | Depends on | |--:|:------|:--------|:-----------| @@ -54,6 +54,7 @@ Patches are applied in **lexicographic order of filename** (`0001` … `0048`) | 0046 | a seeded rebuild reads its ranges from the CBT epoch — `bdev_raid_start_seeded_rebuild_from_epoch {name, base_bdev, cbt_bdev, epoch_id}` seeds the rebuild of `bdev_raid_start_seeded_rebuild` with the dirty ranges of a frozen epoch, read in-process through `vbdev_cbt_query_epoch_ranges` (up to 65536; more is -E2BIG, the caller rebuilds in full). A request carries ~200 ranges (1024 parsed values, a 32 KiB receive buffer): a scattered delta folded to fit recopied every clean block in between, 3.2 GiB for 4485 dirty ranges (368 MiB), measured. Both RPCs share their checks. `bdev_raid_ut` fakes the query | bdev_raid (rpc), module/bdev/cbt (query), test/unit/lib/bdev/raid | 0013, 0045, cbt module | | 0047 | a raid1 rebuild copies its window in parts — `raid_bdev_process_request_max_part` offers a window to the process's requests in `RAID_BDEV_PROCESS_MAX_QD` parts (whole write units), and raid1 copies one part per request, so a window's parts are in flight together: each part's write starts as its own read completes, and the reads spread over the members in sync. Copied as one request, a window was read whole from one member, then written whole: read and write never overlapped, and the first member served every read — a 5 GiB rebuild ran at 36 MiB/s, all of it read across the link from a remote member while the nexus's local member idled. raid5f, bound to its stripe, ignores the part. `raid1_ut` pins the parts, their spread and their writes; `bdev_raid_ut` the part's bounds | bdev_raid, raid1, test/unit/lib/bdev/raid | 0013 | | 0048 | a raid1 rebuild writes a zero part as write-zeroes — a part the source read back as nothing but zeroes goes to the target as `write_zeroes` (`raid_bdev_write_zeroes_blocks`): the same blocks, read back the same, and no payload on the wire, so a full rebuild of a sparse volume no longer ships its never-written clusters across the link (about 3 of the 5 GiB of the rebuild measured for 0047). Not when the target cannot take write-zeroes (the bdev layer would send a zero buffer anyway), nor when the blocks carry metadata. `raid1_ut` pins both kinds of write, and a target without write-zeroes getting data | raid1, bdev_raid (header), test/unit/lib/bdev/raid | 0047 | +| 0049 | write-zeroes leaves an unallocated thin cluster unallocated — on a blob backed by zeroes, an unallocated cluster already reads zeroes, so write-zeroes over it completes with no cluster allocated and nothing written. A full rebuild sends every never-written part of its source as write-zeroes (0048), and allocating each made the rebuilt replica fully allocated — a thin volume rebuilt in full took its whole size on the target — and wrote zeroes across the target's disk. An allocated cluster is still zeroed; a clone, which reads its unallocated clusters from its parent, still allocates and zeroes. `blob_ut` pins the three | lib/blob (blobstore.c), test/unit/lib/blob | 0044 | 0005 `#include`s `vbdev_tier.h` and adds `-I module/bdev/tier` to the lvol module CFLAGS via its own Makefile hunk; the Dockerfile injects the module dirs before applying patches (copy-before-apply ordering matters). From f8ba4e77781d50ba797fff4bef65f3a81c454443 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20DUCOM?= Date: Thu, 1 Oct 2026 08:24:37 +0200 Subject: [PATCH 11/18] fix(blob): write-zeroes leaves an unallocated thin cluster unallocated (0049) On a blob backed by zeroes, an unallocated cluster already reads zeroes: write-zeroes over it now completes at once, with no cluster allocated and nothing written. Before, it allocated the cluster (cleared, since 0044) and zeroed it. A full rebuild sends every never-written part of its source as write-zeroes (0048), so a thin volume rebuilt in full took its whole size on the target, and the zeroes it carried no longer on the wire still went to the target's disk: measured on a 3-node bench, the SATA disk of the rebuilt replica wrote 94 MiB/s through a 5 GiB rebuild with about 3 GiB never written. An allocated cluster is still zeroed. A clone or any blob with another back device reads its unallocated clusters from it, and keeps the allocation. --- ...unallocated-thin-cluster-unallocated.patch | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/patches/0049-blob-write-zeroes-leaves-an-unallocated-thin-cluster-unallocated.patch b/patches/0049-blob-write-zeroes-leaves-an-unallocated-thin-cluster-unallocated.patch index c603d81..da4a95b 100644 --- a/patches/0049-blob-write-zeroes-leaves-an-unallocated-thin-cluster-unallocated.patch +++ b/patches/0049-blob-write-zeroes-leaves-an-unallocated-thin-cluster-unallocated.patch @@ -1,3 +1,22 @@ +diff --git a/lib/blob/blobstore.c b/lib/blob/blobstore.c +index 4d656c9..b3105f4 100644 +--- a/lib/blob/blobstore.c ++++ b/lib/blob/blobstore.c +@@ -3255,6 +3255,14 @@ blob_request_submit_op_single(struct spdk_io_channel *_ch, struct spdk_blob *blo + } + + bs_batch_close(batch); ++ } else if (op_type == SPDK_BLOB_WRITE_ZEROES && blob_backed_with_zeroes_dev(blob)) { ++ /* Evariops 0049: an unallocated cluster of a blob backed by zeroes already reads ++ * zeroes, so zeroing it changes nothing: no cluster is allocated and nothing is ++ * written. A full rebuild sends every never-written part of its source as ++ * write-zeroes, and allocating each made the rebuilt replica fully allocated. A ++ * clone reads its unallocated clusters from its parent and keeps the allocation. */ ++ cb_fn(cb_arg, 0); ++ return; + } else { + /* Queue this operation and allocate the cluster */ + spdk_bs_user_op_t *op; diff --git a/test/unit/lib/blob/blob.c/blob_ut.c b/test/unit/lib/blob/blob.c/blob_ut.c index 3bb3f5c..cdf55b1 100644 --- a/test/unit/lib/blob/blob.c/blob_ut.c From a55abb1cf3237046d3b4c9b9657a7b2b6dace7b4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20DUCOM?= Date: Thu, 1 Oct 2026 09:56:28 +0200 Subject: [PATCH 12/18] test(nvmf): a reservation held by all registrants outlives its acquirer (0050, tests only) subsystem_ut, red on the series as it stands. A Write Exclusive - All Registrants reservation acquired by A, then A preempted by B: the reservation stays under A's key, which no registrant holds, and the state persisted for it fails to restore (-EINVAL). The same once B unregisters after C registered. The restore test now refuses a missing key for a single holder only, and takes it for a reservation held by all registrants, under the key of its first registrant. --- ...ts-reservation-outlives-its-acquirer.patch | 143 ++++++++++++++++++ patches/README.md | 3 +- 2 files changed, 145 insertions(+), 1 deletion(-) create mode 100644 patches/0050-nvmf-all-registrants-reservation-outlives-its-acquirer.patch diff --git a/patches/0050-nvmf-all-registrants-reservation-outlives-its-acquirer.patch b/patches/0050-nvmf-all-registrants-reservation-outlives-its-acquirer.patch new file mode 100644 index 0000000..53703e3 --- /dev/null +++ b/patches/0050-nvmf-all-registrants-reservation-outlives-its-acquirer.patch @@ -0,0 +1,143 @@ +diff --git a/test/unit/lib/nvmf/subsystem.c/subsystem_ut.c b/test/unit/lib/nvmf/subsystem.c/subsystem_ut.c +index d53d45d..28622b8 100644 +--- a/test/unit/lib/nvmf/subsystem.c/subsystem_ut.c ++++ b/test/unit/lib/nvmf/subsystem.c/subsystem_ut.c +@@ -1779,6 +1779,98 @@ test_reservation_acquire_release_with_ptpl(void) + ut_reservation_deinit(); + } + ++/* Restores the persisted reservation of g_ns onto a fresh namespace of the same bdev, as a ++ * target restart does, and checks it stands under the expected key. */ ++static void ++ut_reservation_check_restores_under(uint64_t crkey) ++{ ++ struct spdk_nvmf_reservation_info info; ++ struct spdk_nvmf_ns ns = {}; ++ int rc; ++ ++ rc = nvmf_ns_update_reservation_info(&g_ns); ++ SPDK_CU_ASSERT_FATAL(rc == 0); ++ memset(&info, 0, sizeof(info)); ++ rc = nvmf_ns_reservation_load(&g_ns, &info); ++ SPDK_CU_ASSERT_FATAL(rc == 0); ++ CU_ASSERT(info.rtype == SPDK_NVME_RESERVE_WRITE_EXCLUSIVE_ALL_REGS); ++ CU_ASSERT(info.crkey == crkey); ++ ++ ns.bdev = g_ns.bdev; ++ TAILQ_INIT(&ns.registrants); ++ rc = nvmf_ns_reservation_restore(&ns, &info); ++ CU_ASSERT(rc == 0); ++ CU_ASSERT(ns.rtype == SPDK_NVME_RESERVE_WRITE_EXCLUSIVE_ALL_REGS); ++ CU_ASSERT(ns.crkey == crkey); ++ nvmf_ns_reservation_clear_all_registrants(&ns); ++} ++ ++/* Evariops 0050: a reservation held by all registrants outlives the registrant that acquired ++ * it. When that registrant leaves, preempted by another or unregistering, the reservation ++ * stands under the key of a registrant that stays, and its persisted state restores. */ ++static void ++test_reservation_all_regs_acquirer_leaves_with_ptpl(void) ++{ ++ struct spdk_nvmf_request *req; ++ struct spdk_nvme_cpl *rsp; ++ struct spdk_nvmf_registrant *reg; ++ bool update_sgroup; ++ ++ ut_reservation_init(); ++ ++ req = ut_reservation_build_req(16); ++ rsp = &req->rsp->nvme_cpl; ++ SPDK_CU_ASSERT_FATAL(req != NULL); ++ ++ g_ns.ptpl_file = "/tmp/Ns1PR.cfg"; ++ ut_reservation_build_register_request(req, SPDK_NVME_RESERVE_REGISTER_KEY, 0, ++ SPDK_NVME_RESERVE_PTPL_PERSIST_POWER_LOSS, 0, 0xa1); ++ update_sgroup = nvmf_ns_reservation_register(&g_ns, &g_ctrlr1_A, req); ++ SPDK_CU_ASSERT_FATAL(update_sgroup == true); ++ SPDK_CU_ASSERT_FATAL(rsp->status.sc == SPDK_NVME_SC_SUCCESS); ++ ut_reservation_build_register_request(req, SPDK_NVME_RESERVE_REGISTER_KEY, 0, 0, 0, 0xb1); ++ nvmf_ns_reservation_register(&g_ns, &g_ctrlr_B, req); ++ SPDK_CU_ASSERT_FATAL(rsp->status.sc == SPDK_NVME_SC_SUCCESS); ++ ++ ut_reservation_build_acquire_request(req, SPDK_NVME_RESERVE_ACQUIRE, 0, ++ SPDK_NVME_RESERVE_WRITE_EXCLUSIVE_ALL_REGS, 0xa1, 0x0); ++ nvmf_ns_reservation_acquire(&g_ns, &g_ctrlr1_A, req); ++ SPDK_CU_ASSERT_FATAL(rsp->status.sc == SPDK_NVME_SC_SUCCESS); ++ SPDK_CU_ASSERT_FATAL(g_ns.crkey == 0xa1); ++ ++ /* TEST CASE: B preempts A, the registrant that acquired the reservation */ ++ ut_reservation_build_acquire_request(req, SPDK_NVME_RESERVE_PREEMPT, 0, ++ SPDK_NVME_RESERVE_WRITE_EXCLUSIVE_ALL_REGS, 0xb1, 0xa1); ++ nvmf_ns_reservation_acquire(&g_ns, &g_ctrlr_B, req); ++ SPDK_CU_ASSERT_FATAL(rsp->status.sc == SPDK_NVME_SC_SUCCESS); ++ SPDK_CU_ASSERT_FATAL(nvmf_ns_reservation_get_registrant(&g_ns, &g_ctrlr1_A.hostid) == NULL); ++ reg = nvmf_ns_reservation_get_registrant(&g_ns, &g_ctrlr_B.hostid); ++ SPDK_CU_ASSERT_FATAL(reg != NULL); ++ CU_ASSERT(g_ns.rtype == SPDK_NVME_RESERVE_WRITE_EXCLUSIVE_ALL_REGS); ++ CU_ASSERT(g_ns.holder == reg); ++ CU_ASSERT(g_ns.crkey == 0xb1); ++ ut_reservation_check_restores_under(0xb1); ++ ++ /* TEST CASE: C registers, then B, whose key the reservation now stands under, unregisters */ ++ ut_reservation_build_register_request(req, SPDK_NVME_RESERVE_REGISTER_KEY, 0, 0, 0, 0xc1); ++ nvmf_ns_reservation_register(&g_ns, &g_ctrlr_C, req); ++ SPDK_CU_ASSERT_FATAL(rsp->status.sc == SPDK_NVME_SC_SUCCESS); ++ ut_reservation_build_register_request(req, SPDK_NVME_RESERVE_UNREGISTER_KEY, 0, 0, 0xb1, 0); ++ nvmf_ns_reservation_register(&g_ns, &g_ctrlr_B, req); ++ SPDK_CU_ASSERT_FATAL(rsp->status.sc == SPDK_NVME_SC_SUCCESS); ++ SPDK_CU_ASSERT_FATAL(nvmf_ns_reservation_get_registrant(&g_ns, &g_ctrlr_B.hostid) == NULL); ++ reg = nvmf_ns_reservation_get_registrant(&g_ns, &g_ctrlr_C.hostid); ++ SPDK_CU_ASSERT_FATAL(reg != NULL); ++ CU_ASSERT(g_ns.rtype == SPDK_NVME_RESERVE_WRITE_EXCLUSIVE_ALL_REGS); ++ CU_ASSERT(g_ns.holder == reg); ++ CU_ASSERT(g_ns.crkey == 0xc1); ++ ut_reservation_check_restores_under(0xc1); ++ ++ unlink(g_ns.ptpl_file); ++ ut_reservation_free_req(req); ++ ut_reservation_deinit(); ++} ++ + static void + test_reservation_release(void) + { +@@ -3346,12 +3438,30 @@ test_nvmf_ns_reservation_restore(void) + spdk_uuid_fmt_lower(uuid, sizeof(uuid), &s_uuid); + snprintf(info.registrants[1].host_uuid, SPDK_UUID_STRING_LEN, "%s", uuid); + +- /* info->rkey not exist in registrants */ ++ /* info->rkey not exist in registrants: the key of a single holder that no registrant holds */ ++ info.rtype = SPDK_NVME_RESERVE_WRITE_EXCLUSIVE; + info.crkey = 0xa; + + rc = nvmf_ns_reservation_restore(&ns, &info); + CU_ASSERT(rc == -EINVAL); + ++ /* Evariops 0050: info->rkey not exist in registrants of a reservation held by all registrants: ++ * the registrant that acquired it left, which a state persisted before 0050 still records. It ++ * restores, under the key of the registrant that holds it now. */ ++ info.rtype = SPDK_NVME_RESERVE_WRITE_EXCLUSIVE_ALL_REGS; ++ ++ rc = nvmf_ns_reservation_restore(&ns, &info); ++ CU_ASSERT(rc == 0); ++ CU_ASSERT(ns.rtype == SPDK_NVME_RESERVE_WRITE_EXCLUSIVE_ALL_REGS); ++ reg0 = TAILQ_FIRST(&ns.registrants); ++ SPDK_CU_ASSERT_FATAL(reg0 != NULL); ++ CU_ASSERT(ns.holder == reg0); ++ CU_ASSERT(ns.crkey == reg0->rkey); ++ ++ rc = nvmf_ns_reservation_clear_all_registrants(&ns); ++ CU_ASSERT(rc == 2); ++ CU_ASSERT(TAILQ_EMPTY(&ns.registrants)); ++ + /* info->rkey exists in registrants */ + info.crkey = 0xb; + +@@ -3630,6 +3740,7 @@ main(int argc, char **argv) + CU_ADD_TEST(suite, test_reservation_register_with_ptpl); + CU_ADD_TEST(suite, test_reservation_acquire_preempt); + CU_ADD_TEST(suite, test_reservation_acquire_release_with_ptpl); ++ CU_ADD_TEST(suite, test_reservation_all_regs_acquirer_leaves_with_ptpl); + CU_ADD_TEST(suite, test_reservation_release); + CU_ADD_TEST(suite, test_reservation_unregister_notification); + CU_ADD_TEST(suite, test_reservation_release_notification); diff --git a/patches/README.md b/patches/README.md index 619c3cb..750e515 100644 --- a/patches/README.md +++ b/patches/README.md @@ -4,7 +4,7 @@ Out-of-tree patches applied on top of upstream SPDK during the container build ( ## Application order -Patches are applied in **lexicographic order of filename** (`0001` … `0049`) — the Dockerfile globs `patches/*.patch` and `git apply`s each. The numeric prefix IS the contract; do not rely on any other ordering. Order matters: +Patches are applied in **lexicographic order of filename** (`0001` … `0050`) — the Dockerfile globs `patches/*.patch` and `git apply`s each. The numeric prefix IS the contract; do not rely on any other ordering. Order matters: | # | Patch | Touches | Depends on | |--:|:------|:--------|:-----------| @@ -55,6 +55,7 @@ Patches are applied in **lexicographic order of filename** (`0001` … `0049`) | 0047 | a raid1 rebuild copies its window in parts — `raid_bdev_process_request_max_part` offers a window to the process's requests in `RAID_BDEV_PROCESS_MAX_QD` parts (whole write units), and raid1 copies one part per request, so a window's parts are in flight together: each part's write starts as its own read completes, and the reads spread over the members in sync. Copied as one request, a window was read whole from one member, then written whole: read and write never overlapped, and the first member served every read — a 5 GiB rebuild ran at 36 MiB/s, all of it read across the link from a remote member while the nexus's local member idled. raid5f, bound to its stripe, ignores the part. `raid1_ut` pins the parts, their spread and their writes; `bdev_raid_ut` the part's bounds | bdev_raid, raid1, test/unit/lib/bdev/raid | 0013 | | 0048 | a raid1 rebuild writes a zero part as write-zeroes — a part the source read back as nothing but zeroes goes to the target as `write_zeroes` (`raid_bdev_write_zeroes_blocks`): the same blocks, read back the same, and no payload on the wire, so a full rebuild of a sparse volume no longer ships its never-written clusters across the link (about 3 of the 5 GiB of the rebuild measured for 0047). Not when the target cannot take write-zeroes (the bdev layer would send a zero buffer anyway), nor when the blocks carry metadata. `raid1_ut` pins both kinds of write, and a target without write-zeroes getting data | raid1, bdev_raid (header), test/unit/lib/bdev/raid | 0047 | | 0049 | write-zeroes leaves an unallocated thin cluster unallocated — on a blob backed by zeroes, an unallocated cluster already reads zeroes, so write-zeroes over it completes with no cluster allocated and nothing written. A full rebuild sends every never-written part of its source as write-zeroes (0048), and allocating each made the rebuilt replica fully allocated — a thin volume rebuilt in full took its whole size on the target — and wrote zeroes across the target's disk. An allocated cluster is still zeroed; a clone, which reads its unallocated clusters from its parent, still allocates and zeroes. `blob_ut` pins the three | lib/blob (blobstore.c), test/unit/lib/blob | 0044 | +| 0050 | a reservation held by all registrants outlives the registrant that acquired it — when that registrant leaves (preempted by another, or unregistering), the reservation, which every registrant holds, now stands under the key of the registrant that holds it next. It kept the key of the one that left, and the persisted state (`ptpl_file`) recorded a reservation key no registrant held: the restore refused it, so after a writer handover, the next restart of a target failed to add the namespace back, and the target stayed unexported. The restore also takes a state persisted before this patch, under the key of its first registrant; a single holder's key that no registrant holds is still refused. `subsystem_ut` pins the preempt, the unregister and the restore | lib/nvmf (subsystem.c), test/unit/lib/nvmf/subsystem.c | — | 0005 `#include`s `vbdev_tier.h` and adds `-I module/bdev/tier` to the lvol module CFLAGS via its own Makefile hunk; the Dockerfile injects the module dirs before applying patches (copy-before-apply ordering matters). From d7b99d82c0530fcad7204a8d8a1d01508e268245 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20DUCOM?= Date: Thu, 1 Oct 2026 09:57:32 +0200 Subject: [PATCH 13/18] fix(nvmf): a reservation held by all registrants outlives its acquirer (0050) Every registrant holds a Write Exclusive or Exclusive Access - All Registrants reservation, so when the registrant that acquired it leaves, preempted by another or unregistering, the reservation stays. It stayed under the key of the registrant that left: the state persisted for it named a reservation key no registrant held, and the restore refused it (-EINVAL), failing the add of the namespace. After a writer handover, the next restart of a target left its namespace out, and the target unexported. The reservation now stands under the key of the registrant that holds it next. The restore takes a state persisted before this, under the key of its first registrant; a single holder's key that no registrant holds is still refused. --- ...ts-reservation-outlives-its-acquirer.patch | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/patches/0050-nvmf-all-registrants-reservation-outlives-its-acquirer.patch b/patches/0050-nvmf-all-registrants-reservation-outlives-its-acquirer.patch index 53703e3..2340a37 100644 --- a/patches/0050-nvmf-all-registrants-reservation-outlives-its-acquirer.patch +++ b/patches/0050-nvmf-all-registrants-reservation-outlives-its-acquirer.patch @@ -1,3 +1,50 @@ +diff --git a/lib/nvmf/subsystem.c b/lib/nvmf/subsystem.c +index 356361f..3d53309 100644 +--- a/lib/nvmf/subsystem.c ++++ b/lib/nvmf/subsystem.c +@@ -2919,6 +2919,7 @@ nvmf_ns_reservation_restore(struct spdk_nvmf_ns *ns, struct spdk_nvmf_reservatio + struct spdk_nvmf_registrant *reg, *holder = NULL; + struct spdk_uuid bdev_uuid, holder_uuid; + bool rkey_flag = false; ++ bool all_regs; + + SPDK_DEBUGLOG(nvmf, "NSID %u, PTPL %u, Number of registrants %u\n", + ns->nsid, info->ptpl_activated, info->num_regs); +@@ -2934,7 +2935,12 @@ nvmf_ns_reservation_restore(struct spdk_nvmf_ns *ns, struct spdk_nvmf_reservatio + rkey_flag = true; + } + } +- if (!rkey_flag && info->crkey != 0) { ++ /* Evariops 0050: a reservation held by all registrants outlives the registrant that ++ * acquired it, and a state persisted before 0050 still records the key of that registrant ++ * after it left: it restores, under the key of the registrant that holds it (below). */ ++ all_regs = (info->rtype == SPDK_NVME_RESERVE_WRITE_EXCLUSIVE_ALL_REGS || ++ info->rtype == SPDK_NVME_RESERVE_EXCLUSIVE_ACCESS_ALL_REGS); ++ if (!rkey_flag && info->crkey != 0 && !all_regs) { + return -EINVAL; + } + +@@ -2972,6 +2978,9 @@ nvmf_ns_reservation_restore(struct spdk_nvmf_ns *ns, struct spdk_nvmf_reservatio + + if (nvmf_ns_reservation_all_registrants_type(ns)) { + ns->holder = TAILQ_FIRST(&ns->registrants); ++ if (!rkey_flag) { ++ ns->crkey = ns->holder->rkey; ++ } + } else { + ns->holder = holder; + } +@@ -3267,6 +3276,10 @@ nvmf_ns_reservation_check_release_on_remove_registrant(struct spdk_nvmf_ns *ns, + if (next_reg && nvmf_ns_reservation_all_registrants_type(ns)) { + /* the next valid registrant is the new holder now */ + ns->holder = next_reg; ++ /* Evariops 0050: every registrant holds the reservation, so it outlives the one that ++ * acquired it, and now stands under the key of the registrant that holds it. Left under ++ * the key of a registrant that is gone, its persisted state fails to restore. */ ++ ns->crkey = next_reg->rkey; + } else if (nvmf_ns_reservation_registrant_is_holder(ns, reg)) { + /* release the reservation */ + nvmf_ns_reservation_release_reservation(ns); diff --git a/test/unit/lib/nvmf/subsystem.c/subsystem_ut.c b/test/unit/lib/nvmf/subsystem.c/subsystem_ut.c index d53d45d..28622b8 100644 --- a/test/unit/lib/nvmf/subsystem.c/subsystem_ut.c From f74e7564ef1a36f0fd67d97bfbee7e9f79df4b38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20DUCOM?= Date: Thu, 1 Oct 2026 22:11:53 +0200 Subject: [PATCH 14/18] test(bdev-nvme): an I/O passthrough needs a connected qpair (0051, tests only) bdev_nvme_ut, red on the series as it stands. The suite now compiles nvme_rpc.c in, so the I/O passthrough can be driven against a real controller channel. With the channel's I/O qpair cleared, as bdev_nvme clears it from a disconnect until the reconnect, the command is still handed to the NVMe library (rc 0, the channel kept): in a real process that NULL qpair is dereferenced. The test expects -ENXIO with the channel given back, and the command to go through once connected again. --- ...-passthrough-needs-a-connected-qpair.patch | 101 ++++++++++++++++++ patches/README.md | 3 +- 2 files changed, 103 insertions(+), 1 deletion(-) create mode 100644 patches/0051-bdev-nvme-io-passthrough-needs-a-connected-qpair.patch diff --git a/patches/0051-bdev-nvme-io-passthrough-needs-a-connected-qpair.patch b/patches/0051-bdev-nvme-io-passthrough-needs-a-connected-qpair.patch new file mode 100644 index 0000000..7365c45 --- /dev/null +++ b/patches/0051-bdev-nvme-io-passthrough-needs-a-connected-qpair.patch @@ -0,0 +1,101 @@ +diff --git a/test/unit/lib/bdev/nvme/bdev_nvme.c/bdev_nvme_ut.c b/test/unit/lib/bdev/nvme/bdev_nvme.c/bdev_nvme_ut.c +index c64dc7c..bd022fe 100644 +--- a/test/unit/lib/bdev/nvme/bdev_nvme.c/bdev_nvme_ut.c ++++ b/test/unit/lib/bdev/nvme/bdev_nvme.c/bdev_nvme_ut.c +@@ -15,6 +15,19 @@ + + #include "unit/lib/json_mock.c" + ++/* nvme_rpc.c is compiled in so its I/O passthrough can be driven against a real controller ++ * channel (Evariops 0051); its JSON decoding and RPC registration are not exercised here. */ ++#include "spdk/rpc.h" ++DEFINE_STUB(spdk_json_decode_object, int, (const struct spdk_json_val *values, ++ const struct spdk_json_object_decoder *decoders, size_t num_decoders, void *out), 0); ++DEFINE_STUB(spdk_json_decode_string, int, (const struct spdk_json_val *val, void *out), 0); ++DEFINE_STUB(spdk_json_decode_uint32, int, (const struct spdk_json_val *val, void *out), 0); ++DEFINE_STUB(spdk_json_strequal, bool, (const struct spdk_json_val *val, const char *str), false); ++DEFINE_STUB_V(spdk_rpc_register_method, (const char *method, spdk_rpc_method_handler func, ++ uint32_t state_mask)); ++ ++#include "bdev/nvme/nvme_rpc.c" ++ + #include "bdev/nvme/bdev_mdns_client.c" + + static void *g_accel_p = (void *)0xdeadbeaf; +@@ -8490,6 +8503,68 @@ test_race_between_ctrlr_loss_timeout_and_pending_failover(void) + CU_ASSERT(nvme_ctrlr_get_by_name("nvme0") == NULL); + } + ++/* bdev_nvme frees a controller channel's I/O qpair from a disconnect until the reconnect ++ * (reset, failed controller, delayed reconnect). bdev_nvme_send_cmd handed that NULL qpair to ++ * the NVMe library and the process died on it (SIGSEGV), taking every raid of the process down: ++ * a reservation command is an I/O command. Refused instead, with the channel given back. */ ++static void ++test_nvme_rpc_io_cmd_needs_a_connected_qpair(void) ++{ ++ struct spdk_nvme_transport_id trid = {}; ++ struct spdk_nvme_ctrlr ctrlr = {}; ++ struct nvme_ctrlr *nvme_ctrlr; ++ struct rpc_bdev_nvme_send_cmd_ctx ctx = {}; ++ struct spdk_nvme_cmd cmd = {}; ++ struct spdk_io_channel *ch; ++ struct nvme_ctrlr_channel *ctrlr_ch; ++ struct spdk_nvme_qpair *qpair; ++ int rc; ++ ++ ut_init_trid(&trid); ++ TAILQ_INIT(&ctrlr.active_io_qpairs); ++ ++ set_thread(0); ++ ++ rc = nvme_ctrlr_create(&ctrlr, "nvme0", &trid, NULL); ++ CU_ASSERT(rc == 0); ++ ++ nvme_ctrlr = nvme_ctrlr_get_by_name("nvme0"); ++ SPDK_CU_ASSERT_FATAL(nvme_ctrlr != NULL); ++ ++ ch = spdk_get_io_channel(nvme_ctrlr); ++ SPDK_CU_ASSERT_FATAL(ch != NULL); ++ ctrlr_ch = spdk_io_channel_get_ctx(ch); ++ qpair = ctrlr_ch->qpair->qpair; ++ SPDK_CU_ASSERT_FATAL(qpair != NULL); ++ ++ /* Disconnected: no qpair to carry the command. */ ++ ctrlr_ch->qpair->qpair = NULL; ++ ctx.nvme_ctrlr = nvme_ctrlr; ++ rc = nvme_rpc_io_cmd_bdev_nvme(&ctx, &cmd, NULL, 0, NULL, 0, 0); ++ CU_ASSERT(rc == -ENXIO); ++ CU_ASSERT(ctx.ctrlr_io_ch == NULL); ++ ++ /* Connected again: the command goes through, holding the channel until it completes. */ ++ ctrlr_ch->qpair->qpair = qpair; ++ rc = nvme_rpc_io_cmd_bdev_nvme(&ctx, &cmd, NULL, 0, NULL, 0, 0); ++ CU_ASSERT(rc == 0); ++ CU_ASSERT(ctx.ctrlr_io_ch == ch); ++ spdk_put_io_channel(ctx.ctrlr_io_ch); ++ ++ spdk_put_io_channel(ch); ++ ++ poll_threads(); ++ ++ rc = spdk_bdev_nvme_delete("nvme0", &g_any_path, NULL, NULL); ++ CU_ASSERT(rc == 0); ++ ++ poll_threads(); ++ spdk_delay_us(1000); ++ poll_threads(); ++ ++ CU_ASSERT(nvme_ctrlr_get_by_name("nvme0") == NULL); ++} ++ + int + main(int argc, char **argv) + { +@@ -8554,6 +8629,7 @@ main(int argc, char **argv) + CU_ADD_TEST(suite, test_bdev_reset_abort_io); + CU_ADD_TEST(suite, test_race_between_clear_pending_resets_and_reset_ctrlr_complete); + CU_ADD_TEST(suite, test_race_between_ctrlr_loss_timeout_and_pending_failover); ++ CU_ADD_TEST(suite, test_nvme_rpc_io_cmd_needs_a_connected_qpair); + + allocate_threads(3); + set_thread(0); diff --git a/patches/README.md b/patches/README.md index 750e515..f4aef02 100644 --- a/patches/README.md +++ b/patches/README.md @@ -4,7 +4,7 @@ Out-of-tree patches applied on top of upstream SPDK during the container build ( ## Application order -Patches are applied in **lexicographic order of filename** (`0001` … `0050`) — the Dockerfile globs `patches/*.patch` and `git apply`s each. The numeric prefix IS the contract; do not rely on any other ordering. Order matters: +Patches are applied in **lexicographic order of filename** (`0001` … `0051`) — the Dockerfile globs `patches/*.patch` and `git apply`s each. The numeric prefix IS the contract; do not rely on any other ordering. Order matters: | # | Patch | Touches | Depends on | |--:|:------|:--------|:-----------| @@ -56,6 +56,7 @@ Patches are applied in **lexicographic order of filename** (`0001` … `0050`) | 0048 | a raid1 rebuild writes a zero part as write-zeroes — a part the source read back as nothing but zeroes goes to the target as `write_zeroes` (`raid_bdev_write_zeroes_blocks`): the same blocks, read back the same, and no payload on the wire, so a full rebuild of a sparse volume no longer ships its never-written clusters across the link (about 3 of the 5 GiB of the rebuild measured for 0047). Not when the target cannot take write-zeroes (the bdev layer would send a zero buffer anyway), nor when the blocks carry metadata. `raid1_ut` pins both kinds of write, and a target without write-zeroes getting data | raid1, bdev_raid (header), test/unit/lib/bdev/raid | 0047 | | 0049 | write-zeroes leaves an unallocated thin cluster unallocated — on a blob backed by zeroes, an unallocated cluster already reads zeroes, so write-zeroes over it completes with no cluster allocated and nothing written. A full rebuild sends every never-written part of its source as write-zeroes (0048), and allocating each made the rebuilt replica fully allocated — a thin volume rebuilt in full took its whole size on the target — and wrote zeroes across the target's disk. An allocated cluster is still zeroed; a clone, which reads its unallocated clusters from its parent, still allocates and zeroes. `blob_ut` pins the three | lib/blob (blobstore.c), test/unit/lib/blob | 0044 | | 0050 | a reservation held by all registrants outlives the registrant that acquired it — when that registrant leaves (preempted by another, or unregistering), the reservation, which every registrant holds, now stands under the key of the registrant that holds it next. It kept the key of the one that left, and the persisted state (`ptpl_file`) recorded a reservation key no registrant held: the restore refused it, so after a writer handover, the next restart of a target failed to add the namespace back, and the target stayed unexported. The restore also takes a state persisted before this patch, under the key of its first registrant; a single holder's key that no registrant holds is still refused. `subsystem_ut` pins the preempt, the unregister and the restore | lib/nvmf (subsystem.c), test/unit/lib/nvmf/subsystem.c | — | +| 0051 | an I/O passthrough needs a connected qpair — `bdev_nvme_send_cmd` with an I/O command (NVMe reservations are I/O commands) took the controller channel's qpair without a check and handed it to the NVMe library. bdev_nvme frees that qpair from a disconnect until the reconnect (reset, failed controller, delayed reconnect), so a reservation sent to a member whose path had just gone down dereferenced NULL and killed the process, and every raid it carried with it. The command is now refused (`-ENXIO`, the channel given back), as is one for which no channel can be had. `bdev_nvme_ut` pins the disconnected and the connected case | module/bdev/nvme (nvme_rpc.c), test/unit/lib/bdev/nvme | — | 0005 `#include`s `vbdev_tier.h` and adds `-I module/bdev/tier` to the lvol module CFLAGS via its own Makefile hunk; the Dockerfile injects the module dirs before applying patches (copy-before-apply ordering matters). From 8a3f3f1bf2f715f22545f00139bed20687cd7db6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20DUCOM?= Date: Thu, 1 Oct 2026 22:12:06 +0200 Subject: [PATCH 15/18] fix(bdev-nvme): an I/O passthrough needs a connected qpair (0051) bdev_nvme_send_cmd with an I/O command took the controller channel's qpair (bdev_nvme_get_io_qpair) without a check and handed it to spdk_nvme_ctrlr_cmd_io_raw_with_md. bdev_nvme frees that qpair from a disconnect until the reconnect (bdev_nvme_disconnected_qpair_cb), so a reservation (reservations are I/O commands) sent to a member whose path had just gone down dereferenced NULL and killed the process, every raid it carried with it. The command is now refused with -ENXIO and the channel given back; a command for which no channel can be had is refused with -ENOMEM instead of reaching the assert-only accessor. --- ...-passthrough-needs-a-connected-qpair.patch | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/patches/0051-bdev-nvme-io-passthrough-needs-a-connected-qpair.patch b/patches/0051-bdev-nvme-io-passthrough-needs-a-connected-qpair.patch index 7365c45..3f0026d 100644 --- a/patches/0051-bdev-nvme-io-passthrough-needs-a-connected-qpair.patch +++ b/patches/0051-bdev-nvme-io-passthrough-needs-a-connected-qpair.patch @@ -1,3 +1,27 @@ +diff --git a/module/bdev/nvme/nvme_rpc.c b/module/bdev/nvme/nvme_rpc.c +index bbcb903..d43a9ec 100644 +--- a/module/bdev/nvme/nvme_rpc.c ++++ b/module/bdev/nvme/nvme_rpc.c +@@ -162,7 +162,19 @@ nvme_rpc_io_cmd_bdev_nvme(struct rpc_bdev_nvme_send_cmd_ctx *ctx, struct spdk_nv + int ret; + + ctx->ctrlr_io_ch = spdk_get_io_channel(_nvme_ctrlr); ++ if (ctx->ctrlr_io_ch == NULL) { ++ return -ENOMEM; ++ } ++ ++ /* bdev_nvme frees a channel's I/O qpair from a disconnect until the reconnect (reset, ++ * failed controller, delayed reconnect). Submitting on it dereferenced NULL and took the ++ * whole process down, every raid with it; refuse the command instead. */ + io_qpair = bdev_nvme_get_io_qpair(ctx->ctrlr_io_ch); ++ if (io_qpair == NULL) { ++ spdk_put_io_channel(ctx->ctrlr_io_ch); ++ ctx->ctrlr_io_ch = NULL; ++ return -ENXIO; ++ } + + ret = spdk_nvme_ctrlr_cmd_io_raw_with_md(_nvme_ctrlr->ctrlr, io_qpair, + cmd, buf, nbytes, md_buf, nvme_rpc_bdev_nvme_cb, ctx); diff --git a/test/unit/lib/bdev/nvme/bdev_nvme.c/bdev_nvme_ut.c b/test/unit/lib/bdev/nvme/bdev_nvme.c/bdev_nvme_ut.c index c64dc7c..bd022fe 100644 --- a/test/unit/lib/bdev/nvme/bdev_nvme.c/bdev_nvme_ut.c From 350d9cf1f208b204bf658e49df4d71989c07cfda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20DUCOM?= Date: Fri, 2 Oct 2026 09:12:06 +0200 Subject: [PATCH 16/18] test(raid): a raid1 ejection opens its epoch on the raid's cbt (0052, tests only) bdev_raid_ut, red on the series as it stands. A current member that leaves a raid1, removed or failed, should open one epoch, on the cbt stacked on the raid and named after the member's UUID. 0019 asks each survivor's own name instead (31 calls here, none to the raid), a bdev no cbt sits on, so no epoch ever opens and a member lost without notice is always rebuilt in full. The suite also pins five ejections that must open none: a member being rebuilt in full (its superblock slot not configured), one behind the generation its peers carry, a write-only joiner, a raid without a superblock, and a failed member an earlier removal failed to take. --- ...tion-opens-an-epoch-on-the-raids-cbt.patch | 220 ++++++++++++++++++ patches/README.md | 3 +- 2 files changed, 222 insertions(+), 1 deletion(-) create mode 100644 patches/0052-raid1-ejection-opens-an-epoch-on-the-raids-cbt.patch diff --git a/patches/0052-raid1-ejection-opens-an-epoch-on-the-raids-cbt.patch b/patches/0052-raid1-ejection-opens-an-epoch-on-the-raids-cbt.patch new file mode 100644 index 0000000..394ae22 --- /dev/null +++ b/patches/0052-raid1-ejection-opens-an-epoch-on-the-raids-cbt.patch @@ -0,0 +1,220 @@ +diff --git a/test/unit/lib/bdev/raid/bdev_raid.c/bdev_raid_ut.c b/test/unit/lib/bdev/raid/bdev_raid.c/bdev_raid_ut.c +index a488d6d..c359160 100644 +--- a/test/unit/lib/bdev/raid/bdev_raid.c/bdev_raid_ut.c ++++ b/test/unit/lib/bdev/raid/bdev_raid.c/bdev_raid_ut.c +@@ -242,10 +242,22 @@ DEFINE_STUB(vbdev_cbt_query_latest_epoch, int, (const char *bdev_name, + struct vbdev_cbt_epoch_facts *out), -ENODEV); + /* Evariops 0018: a rebuild's outcome turns to verifying before its verify pass. */ + DEFINE_STUB_V(raid_rebuild_outcome_set_verifying, (struct raid_rebuild_outcome *outcome)); +-/* Evariops 0019: a raid1 ejection opens a CBT epoch on each survivor. No survivor +- * has CBT here (-ENODEV), which the raid passes over. */ +-DEFINE_STUB(vbdev_cbt_auto_epoch_open, int, (const char *bdev_name, const char *stale_backend_id), +- -ENODEV); ++/* Evariops 0019, 0052: a raid1 ejection opens a CBT epoch for the member that ++ * leaves. The CBT module is not part of this suite: this fake records what the ++ * raid asks it for. */ ++static uint32_t g_ejection_epochs; ++static char g_ejection_epoch_over[64]; ++static char g_ejection_epoch_member[64]; ++ ++int ++vbdev_cbt_auto_epoch_open(const char *bdev_name, const char *member_id) ++{ ++ g_ejection_epochs++; ++ snprintf(g_ejection_epoch_over, sizeof(g_ejection_epoch_over), "%s", bdev_name); ++ snprintf(g_ejection_epoch_member, sizeof(g_ejection_epoch_member), "%s", member_id); ++ ++ return 0; ++} + + /* Evariops 0046: a seeded rebuild reads its ranges from a CBT epoch. The CBT + * module is not part of this suite: the epoch's delta is this fake's. */ +@@ -679,12 +691,15 @@ spdk_bdev_get_by_name(const char *bdev_name) + return NULL; + } + ++/* Evariops 0052: the status a quiesce of a whole bdev completes with. */ ++static int g_quiesce_status; ++ + int + spdk_bdev_quiesce(struct spdk_bdev *bdev, struct spdk_bdev_module *module, + spdk_bdev_quiesce_cb cb_fn, void *cb_arg) + { + if (cb_fn) { +- cb_fn(cb_arg, 0); ++ cb_fn(cb_arg, g_quiesce_status); + } + + return 0; +@@ -2156,6 +2171,162 @@ test_raid_io_split(void) + reset_globals(); + } + ++/* Evariops 0052: how the member in slot 0 leaves the raid. */ ++enum ut_ejection { ++ /* removed by the control plane, or hot-removed with its bdev */ ++ UT_EJECTION_REMOVED, ++ /* failed by an I/O error */ ++ UT_EJECTION_FAILED, ++ /* failed again after its first removal failed */ ++ UT_EJECTION_FAILED_AFTER_A_FAILED_REMOVAL, ++}; ++ ++/* Evariops 0052: creates a superblock raid1 whose members all sit CONFIGURED at ++ * one generation, lets setup change that, ejects the member in slot 0 and returns ++ * how many epochs the ejection opened; member receives that member's UUID. The ++ * suite stubs the superblock's allocation, so the raid gets one here, and the ++ * suite's own module stands in for raid1. */ ++static uint32_t ++ejection_epochs_run(enum ut_ejection ejection, ++ void (*setup)(struct raid_bdev *raid_bdev, struct raid_bdev_superblock *sb), ++ char *member, size_t member_len) ++{ ++ struct rpc_bdev_raid_create req; ++ struct rpc_bdev_raid_delete destroy_req; ++ struct raid_bdev_superblock *sb; ++ struct raid_base_bdev_info *base_info; ++ struct raid_bdev *pbdev; ++ uint8_t i; ++ ++ set_globals(); ++ CU_ASSERT(raid_bdev_init() == 0); ++ create_raid_bdev_create_req(&req, "raid1", 0, true, 0, true); ++ rpc_bdev_raid_create(NULL, NULL); ++ CU_ASSERT(g_rpc_err == 0); ++ free_test_req(&req); ++ ++ pbdev = raid_bdev_find_by_name("raid1"); ++ SPDK_CU_ASSERT_FATAL(pbdev != NULL); ++ pbdev->level = RAID1; ++ pbdev->min_base_bdevs_operational = 1; ++ sb = calloc(1, sizeof(*sb) + pbdev->num_base_bdevs * sizeof(sb->base_bdevs[0])); ++ SPDK_CU_ASSERT_FATAL(sb != NULL); ++ sb->base_bdevs_size = pbdev->num_base_bdevs; ++ for (i = 0; i < pbdev->num_base_bdevs; i++) { ++ sb->base_bdevs[i].slot = i; ++ sb->base_bdevs[i].state = RAID_SB_BASE_BDEV_CONFIGURED; ++ sb->base_bdevs[i].content_generation = 7; ++ pbdev->base_bdev_info[i].content_generation = 7; ++ } ++ pbdev->sb = sb; ++ if (setup != NULL) { ++ setup(pbdev, sb); ++ } ++ ++ base_info = &pbdev->base_bdev_info[0]; ++ spdk_uuid_fmt_lower(member, member_len, &base_info->uuid); ++ g_ejection_epochs = 0; ++ g_ejection_epoch_over[0] = '\0'; ++ g_ejection_epoch_member[0] = '\0'; ++ ++ switch (ejection) { ++ case UT_EJECTION_REMOVED: ++ CU_ASSERT(raid_bdev_remove_base_bdev(spdk_bdev_desc_get_bdev(base_info->desc), NULL, ++ NULL, NULL) == 0); ++ break; ++ case UT_EJECTION_FAILED_AFTER_A_FAILED_REMOVAL: ++ g_quiesce_status = -EIO; ++ raid_bdev_fail_base_bdev(base_info); ++ poll_app_thread(); ++ g_quiesce_status = 0; ++ CU_ASSERT(base_info->is_configured); ++ CU_ASSERT(base_info->remove_error == -EIO); ++ /* fallthrough */ ++ case UT_EJECTION_FAILED: ++ raid_bdev_fail_base_bdev(base_info); ++ break; ++ } ++ poll_app_thread(); ++ CU_ASSERT(!base_info->is_configured); ++ ++ pbdev->sb = NULL; ++ free(sb); ++ create_raid_bdev_delete_req(&destroy_req, "raid1", 0); ++ rpc_bdev_raid_delete(NULL, NULL); ++ CU_ASSERT(g_rpc_err == 0); ++ raid_bdev_exit(); ++ base_bdevs_cleanup(); ++ reset_globals(); ++ ++ return g_ejection_epochs; ++} ++ ++/* Evariops 0052: a raid1 member that leaves current, removed or failed, opens one ++ * epoch, on the cbt bdev stacked on the raid and named after the member's UUID: ++ * the identity the control plane derives from the replica it placed there. 0019 ++ * opened one under each survivor's own name, a bdev no cbt sits on (-ENODEV), so ++ * a member lost without notice was always rebuilt in full. */ ++static void ++test_raid1_ejection_opens_an_epoch_on_the_raids_cbt(void) ++{ ++ const enum ut_ejection ejections[] = { UT_EJECTION_REMOVED, UT_EJECTION_FAILED }; ++ char member[SPDK_UUID_STRING_LEN]; ++ size_t i; ++ ++ for (i = 0; i < SPDK_COUNTOF(ejections); i++) { ++ CU_ASSERT(ejection_epochs_run(ejections[i], NULL, member, sizeof(member)) == 1); ++ CU_ASSERT(strcmp(g_ejection_epoch_over, "raid1") == 0); ++ CU_ASSERT(strcmp(g_ejection_epoch_member, member) == 0); ++ } ++} ++ ++static void ++ut_member_rebuilt_in_full(struct raid_bdev *raid_bdev, struct raid_bdev_superblock *sb) ++{ ++ sb->base_bdevs[0].state = RAID_SB_BASE_BDEV_MISSING; ++} ++ ++static void ++ut_member_behind(struct raid_bdev *raid_bdev, struct raid_bdev_superblock *sb) ++{ ++ sb->base_bdevs[0].content_generation = 6; ++ raid_bdev->base_bdev_info[0].content_generation = 6; ++} ++ ++static void ++ut_member_joining_write_only(struct raid_bdev *raid_bdev, struct raid_bdev_superblock *sb) ++{ ++ raid_bdev->base_bdev_info[0].write_only_pending = true; ++ raid_bdev->base_bdev_info[0].read_excluded = true; ++} ++ ++static void ++ut_no_superblock(struct raid_bdev *raid_bdev, struct raid_bdev_superblock *sb) ++{ ++ raid_bdev->sb = NULL; ++} ++ ++/* Evariops 0052: an epoch holds the writes a member misses from its departure on. ++ * A member that already lacked some (being rebuilt in full, behind the generation ++ * its peers carry, or joining write-only) would be seeded with less than it owes, ++ * and so would one a removal already failed to take, which may have missed writes ++ * since that attempt. None of their ejections opens an epoch, nor one with no ++ * superblock to tell: each of them is rebuilt in full. */ ++static void ++test_raid1_ejection_of_a_member_behind_opens_no_epoch(void) ++{ ++ char member[SPDK_UUID_STRING_LEN]; ++ ++ CU_ASSERT(ejection_epochs_run(UT_EJECTION_REMOVED, ut_member_rebuilt_in_full, ++ member, sizeof(member)) == 0); ++ CU_ASSERT(ejection_epochs_run(UT_EJECTION_REMOVED, ut_member_behind, member, sizeof(member)) == 0); ++ CU_ASSERT(ejection_epochs_run(UT_EJECTION_REMOVED, ut_member_joining_write_only, ++ member, sizeof(member)) == 0); ++ CU_ASSERT(ejection_epochs_run(UT_EJECTION_REMOVED, ut_no_superblock, member, sizeof(member)) == 0); ++ CU_ASSERT(ejection_epochs_run(UT_EJECTION_FAILED_AFTER_A_FAILED_REMOVAL, NULL, ++ member, sizeof(member)) == 0); ++} ++ + static int + test_new_thread_fn(struct spdk_thread *thread) + { +@@ -2207,6 +2378,8 @@ main(int argc, char **argv) + CU_ADD_TEST(suite, test_raid_seeded_rebuild_from_epoch_refuses_a_delta_it_cannot_take_whole); + CU_ADD_TEST(suite, test_raid_process_request_max_part); + CU_ADD_TEST(suite, test_raid_process_with_qos); ++ CU_ADD_TEST(suite, test_raid1_ejection_opens_an_epoch_on_the_raids_cbt); ++ CU_ADD_TEST(suite, test_raid1_ejection_of_a_member_behind_opens_no_epoch); + + spdk_thread_lib_init(test_new_thread_fn, 0); + g_app_thread = spdk_thread_create("app_thread", NULL); diff --git a/patches/README.md b/patches/README.md index f4aef02..8035ceb 100644 --- a/patches/README.md +++ b/patches/README.md @@ -4,7 +4,7 @@ Out-of-tree patches applied on top of upstream SPDK during the container build ( ## Application order -Patches are applied in **lexicographic order of filename** (`0001` … `0051`) — the Dockerfile globs `patches/*.patch` and `git apply`s each. The numeric prefix IS the contract; do not rely on any other ordering. Order matters: +Patches are applied in **lexicographic order of filename** (`0001` … `0052`) — the Dockerfile globs `patches/*.patch` and `git apply`s each. The numeric prefix IS the contract; do not rely on any other ordering. Order matters: | # | Patch | Touches | Depends on | |--:|:------|:--------|:-----------| @@ -57,6 +57,7 @@ Patches are applied in **lexicographic order of filename** (`0001` … `0051`) | 0049 | write-zeroes leaves an unallocated thin cluster unallocated — on a blob backed by zeroes, an unallocated cluster already reads zeroes, so write-zeroes over it completes with no cluster allocated and nothing written. A full rebuild sends every never-written part of its source as write-zeroes (0048), and allocating each made the rebuilt replica fully allocated — a thin volume rebuilt in full took its whole size on the target — and wrote zeroes across the target's disk. An allocated cluster is still zeroed; a clone, which reads its unallocated clusters from its parent, still allocates and zeroes. `blob_ut` pins the three | lib/blob (blobstore.c), test/unit/lib/blob | 0044 | | 0050 | a reservation held by all registrants outlives the registrant that acquired it — when that registrant leaves (preempted by another, or unregistering), the reservation, which every registrant holds, now stands under the key of the registrant that holds it next. It kept the key of the one that left, and the persisted state (`ptpl_file`) recorded a reservation key no registrant held: the restore refused it, so after a writer handover, the next restart of a target failed to add the namespace back, and the target stayed unexported. The restore also takes a state persisted before this patch, under the key of its first registrant; a single holder's key that no registrant holds is still refused. `subsystem_ut` pins the preempt, the unregister and the restore | lib/nvmf (subsystem.c), test/unit/lib/nvmf/subsystem.c | — | | 0051 | an I/O passthrough needs a connected qpair — `bdev_nvme_send_cmd` with an I/O command (NVMe reservations are I/O commands) took the controller channel's qpair without a check and handed it to the NVMe library. bdev_nvme frees that qpair from a disconnect until the reconnect (reset, failed controller, delayed reconnect), so a reservation sent to a member whose path had just gone down dereferenced NULL and killed the process, and every raid it carried with it. The command is now refused (`-ENXIO`, the channel given back), as is one for which no channel can be had. `bdev_nvme_ut` pins the disconnected and the connected case | module/bdev/nvme (nvme_rpc.c), test/unit/lib/bdev/nvme | — | +| 0052 | a raid1 ejection opens its epoch on the raid's cbt — when a current member leaves a raid1 (removed, hot-removed or failed), the cbt stacked on the raid opens an epoch named after the member's UUID, the identity the control plane derives from the replica it placed there, and records from then on every write the member misses, so its return is a delta instead of a full rebuild. 0019 asked each survivor's own name for it, a bdev no cbt sits on, and no epoch ever opened. Only for a member that left current (its superblock slot configured at its peers' generation, not a write-only joiner, not a failed member an earlier removal failed to take), and only while the cbt carries no live epoch: a freeze empties the live bitmap, and an epoch opened after one may start short of writes the member missed just before it left. `bdev_raid_ut` pins the removed and the failed member, and the five ejections that open none | bdev_raid, cbt module (`vbdev_cbt_auto_epoch_open`), test/unit/lib/bdev/raid | 0016, 0019, 0033 | 0005 `#include`s `vbdev_tier.h` and adds `-I module/bdev/tier` to the lvol module CFLAGS via its own Makefile hunk; the Dockerfile injects the module dirs before applying patches (copy-before-apply ordering matters). From eac15e9404d05e9dea6ce4fee355cb6d6a97864e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20DUCOM?= Date: Fri, 2 Oct 2026 09:12:34 +0200 Subject: [PATCH 17/18] fix(raid): a raid1 ejection opens its epoch on the raid's cbt (0052) 0019 opened the epoch at a member's ejection under each survivor's own name. The cbt sits on the raid, not on its members: every call answered -ENODEV, no epoch ever opened, and a member lost without notice was always rebuilt in full. The raid now asks once, for the raid's name and the member's UUID, the identity the control plane derives from the replica it placed there. vbdev_cbt_auto_epoch_open finds the cbt stacked on that bdev and opens the epoch there. An epoch holds the writes a member misses from its departure on, so it opens only for a member that left current: its superblock slot configured at the generation its peers carry, not a write-only joiner, and not a failed member an earlier removal failed to take (a new removal_failed flag, which unlike remove_error survives the retry). The cbt also refuses while it carries any live epoch, frozen or rebuilding included: a freeze empties the live bitmap, and an epoch opened after one may start short of writes the member missed just before it left. Each of those members is rebuilt in full, as before. --- module/bdev/cbt/vbdev_cbt.c | 43 ++++-- module/bdev/cbt/vbdev_cbt_query.h | 22 +-- ...tion-opens-an-epoch-on-the-raids-cbt.patch | 139 ++++++++++++++++++ patches/README.md | 2 +- 4 files changed, 183 insertions(+), 23 deletions(-) diff --git a/module/bdev/cbt/vbdev_cbt.c b/module/bdev/cbt/vbdev_cbt.c index b4221f2..0c11661 100644 --- a/module/bdev/cbt/vbdev_cbt.c +++ b/module/bdev/cbt/vbdev_cbt.c @@ -503,44 +503,61 @@ cbt_epoch_state_name(enum cbt_epoch_state state) } } -/* Epoch-at-ejection — see vbdev_cbt_query.h. Refuses (-EEXIST) when an OPEN - * epoch already bounds the round: an implicit takeover would steal the nonce - * from under the controller. FROZEN/REBUILDING epochs do not block — those are - * prior rounds being digested while the live bitmap keeps tracking. */ +/* The cbt bdev stacked on the bdev named base_bdev_name, if any. */ +static struct vbdev_cbt * +cbt_find_by_base_name(const char *base_bdev_name) +{ + struct vbdev_cbt *node; + + TAILQ_FOREACH(node, &g_cbt_nodes, link) { + if (node->base_bdev != NULL && + strcmp(spdk_bdev_get_name(node->base_bdev), base_bdev_name) == 0) { + return node; + } + } + return NULL; +} + +/* Epoch-at-ejection — see vbdev_cbt_query.h. Refuses (-EEXIST) while the cbt + * carries a live epoch. An open one bounds a round the controller started, and + * taking it over would steal its nonce. A frozen or rebuilding one may have taken + * out of the live bitmap, at its freeze, writes the member missed just before it + * left: an epoch opened now would start short of them. */ int -vbdev_cbt_auto_epoch_open(const char *bdev_name, const char *stale_backend_id) +vbdev_cbt_auto_epoch_open(const char *base_bdev_name, const char *member_id) { struct vbdev_cbt *cbt; struct cbt_epoch *ep; char epoch_id[CBT_EPOCH_ID_MAX]; char nonce[CBT_NONCE_MAX]; + const char *cbt_name; uint64_t max_gen = 0; uint64_t ticks = spdk_get_ticks(); assert(spdk_get_thread() == spdk_thread_get_app_thread()); - cbt = cbt_find_by_name(bdev_name); + cbt = cbt_find_by_base_name(base_bdev_name); if (cbt == NULL) { return -ENODEV; } + if (cbt_any_epoch_open(cbt)) { + return -EEXIST; + } TAILQ_FOREACH(ep, &cbt->epochs, link) { - if (ep->state == CBT_EPOCH_OPEN) { - return -EEXIST; - } if (ep->generation > max_gen) { max_gen = ep->generation; } } + cbt_name = spdk_bdev_get_name(&cbt->cbt_bdev); snprintf(epoch_id, sizeof(epoch_id), "auto-%016" PRIx64, ticks); snprintf(nonce, sizeof(nonce), "auto%08x", (uint32_t)ticks); - SPDK_NOTICELOG("CBT: auto epoch '%s' (nonce %s) on '%s' — member '%s' ejected\n", - epoch_id, nonce, bdev_name, stale_backend_id); + SPDK_NOTICELOG("CBT: auto epoch '%s' (nonce %s) on '%s' — member '%s' left '%s'\n", + epoch_id, nonce, cbt_name, member_id, base_bdev_name); - return bdev_cbt_epoch_open(bdev_name, epoch_id, stale_backend_id, - max_gen + 1, nonce); + return bdev_cbt_epoch_open(cbt_name, epoch_id, member_id, max_gen + 1, nonce); } /* Cross-module query — the raid module publishes these facts per member in ITS diff --git a/module/bdev/cbt/vbdev_cbt_query.h b/module/bdev/cbt/vbdev_cbt_query.h index 0053d7c..b7571d3 100644 --- a/module/bdev/cbt/vbdev_cbt_query.h +++ b/module/bdev/cbt/vbdev_cbt_query.h @@ -33,17 +33,21 @@ struct vbdev_cbt_epoch_facts { int vbdev_cbt_query_latest_epoch(const char *bdev_name, struct vbdev_cbt_epoch_facts *out); /** - * Open a delta epoch bounding an unplanned member loss. The raid module calls - * this on each surviving member's cbt when a member leaves, so the missing - * writes stay a delta instead of degrading to a full rebuild. The generated - * epoch id and nonce are reported through get_bdevs, which is how the - * control-plane adopts the round. + * Open a delta epoch bounding an unplanned member loss, on the cbt bdev stacked + * on the bdev named \c base_bdev_name. The raid module calls this when a current + * member leaves a raid1, with the raid's name and the member's UUID as + * \c member_id: from then on the cbt records every write the member misses, so + * its return is a delta instead of a full rebuild. The generated epoch id and + * nonce are reported through get_bdevs, which is how the control-plane adopts + * the round. * - * \return 0 on success; -EEXIST if an OPEN epoch already tracks the round - * (never take over implicitly); -ENODEV if \c bdev_name is not a cbt - * bdev; other negative errno from the epoch machinery. App thread only. + * \return 0 on success; -EEXIST if the cbt carries a live epoch (open, frozen or + * rebuilding): never take a round over, nor start one short of writes a + * freeze took out of the live bitmap; -ENODEV if no cbt bdev sits on + * \c base_bdev_name; other negative errno from the epoch machinery. App + * thread only. */ -int vbdev_cbt_auto_epoch_open(const char *bdev_name, const char *stale_backend_id); +int vbdev_cbt_auto_epoch_open(const char *base_bdev_name, const char *member_id); /* One dirty range of an epoch, in blocks of the cbt bdev: the raid's own, since * a cbt bdev passes its LBAs through. */ diff --git a/patches/0052-raid1-ejection-opens-an-epoch-on-the-raids-cbt.patch b/patches/0052-raid1-ejection-opens-an-epoch-on-the-raids-cbt.patch index 394ae22..f383feb 100644 --- a/patches/0052-raid1-ejection-opens-an-epoch-on-the-raids-cbt.patch +++ b/patches/0052-raid1-ejection-opens-an-epoch-on-the-raids-cbt.patch @@ -1,3 +1,142 @@ +diff --git a/module/bdev/raid/bdev_raid.c b/module/bdev/raid/bdev_raid.c +index da919f1..22ea08b 100644 +--- a/module/bdev/raid/bdev_raid.c ++++ b/module/bdev/raid/bdev_raid.c +@@ -516,6 +516,7 @@ raid_bdev_free_base_bdev_resource(struct raid_base_bdev_info *base_info) + /* Evariops 0033: reset remove_error with is_failed, so a reused slot never + * inherits the previous incarnation's removal failure. */ + base_info->remove_error = 0; ++ base_info->removal_failed = false; + raid_bdev_base_bdev_state_touch(base_info); + + /* clear `data_offset` to allow it to be recalculated during configuration */ +@@ -2271,12 +2272,69 @@ raid_bdev_remove_base_bdev_write_sb_cb(int status, struct raid_bdev *raid_bdev, + raid_bdev_remove_base_bdev_cont(base_info); + } + ++static uint64_t raid_bdev_sb_max_content_generation(const struct raid_bdev_superblock *sb); ++ ++/* Evariops 0052: whether the member leaving held every write the raid acknowledged ++ * before it left: its superblock slot CONFIGURED at the generation its peers carry ++ * (read before the ejection rewrites the slot), not a write-only joiner, and not a ++ * failed member whose earlier removal failed, which may have missed writes since. */ ++static bool ++raid_bdev_base_bdev_left_current(struct raid_base_bdev_info *base_info) ++{ ++ const struct raid_bdev_superblock *sb = base_info->raid_bdev->sb; ++ uint8_t slot = raid_bdev_base_bdev_slot(base_info); ++ uint8_t i; ++ ++ if (sb == NULL || base_info->write_only_pending || base_info->read_excluded || ++ base_info->removal_failed) { ++ return false; ++ } ++ ++ for (i = 0; i < sb->base_bdevs_size; i++) { ++ const struct raid_bdev_sb_base_bdev *sb_base_bdev = &sb->base_bdevs[i]; ++ ++ if (sb_base_bdev->state == RAID_SB_BASE_BDEV_CONFIGURED && sb_base_bdev->slot == slot) { ++ return sb_base_bdev->content_generation == raid_bdev_sb_max_content_generation(sb); ++ } ++ } ++ ++ return false; ++} ++ ++/* Evariops 0019, 0052: bound the delta the moment a raid1 member leaves. The cbt ++ * bdev stacked on the raid opens an epoch named after the member's UUID, which the ++ * control plane derives from the replica it placed there, and records from then on ++ * every write the member misses. Only for a member that left current: the epoch ++ * holds what it misses from now on, not what it never had. -EEXIST means the cbt ++ * already carries a live epoch, -ENODEV that no cbt sits on the raid; the member is ++ * then rebuilt in full, as it is when it leaves behind. */ ++static void ++raid_bdev_open_ejection_epoch(struct raid_base_bdev_info *base_info) ++{ ++ struct raid_bdev *raid_bdev = base_info->raid_bdev; ++ char member_id[SPDK_UUID_STRING_LEN]; ++ int rc; ++ ++ if (raid_bdev->level != RAID1 || spdk_uuid_is_null(&base_info->uuid) || ++ !raid_bdev_base_bdev_left_current(base_info)) { ++ return; ++ } ++ ++ spdk_uuid_fmt_lower(member_id, sizeof(member_id), &base_info->uuid); ++ rc = vbdev_cbt_auto_epoch_open(raid_bdev->bdev.name, member_id); ++ if (rc != 0 && rc != -EEXIST && rc != -ENODEV) { ++ SPDK_WARNLOG("raid bdev '%s': no epoch for ejected member '%s' (%s): %s; it will be " ++ "rebuilt in full\n", raid_bdev->bdev.name, ++ base_info->name != NULL ? base_info->name : "(null)", member_id, ++ spdk_strerror(-rc)); ++ } ++} ++ + static void + raid_bdev_remove_base_bdev_on_quiesced(void *ctx, int status) + { + struct raid_base_bdev_info *base_info = ctx; + struct raid_bdev *raid_bdev = base_info->raid_bdev; +- struct raid_base_bdev_info *survivor; + + if (status != 0) { + SPDK_ERRLOG("Failed to quiesce raid bdev %s: %s\n", +@@ -2285,28 +2343,7 @@ raid_bdev_remove_base_bdev_on_quiesced(void *ctx, int status) + return; + } + +- /* Evariops 0019: bound the delta the moment the member leaves, by opening +- * an auto epoch on every surviving member's cbt — each survivor then records +- * every post-ejection write. -EEXIST means a live round already bounds it, +- * -ENODEV that the member is not cbt-wrapped; both are expected. */ +- if (raid_bdev->level == RAID1 && base_info->name != NULL) { +- RAID_FOR_EACH_BASE_BDEV(raid_bdev, survivor) { +- int rc; +- +- if (survivor == base_info || !survivor->is_configured || +- survivor->name == NULL) { +- continue; +- } +- rc = vbdev_cbt_auto_epoch_open(survivor->name, base_info->name); +- if (rc != 0 && rc != -EEXIST && rc != -ENODEV) { +- SPDK_WARNLOG("raid bdev '%s': auto epoch on '%s' for " +- "ejected '%s' failed: %s — the debt path " +- "will degrade to a full rebuild (D14)\n", +- raid_bdev->bdev.name, survivor->name, +- base_info->name, spdk_strerror(-rc)); +- } +- } +- } ++ raid_bdev_open_ejection_epoch(base_info); + + if (raid_bdev->sb) { + struct raid_bdev_superblock *sb = raid_bdev->sb; +@@ -2599,6 +2636,9 @@ raid_bdev_fail_base_remove_cb(void *ctx, int status) + * local reflex. Service I/O is unaffected either way: raid1 routes on the + * slot's channel, never on is_failed. */ + base_info->remove_error = status; ++ /* Evariops 0052: unlike remove_error, which the next attempt clears, this ++ * stays until the slot is freed. */ ++ base_info->removal_failed = true; + raid_bdev_base_bdev_state_touch(base_info); + + SPDK_NOTICELOG("Failed to remove failed base bdev '%s' from raid bdev '%s' slot %d: %s (%d) — the member " +diff --git a/module/bdev/raid/bdev_raid.h b/module/bdev/raid/bdev_raid.h +index 947359c..8f5d692 100644 +--- a/module/bdev/raid/bdev_raid.h ++++ b/module/bdev/raid/bdev_raid.h +@@ -116,6 +116,11 @@ struct raid_base_bdev_info { + * the only thing that re-arms the local fail path. */ + int remove_error; + ++ /* Evariops 0052: a removal of this failed member has itself failed. The member ++ * may have missed writes since, so the removal that finally takes it opens no ++ * CBT epoch. Cleared with remove_error when the slot is freed. */ ++ bool removal_failed; ++ + /* callback for base bdev configuration */ + raid_base_bdev_cb configure_cb; + diff --git a/test/unit/lib/bdev/raid/bdev_raid.c/bdev_raid_ut.c b/test/unit/lib/bdev/raid/bdev_raid.c/bdev_raid_ut.c index a488d6d..c359160 100644 --- a/test/unit/lib/bdev/raid/bdev_raid.c/bdev_raid_ut.c diff --git a/patches/README.md b/patches/README.md index 8035ceb..41421bd 100644 --- a/patches/README.md +++ b/patches/README.md @@ -26,7 +26,7 @@ Patches are applied in **lexicographic order of filename** (`0001` … `0052`) | 0016 | raid extended superblock — per-member `content_generation`/`view_epoch` persisted in the same transaction | bdev_raid (SB minor 0→1, carved from reserved bytes) | 0013 | | 0017 | raid per-member observation — `state`/`since`/generations plus live cbt epoch facts in `get_bdevs` | bdev_raid; `#include`s `../cbt/vbdev_cbt_query.h` (requires the cbt module, like 0005 requires tier) | 0013, 0016, cbt module | | 0018 | raid integrated verify — sampled windows compared post-copy under `quiesce_range`, re-copied before declaring DIVERGENT, with a `verified` seal gating `epoch_close(consumed)` | bdev_raid | 0013, 0015, 0016 | -| 0019 | raid auto epoch at member ejection — opens a cbt epoch on the survivors so their tracking bounds the later delta; the nonce is reported in `get_bdevs` | bdev_raid; calls `vbdev_cbt_auto_epoch_open` (cbt module) | 0013, cbt module | +| 0019 | raid auto epoch at member ejection — opens a cbt epoch so its tracking bounds the later delta; the nonce is reported in `get_bdevs`. Asked on the survivors' names, it never opened one: 0052 opens it on the cbt stacked on the raid | bdev_raid; calls `vbdev_cbt_auto_epoch_open` (cbt module) | 0013, cbt module | | 0020 | nvmf audited force-resume — lets the fence path break a standing pause barrier deliberately, leaving an audit record | lib/nvmf (nvmf_pause_rpc.c) | 0003, 0011 | | 0021 | raid envelopes — per-class bandwidth caps × (nominal, maintenance) and a rebuild concurrency bound, via `bdev_raid_set/get_envelopes` | bdev_raid (adds `bdev_raid_envelopes.{c,h}`) | 0013 | | 0022 | raid `verify_ranges` — exhaustive divergence detector, LBA-locked and envelope-paced, which reports and never repairs; raid1 compares copies, raid5f checks the XOR syndrome per stripe | bdev_raid (adds `bdev_raid_verify_ranges.c`) | 0008, 0014, 0015, 0021 | From 34a51639b7fe0ff34fbeba6455d52f896331902f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20DUCOM?= Date: Fri, 2 Oct 2026 10:17:29 +0200 Subject: [PATCH 18/18] feat(cbt): keep the live bitmap in rotating windows, so an ejection's epoch is a delta (bdev_cbt_rotate) The live bitmap is cleared only by bdev_cbt_reset, which needs a moment when every backend is known in sync. A member that leaves without notice gives none, so the epoch the raid opens at its ejection took the whole live bitmap: every write since the last reset. On a volume that went days without an epoch, that is the whole device, and the "delta" rebuild copied all of it (measured on a lab cluster: 81920 of 81920 chunks). bdev_cbt_rotate, called periodically while no epoch is live, moves the live bitmap to a previous-window bitmap (the window before is dropped) and starts it again empty. vbdev_cbt_auto_epoch_open ORs the previous window back before it opens, so its delta holds one to two windows of writes before the departure and everything after. Two rotations are at least CBT_ROTATE_MIN_INTERVAL_US (10 s) apart whoever asks: a write the member missed a few milliseconds before its ejection is always in one of the two bitmaps. A rotation is refused (-EBUSY) while an epoch is live, since that epoch reads the live bitmap whole; a reset clears both. --- module/bdev/cbt/README.md | 2 ++ module/bdev/cbt/vbdev_cbt.c | 52 ++++++++++++++++++++++++++-- module/bdev/cbt/vbdev_cbt.h | 19 ++++++++++ module/bdev/cbt/vbdev_cbt_internal.h | 9 +++++ module/bdev/cbt/vbdev_cbt_rpc.c | 32 +++++++++++++++++ 5 files changed, 112 insertions(+), 2 deletions(-) diff --git a/module/bdev/cbt/README.md b/module/bdev/cbt/README.md index 3d4f7d7..67381b5 100644 --- a/module/bdev/cbt/README.md +++ b/module/bdev/cbt/README.md @@ -55,6 +55,8 @@ The module cannot converge on its own when the write rate approaches rebuild ban Clearing is reset-driven: there is no automatic clear. Once the backend is re-added and all backends are synchronized, the orchestrator MUST call `bdev_cbt_reset`, or the bitmap grows monotonically and "partial" rebuilds degrade toward full-surface copies. +A reset needs a moment when every backend is known in sync, and a member that leaves without notice gives none: the epoch the raid opens at its ejection (`vbdev_cbt_auto_epoch_open`) cannot clear anything, since the writes the member just missed are in the live bitmap. So the live bitmap is also kept in windows: `bdev_cbt_rotate`, called periodically while no epoch is live, moves it to a previous-window bitmap and starts it again empty, and the epoch at ejection takes the previous window back before it opens. Its delta then holds one to two windows of writes before the departure, and everything after, instead of every write since the last reset. Two rotations are at least `CBT_ROTATE_MIN_INTERVAL_US` (10 s) apart whoever asks, so a write missed a few milliseconds before the ejection is always in one of the two bitmaps. A reset clears both. + ## RAID integration The companion patch (`patches/0001-raid-add-skip_rebuild-parameter.patch`) adds a `skip_rebuild` boolean to `bdev_raid_add_base_bdev`. When true, the RAID module skips its full surface rebuild and instead quiesces the raid, opens `base_channel[slot]` on every existing I/O channel for the re-added bdev — without this, existing channels would never write to the backend — then unquiesces and writes the superblock. diff --git a/module/bdev/cbt/vbdev_cbt.c b/module/bdev/cbt/vbdev_cbt.c index 0c11661..359f33b 100644 --- a/module/bdev/cbt/vbdev_cbt.c +++ b/module/bdev/cbt/vbdev_cbt.c @@ -550,6 +550,15 @@ vbdev_cbt_auto_epoch_open(const char *base_bdev_name, const char *member_id) } } + /* The member may have missed writes from just before a rotation: those moved + * to the previous window, and come back into the delta here. With no live + * epoch, no rotation runs until this epoch closes. */ + for (uint64_t i = 0; i < cbt->bitmap_size_bytes; i++) { + if (cbt->bitmap_prev[i] != 0) { + __atomic_fetch_or(&cbt->bitmap[i], cbt->bitmap_prev[i], __ATOMIC_ACQ_REL); + } + } + cbt_name = spdk_bdev_get_name(&cbt->cbt_bdev); snprintf(epoch_id, sizeof(epoch_id), "auto-%016" PRIx64, ticks); snprintf(nonce, sizeof(nonce), "auto%08x", (uint32_t)ticks); @@ -693,6 +702,7 @@ _cbt_device_unregister_cb(void *io_device) struct vbdev_cbt *cbt_node = io_device; free(cbt_node->bitmap); + free(cbt_node->bitmap_prev); free(cbt_node->cbt_bdev.name); struct cbt_epoch *ep; @@ -895,14 +905,18 @@ vbdev_cbt_register(const char *bdev_name) cbt_node->total_blocks = bdev->blockcnt; cbt_node->bitmap = calloc(1, cbt_node->bitmap_size_bytes); - if (!cbt_node->bitmap) { - SPDK_ERRLOG("CBT: bitmap allocation failed (%lu bytes)\n", + cbt_node->bitmap_prev = calloc(1, cbt_node->bitmap_size_bytes); + if (!cbt_node->bitmap || !cbt_node->bitmap_prev) { + SPDK_ERRLOG("CBT: bitmap allocation failed (2 x %lu bytes)\n", (unsigned long)cbt_node->bitmap_size_bytes); spdk_bdev_close(cbt_node->base_desc); + free(cbt_node->bitmap); + free(cbt_node->bitmap_prev); free(cbt_node->cbt_bdev.name); free(cbt_node); return -ENOMEM; } + cbt_node->rotated_at = spdk_get_ticks(); /* ── Copy geometry from base bdev ── */ cbt_node->cbt_bdev.write_cache = bdev->write_cache; @@ -950,6 +964,7 @@ vbdev_cbt_register(const char *bdev_name) TAILQ_REMOVE(&g_cbt_nodes, cbt_node, link); spdk_io_device_unregister(cbt_node, NULL); free(cbt_node->bitmap); + free(cbt_node->bitmap_prev); free(cbt_node->cbt_bdev.name); free(cbt_node); return rc; @@ -963,6 +978,7 @@ vbdev_cbt_register(const char *bdev_name) TAILQ_REMOVE(&g_cbt_nodes, cbt_node, link); spdk_io_device_unregister(cbt_node, NULL); free(cbt_node->bitmap); + free(cbt_node->bitmap_prev); free(cbt_node->cbt_bdev.name); free(cbt_node); return rc; @@ -2542,6 +2558,38 @@ bdev_cbt_reset(const char *cbt_name) __atomic_thread_fence(__ATOMIC_ACQUIRE); memset(cbt->bitmap, 0, cbt->bitmap_size_bytes); + /* The caller vouches every backend in sync through now: the history window + * goes with the rest. */ + memset(cbt->bitmap_prev, 0, cbt->bitmap_size_bytes); + return 0; +} + +int +bdev_cbt_rotate(const char *cbt_name) +{ + struct vbdev_cbt *cbt = cbt_find_by_name(cbt_name); + uint64_t now = spdk_get_ticks(); + + assert(spdk_get_thread() == spdk_thread_get_app_thread()); + + if (!cbt) { + return -ENODEV; + } + /* A live epoch reads the live bitmap whole: moving bits out of it would take + * them out of that epoch's delta. */ + if (cbt_any_epoch_open(cbt)) { + return -EBUSY; + } + if (now - cbt->rotated_at < CBT_ROTATE_MIN_INTERVAL_US * spdk_get_ticks_hz() / SPDK_SEC_TO_USEC) { + return -EAGAIN; + } + + /* Per-byte exchange, as at freeze: an I/O thread OR-ing a bit between a copy + * and a clear would lose it. */ + for (uint64_t i = 0; i < cbt->bitmap_size_bytes; i++) { + cbt->bitmap_prev[i] = __atomic_exchange_n(&cbt->bitmap[i], 0, __ATOMIC_ACQ_REL); + } + cbt->rotated_at = now; return 0; } diff --git a/module/bdev/cbt/vbdev_cbt.h b/module/bdev/cbt/vbdev_cbt.h index e072c62..735a82a 100644 --- a/module/bdev/cbt/vbdev_cbt.h +++ b/module/bdev/cbt/vbdev_cbt.h @@ -24,6 +24,10 @@ extern "C" { #define CBT_EPOCH_ID_MAX 64 #define CBT_BACKEND_ID_MAX 128 #define CBT_NONCE_MAX 32 /* control-plane epoch nonce */ +/* Two rotations of the history window are at least this far apart, whoever asks: + * a write a member missed just before it left must still be in one of the two + * bitmaps when the epoch for that member opens, a few milliseconds later. */ +#define CBT_ROTATE_MIN_INTERVAL_US 10000000 #define CBT_REBUILD_TOKEN_MAX 192 /* consumed-close proof token */ #define CBT_REBUILD_DEFAULT_QD 16 #define CBT_REBUILD_MAX_QD 128 @@ -260,6 +264,21 @@ int bdev_cbt_epoch_get_dirty_ranges(const char *cbt_name, const char *epoch_id, uint32_t *out_chunk_size_kb, bool *out_truncated); +/** + * Start a new history window: the live bitmap moves to the previous-window bitmap + * (dropping the window before it) and starts again empty. Without it, the live + * bitmap holds every write since the last reset, and the epoch opened when a + * member leaves (vbdev_cbt_auto_epoch_open) would copy all of them, on a volume + * that went days without an epoch the whole device. That epoch takes the previous + * window back, so it covers one to two windows before the departure and + * everything after. + * + * \return 0; -EBUSY while an epoch is open, frozen or rebuilding (it reads the + * live bitmap whole); -EAGAIN less than CBT_ROTATE_MIN_INTERVAL_US after + * the previous rotation; -ENODEV if \c cbt_name is not a cbt bdev. + */ +int bdev_cbt_rotate(const char *cbt_name); + /* ── Legacy aliases (deprecated, will be removed in v2) ────────────── */ int bdev_cbt_start_tracking(const char *cbt_name); diff --git a/module/bdev/cbt/vbdev_cbt_internal.h b/module/bdev/cbt/vbdev_cbt_internal.h index 126c0ee..3bdc535 100644 --- a/module/bdev/cbt/vbdev_cbt_internal.h +++ b/module/bdev/cbt/vbdev_cbt_internal.h @@ -33,6 +33,15 @@ struct vbdev_cbt { * bdev_cbt_reset, refused while any epoch is active. Nothing inside the * target can know that every distributed backend is in sync. */ + /* ── History window (bdev_cbt_rotate) ── */ + /* The writes of the previous window, rotated out of the live bitmap while no + * epoch was active. An epoch opened when a member leaves takes them back: + * the writes the member missed just before it left are in one of the two. */ + uint8_t *bitmap_prev; + /* Ticks of the last rotation; rotations are at least + * CBT_ROTATE_MIN_INTERVAL_US apart. */ + uint64_t rotated_at; + /* ── Epoch management ── */ TAILQ_HEAD(, cbt_epoch) epochs; uint64_t epoch_count; diff --git a/module/bdev/cbt/vbdev_cbt_rpc.c b/module/bdev/cbt/vbdev_cbt_rpc.c index 8c8f2d5..148b2ed 100644 --- a/module/bdev/cbt/vbdev_cbt_rpc.c +++ b/module/bdev/cbt/vbdev_cbt_rpc.c @@ -702,6 +702,38 @@ rpc_bdev_cbt_reset(struct spdk_jsonrpc_request *request, } SPDK_RPC_REGISTER("bdev_cbt_reset", rpc_bdev_cbt_reset, SPDK_RPC_RUNTIME) +/* bdev_cbt_rotate: start a new history window (see bdev_cbt_rotate). Answers the + * errno as the JSON-RPC error code, as bdev_cbt_reset does: -EBUSY while an epoch + * is live, -EAGAIN when called again too soon. */ +static void +rpc_bdev_cbt_rotate(struct spdk_jsonrpc_request *request, + const struct spdk_json_val *params) +{ + struct rpc_cbt_name_only req = {NULL}; + int rc; + + if (spdk_json_decode_object(params, rpc_cbt_name_only_decoders, + SPDK_COUNTOF(rpc_cbt_name_only_decoders), &req)) { + spdk_jsonrpc_send_error_response(request, SPDK_JSONRPC_ERROR_INTERNAL_ERROR, + "Failed to decode parameters"); + goto cleanup; + } + + rc = bdev_cbt_rotate(req.name); + if (rc != 0) { + spdk_jsonrpc_send_error_response(request, rc, spdk_strerror(-rc)); + goto cleanup; + } + + struct spdk_json_write_ctx *w = spdk_jsonrpc_begin_result(request); + spdk_json_write_bool(w, true); + spdk_jsonrpc_end_result(request, w); + +cleanup: + free_rpc_cbt_name_only(&req); +} +SPDK_RPC_REGISTER("bdev_cbt_rotate", rpc_bdev_cbt_rotate, SPDK_RPC_RUNTIME) + /* ================================================================== */ /* bdev_cbt_partial_rebuild (async — deferred RPC response) */ /* ================================================================== */