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. 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
