[linux-lvm] resend patch - bcache may mistakenly write data to another disk when writes error
Joe Thornber
thornber at redhat.com
Tue Oct 29 11:05:08 UTC 2019
On Tue, Oct 29, 2019 at 09:46:56AM +0000, Heming Zhao wrote:
> Add another comment.
>
> The key of commit 2938b4dcc & 6b0d969b is function _invalidate_fd().
> But from my analysis, it looks your codes do not fix this issue.
>
> _invalidate_fd calls bcache_invalidate_fd & bcache_abort_fd.
> bcache_invalidate_fd work as before, only return true or false to indicate there is or isn't fd in cache->errored.
> Then bcache_abort_fd calls _abort_v, _abort_v calls _unlink_block & _free_block.
> These two functions only delist block from currently cache->errored & cache->free list.
> The data still in radix tree with flags BF_DIRTY.
In _abort_v():
1402│ // We can't remove the block from the radix tree yet because
1│ // we're in the middle of an iteration.
and then after the iteration:
1416│ radix_tree_remove_prefix(cache->rtree, k.bytes, k.bytes + sizeof(k.parts.fd));
I've added a unit test to demonstrate it does indeed work:
840│static void test_abort_forces_reread(void *context)
1│{
2│ struct fixture *f = context;
3│ struct mock_engine *me = f->me;
4│ struct bcache *cache = f->cache;
5│ struct block *b;
6│ int fd = 17;
7│
8│ _expect_read(me, fd, 0);
9│ _expect(me, E_WAIT);
10│ T_ASSERT(bcache_get(cache, fd, 0, GF_DIRTY, &b));
11│ bcache_put(b);
12│
13│ bcache_abort_fd(cache, fd);
14│ T_ASSERT(bcache_flush(cache));
15│
16│ // Check the block is re-read
17│ _expect_read(me, fd, 0);
18│ _expect(me, E_WAIT);
19│ T_ASSERT(bcache_get(cache, fd, 0, 0, &b));
20│ bcache_put(b);
21│}
- Joe
More information about the linux-lvm
mailing list