From 15a492fe57ff71e886329ef0ce3a653a081ea5f8 Mon Sep 17 00:00:00 2001 From: Zach Brown Date: Mon, 5 Aug 2019 16:06:26 -0700 Subject: [PATCH] scoutfs: always dirty parents when migrating In a previous commit ("1bd094f scoutfs: migrate dirty btree blocks during wrap") we fixed a bug where we wouldn't migrate blocks from the old half of the ring because they were already dirty in memory. The fix accidentally introduced the case where we wouldn't dirty blocks when migrating if they were already in the current half. We always have to dirty parent blocks when migrating because we might need to modify them to reference the new location of child blocks that are migrated. This bug meant that we'd modify clean blocks in memory which would never make it to the persistent copy. The system could survive as long as it never read that block back from its persistent location. To see the corruption you'd either need tall btrees to be shared between mounts or you'd need one mount to evict its clean (actually modified) cached btree block under memory pressure and then try to read it back. Signed-off-by: Zach Brown --- kmod/src/btree.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/kmod/src/btree.c b/kmod/src/btree.c index c680579b..1cf35586 100644 --- a/kmod/src/btree.c +++ b/kmod/src/btree.c @@ -701,16 +701,20 @@ retry: } /* - * We don't need to cow the exiting block if we're not - * dirtying the block, or we're not migrating and it's - * already dirty in this transaction, or we're - * migrating and it's already in the current half. + * We need to create a new dirty copy of the block if + * the caller asked for it. If the block is already + * dirty then we can return it if either we're not + * migrating so it doesn't matter which half it's in, or + * we're migrating and the dirty block is already in the + * second half. We can be migrating into a new half + * while blocks are still dirty in the old half. And we + * always have to dirty parent blocks in the current + * half in case we need to dirty their children. */ if (!(flags & BTW_DIRTY) || - (!(flags & BTW_MIGRATE) && - (le64_to_cpu(bt->hdr.seq) >= bti->first_dirty_seq)) || - ((flags & BTW_MIGRATE) && - blkno_is_current(bring, le64_to_cpu(ref->blkno)))) { + ((le64_to_cpu(bt->hdr.seq) >= bti->first_dirty_seq) && + (!(flags & BTW_MIGRATE) || + blkno_is_current(bring, le64_to_cpu(ref->blkno))))) { ret = 0; goto out; }