Skip to contentSkip to conversation

Btrfs: fix old data problem caused by aio vs dio

RFC PATCH Superseded Miao Xie 2 participants 2 messages Jun 19, 2012 Btrfs file system

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Miao Xie

author
#0 lore raw

RFC PATCH 1/1 Btrfs: fix old data problem caused by aio vs dio

The 209th case of xfstests failed because of the race between aio and dio. The detail reason is following:
Task1 Task2 Btrfs-worker
invalidate pages
read pages
do direct io
invalidate pages*
finish ordered io
read data from
pages

* This step failed because the kernel found the ordered extent object that covered the pages and thought the pages were still under busy. And then Task1 read the old data from those pages.

And beside that, I think all the operations including metadata update and bit cleanup of extent state should complete before the dio ends.

This patch fixes the above problem by changing the end io method of dio back to the old one.

---
We have other method to fix this problem, such as making the dio task wait until the worker completes end io operation. But the way of this patch is the simplest one. Josef, what do you think?

---
 fs/btrfs/inode.c |   14 ++++----------
 1 files changed, 4 insertions(+), 10 deletions(-)
fs/btrfs/inode.c +4 -10
index 3f2c8cb..3ff862f 100644
@@ -5988,7 +5988,6 @@ static void btrfs_endio_direct_write(struct bio *bio, int err)
 {
 	struct btrfs_dio_private *dip = bio->bi_private;
 	struct inode *inode = dip->inode;
-	struct btrfs_root *root = BTRFS_I(inode)->root;
 	struct btrfs_ordered_extent *ordered = NULL;
 	u64 ordered_offset = dip->logical_offset;
 	u64 ordered_bytes = dip->bytes;
@@ -6003,10 +6002,7 @@ again:
 	if (!ret)
 		goto out_test;
 
-	ordered->work.func = finish_ordered_fn;
-	ordered->work.flags = 0;
-	btrfs_queue_worker(&root->fs_info->endio_write_workers,
-			   &ordered->work);
+	btrfs_finish_ordered_io(ordered);
 out_test:
 	/*
 	 * our bio might span multiple ordered extents.  If we haven't
@@ -6089,11 +6085,9 @@ static inline int __btrfs_submit_dio_bio(struct bio *bio, struct inode *inode,
 
 	bio_get(bio);
 
-	if (!write) {
-		ret = btrfs_bio_wq_end_io(root->fs_info, bio, 0);
-		if (ret)
-			goto err;
-	}
+	ret = btrfs_bio_wq_end_io(root->fs_info, bio, 0);
+	if (ret)
+		goto err;
 
 	if (skip_sum)
 		goto map;
Signature · 1 line
1.7.6.5
#1 lore raw
Quoted 11 lines · Miao Xie

The 209th case of xfstests failed because of the race between aio and dio. The detail reason is following:
Task1 Task2 Btrfs-worker
invalidate pages
read pages
do direct io
invalidate pages*
finish ordered io
read data from
pages

Quoted 7 lines

* This step failed because the kernel found the ordered extent object that covered the pages and thought the pages were still under busy. And then Task1 read the old data from those pages.

And beside that, I think all the operations including metadata update and bit cleanup of extent state should complete before the dio ends.

Thanks for tracking this one down. I'd really like to keep Josef's change because it makes a very big difference for latencies. The read operation needs to wait until the ordered IO is completely finished.

-chris