On Thu, Mar 5, 2015 at 5:36 AM, Liu Bo <bo.li....@oracle.com> wrote:
This problem is uncovered by a test case:
http://patchwork.ozlabs.org/patch/244297.
Fsync() can report success when it actually doesn't. When we
have several threads running fsync() at the same tiem and in one
fsync() we
get a transaction abortion due to some problems(in the test case it's
disk
failures), and other fsync()s may return successfully which makes
userspace
programs think that data is now safely flushed into disk.
It's because that after fsyncs() fail btrfs_sync_log() due to disk
failures,
they get to try btrfs_commit_transaction() where it finds that there
is
already a transaction being committed, and they'll just call
wait_for_commit()
and return. Note that we actually check "trans->aborted" in
btrfs_end_transaction,
but it's likely that the error message is still not yet throwed out
and only after
wait_for_commit() we're sure whether the transaction is committed
successfully.
This add the necessary check and it now passes the test.
Signed-off-by: Liu Bo <bo.li....@oracle.com>
---
v2: Use a more generic title since it's not only for fsync, but for
others.
fs/btrfs/transaction.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index 7e80f32..bd7ea86 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -1814,6 +1814,9 @@ int btrfs_commit_transaction(struct
btrfs_trans_handle *trans,
wait_for_commit(root, cur_trans);
+ if (unlikely(ACCESS_ONCE(cur_trans->aborted)))
+ ret = cur_trans->aborted;
+
Thanks Liu, but why are we using ACCESS_ONCE here?
-chris
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html