On Thu, Mar 05, 2015 at 09:59:09AM +0000, Filipe David Manana wrote: > On Thu, Mar 5, 2015 at 8:48 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> > > The change itself is ok but the title and associating it only with > fsync are not quite right, since this is much more generic and not > specific to fsync.
How about "catch errors after waiting for transaction"? I'm open to any suggestions. Thanks, -liubo > > thanks > > > --- > > 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; > > + > > btrfs_put_transaction(cur_trans); > > > > return ret; > > -- > > 1.8.1.4 > > > > -- > > 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 > > > > -- > Filipe David Manana, > > "Reasonable men adapt themselves to the world. > Unreasonable men adapt the world to themselves. > That's why all progress depends on unreasonable men." > -- > 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 -- 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
