On Thu, Mar 05, 2015 at 08:59:57AM -0500, Chris Mason wrote: > > > On Thu, Mar 5, 2015 at 5:36 AM, Liu Bo <bo.li.liu@xxxxxxxxxx> 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.liu@xxxxxxxxxx> > >--- > >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? It should be not necessary, I just copied it from the first check in btrfs_commit_transaction(), not insisting in using it. Thanks, -liubo -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
