| From 64c12921e11b3a0c10d088606e328c58e29274d8 Mon Sep 17 00:00:00 2001 |
| From: Jeff Mahoney <jeffm@suse.com> |
| Date: Wed, 8 Jun 2016 00:36:38 -0400 |
| Subject: btrfs: account for non-CoW'd blocks in btrfs_abort_transaction |
| |
| From: Jeff Mahoney <jeffm@suse.com> |
| |
| commit 64c12921e11b3a0c10d088606e328c58e29274d8 upstream. |
| |
| The test for !trans->blocks_used in btrfs_abort_transaction is |
| insufficient to determine whether it's safe to drop the transaction |
| handle on the floor. btrfs_cow_block, informed by should_cow_block, |
| can return blocks that have already been CoW'd in the current |
| transaction. trans->blocks_used is only incremented for new block |
| allocations. If an operation overlaps the blocks in the current |
| transaction entirely and must abort the transaction, we'll happily |
| let it clean up the trans handle even though it may have modified |
| the blocks and will commit an incomplete operation. |
| |
| In the long-term, I'd like to do closer tracking of when the fs |
| is actually modified so we can still recover as gracefully as possible, |
| but that approach will need some discussion. In the short term, |
| since this is the only code using trans->blocks_used, let's just |
| switch it to a bool indicating whether any blocks were used and set |
| it when should_cow_block returns false. |
| |
| Signed-off-by: Jeff Mahoney <jeffm@suse.com> |
| Reviewed-by: Filipe Manana <fdmanana@suse.com> |
| Signed-off-by: David Sterba <dsterba@suse.com> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| fs/btrfs/ctree.c | 5 ++++- |
| fs/btrfs/extent-tree.c | 2 +- |
| fs/btrfs/super.c | 2 +- |
| fs/btrfs/transaction.h | 2 +- |
| 4 files changed, 7 insertions(+), 4 deletions(-) |
| |
| --- a/fs/btrfs/ctree.c |
| +++ b/fs/btrfs/ctree.c |
| @@ -1551,6 +1551,7 @@ noinline int btrfs_cow_block(struct btrf |
| trans->transid, root->fs_info->generation); |
| |
| if (!should_cow_block(trans, root, buf)) { |
| + trans->dirty = true; |
| *cow_ret = buf; |
| return 0; |
| } |
| @@ -2773,8 +2774,10 @@ again: |
| * then we don't want to set the path blocking, |
| * so we test it here |
| */ |
| - if (!should_cow_block(trans, root, b)) |
| + if (!should_cow_block(trans, root, b)) { |
| + trans->dirty = true; |
| goto cow_done; |
| + } |
| |
| /* |
| * must have write locks on this node and the |
| --- a/fs/btrfs/extent-tree.c |
| +++ b/fs/btrfs/extent-tree.c |
| @@ -7856,7 +7856,7 @@ btrfs_init_new_buffer(struct btrfs_trans |
| set_extent_dirty(&trans->transaction->dirty_pages, buf->start, |
| buf->start + buf->len - 1, GFP_NOFS); |
| } |
| - trans->blocks_used++; |
| + trans->dirty = true; |
| /* this returns a buffer locked for blocking */ |
| return buf; |
| } |
| --- a/fs/btrfs/super.c |
| +++ b/fs/btrfs/super.c |
| @@ -239,7 +239,7 @@ void __btrfs_abort_transaction(struct bt |
| trans->aborted = errno; |
| /* Nothing used. The other threads that have joined this |
| * transaction may be able to continue. */ |
| - if (!trans->blocks_used && list_empty(&trans->new_bgs)) { |
| + if (!trans->dirty && list_empty(&trans->new_bgs)) { |
| const char *errstr; |
| |
| errstr = btrfs_decode_error(errno); |
| --- a/fs/btrfs/transaction.h |
| +++ b/fs/btrfs/transaction.h |
| @@ -110,7 +110,6 @@ struct btrfs_trans_handle { |
| u64 chunk_bytes_reserved; |
| unsigned long use_count; |
| unsigned long blocks_reserved; |
| - unsigned long blocks_used; |
| unsigned long delayed_ref_updates; |
| struct btrfs_transaction *transaction; |
| struct btrfs_block_rsv *block_rsv; |
| @@ -121,6 +120,7 @@ struct btrfs_trans_handle { |
| bool can_flush_pending_bgs; |
| bool reloc_reserved; |
| bool sync; |
| + bool dirty; |
| unsigned int type; |
| /* |
| * this root is only needed to validate that the root passed to |