You are right. I forgot the radix_tree_remove_prefix().
The radix_tree_remove_prefix() is called both bcache_invalidate_fd & bcache_abort_fd.
So the job will not work as expected in bcache_abort_fd, because the node is already removed.
Please correct me if my thinking is wrong.
On 10/29/19 7:05 PM, Joe Thornber wrote:
> 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
>
_______________________________________________
linux-lvm mailing list
linux-lvm@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/linux-lvm
read the LVM HOW-TO at http://tldp.org/HOWTO/LVM-HOWTO/
