Re: [PATCH V2] Btrfs: catch transaction abortion after waiting for it

[Chris Mason <clm@fb.com>]

On Thu, Mar 5, 2015 at 5:36 AM, Liu Bo <bo.li.liu@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.liu@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