* [PATCH 2/4] Btrfs: track transaction aborted across the fs_info
2012-05-31 20:04 [PATCH 1/4] Btrfs: wake up transaction waiters when aborting a transaction Josef Bacik
@ 2012-05-31 20:04 ` Josef Bacik
2012-06-01 1:22 ` Liu Bo
2012-05-31 20:04 ` [PATCH 3/4] Btrfs: do not reset the eb ref count to 1 when aborting the transaction Josef Bacik
2012-05-31 20:04 ` [PATCH 4/4] Btrfs: unlock everything properly in the error case for nocow Josef Bacik
2 siblings, 1 reply; 6+ messages in thread
From: Josef Bacik @ 2012-05-31 20:04 UTC (permalink / raw)
To: linux-btrfs
If we abort a transaction during the commit phase we can have people who
start new transactions because we nave no way of signaling to them that
something went wrong. So add a trans_aborted flag to the fs_info so the
start transaction code can be notified that the last transaction was aborted
and it needs to return an error. Thanks,
Signed-off-by: Josef Bacik <josef@redhat.com>
---
fs/btrfs/ctree.h | 1 +
fs/btrfs/super.c | 1 +
fs/btrfs/transaction.c | 14 ++++++++++++++
3 files changed, 16 insertions(+), 0 deletions(-)
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 5f2c6d1..34549ca 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -1213,6 +1213,7 @@ struct btrfs_fs_info {
int log_root_recovering;
int enospc_unlink;
int trans_no_join;
+ int trans_aborted;
u64 total_pinned;
diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
index 85cef50..e0abe7f 100644
--- a/fs/btrfs/super.c
+++ b/fs/btrfs/super.c
@@ -226,6 +226,7 @@ void __btrfs_abort_transaction(struct btrfs_trans_handle *trans,
return;
}
trans->transaction->aborted = errno;
+ root->fs_info->trans_aborted = errno;
__btrfs_std_error(root->fs_info, function, line, errno, NULL);
}
/*
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index 4e6f63e..e292b83 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -77,13 +77,21 @@ loop:
if (cur_trans->aborted) {
spin_unlock(&root->fs_info->trans_lock);
return cur_trans->aborted;
+ } else if (root->fs_info->trans_aborted) {
+ WARN_ON(1);
+ spin_unlock(&root->fs_info->trans_lock);
+ return root->fs_info->trans_aborted;
}
atomic_inc(&cur_trans->use_count);
atomic_inc(&cur_trans->num_writers);
cur_trans->num_joined++;
spin_unlock(&root->fs_info->trans_lock);
return 0;
+ } else if (root->fs_info->trans_aborted) {
+ spin_unlock(&root->fs_info->trans_lock);
+ return root->fs_info->trans_aborted;
}
+
spin_unlock(&root->fs_info->trans_lock);
cur_trans = kmem_cache_alloc(btrfs_transaction_cachep, GFP_NOFS);
@@ -99,6 +107,10 @@ loop:
kmem_cache_free(btrfs_transaction_cachep, cur_trans);
cur_trans = root->fs_info->running_transaction;
goto loop;
+ } else if (root->fs_info->trans_aborted) {
+ spin_unlock(&root->fs_info->trans_lock);
+ kmem_cache_free(btrfs_transaction_cachep, cur_trans);
+ return root->fs_info->trans_aborted;
}
atomic_set(&cur_trans->num_writers, 1);
@@ -1209,6 +1221,8 @@ static void cleanup_transaction(struct btrfs_trans_handle *trans,
root->fs_info->running_transaction = NULL;
root->fs_info->trans_no_join = 0;
}
+ if (!root->fs_info->trans_aborted)
+ root->fs_info->trans_aborted = -EROFS;
spin_unlock(&root->fs_info->trans_lock);
btrfs_cleanup_one_transaction(trans->transaction, root);
--
1.7.7.6
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH 2/4] Btrfs: track transaction aborted across the fs_info
2012-05-31 20:04 ` [PATCH 2/4] Btrfs: track transaction aborted across the fs_info Josef Bacik
@ 2012-06-01 1:22 ` Liu Bo
2012-06-01 12:46 ` Josef Bacik
0 siblings, 1 reply; 6+ messages in thread
From: Liu Bo @ 2012-06-01 1:22 UTC (permalink / raw)
To: Josef Bacik; +Cc: linux-btrfs
On 06/01/2012 04:04 AM, Josef Bacik wrote:
> If we abort a transaction during the commit phase we can have people who
> start new transactions because we nave no way of signaling to them that
> something went wrong. So add a trans_aborted flag to the fs_info so the
> start transaction code can be notified that the last transaction was aborted
> and it needs to return an error. Thanks,
>
We already have a (root->fs_info->fs_state & BTRFS_SUPER_FLAG_ERROR),
why don't just use that?
thanks,
liubo
> Signed-off-by: Josef Bacik <josef@redhat.com>
> ---
> fs/btrfs/ctree.h | 1 +
> fs/btrfs/super.c | 1 +
> fs/btrfs/transaction.c | 14 ++++++++++++++
> 3 files changed, 16 insertions(+), 0 deletions(-)
>
> diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
> index 5f2c6d1..34549ca 100644
> --- a/fs/btrfs/ctree.h
> +++ b/fs/btrfs/ctree.h
> @@ -1213,6 +1213,7 @@ struct btrfs_fs_info {
> int log_root_recovering;
> int enospc_unlink;
> int trans_no_join;
> + int trans_aborted;
>
> u64 total_pinned;
>
> diff --git a/fs/btrfs/super.c b/fs/btrfs/super.c
> index 85cef50..e0abe7f 100644
> --- a/fs/btrfs/super.c
> +++ b/fs/btrfs/super.c
> @@ -226,6 +226,7 @@ void __btrfs_abort_transaction(struct btrfs_trans_handle *trans,
> return;
> }
> trans->transaction->aborted = errno;
> + root->fs_info->trans_aborted = errno;
> __btrfs_std_error(root->fs_info, function, line, errno, NULL);
> }
> /*
> diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
> index 4e6f63e..e292b83 100644
> --- a/fs/btrfs/transaction.c
> +++ b/fs/btrfs/transaction.c
> @@ -77,13 +77,21 @@ loop:
> if (cur_trans->aborted) {
> spin_unlock(&root->fs_info->trans_lock);
> return cur_trans->aborted;
> + } else if (root->fs_info->trans_aborted) {
> + WARN_ON(1);
> + spin_unlock(&root->fs_info->trans_lock);
> + return root->fs_info->trans_aborted;
> }
> atomic_inc(&cur_trans->use_count);
> atomic_inc(&cur_trans->num_writers);
> cur_trans->num_joined++;
> spin_unlock(&root->fs_info->trans_lock);
> return 0;
> + } else if (root->fs_info->trans_aborted) {
> + spin_unlock(&root->fs_info->trans_lock);
> + return root->fs_info->trans_aborted;
> }
> +
> spin_unlock(&root->fs_info->trans_lock);
>
> cur_trans = kmem_cache_alloc(btrfs_transaction_cachep, GFP_NOFS);
> @@ -99,6 +107,10 @@ loop:
> kmem_cache_free(btrfs_transaction_cachep, cur_trans);
> cur_trans = root->fs_info->running_transaction;
> goto loop;
> + } else if (root->fs_info->trans_aborted) {
> + spin_unlock(&root->fs_info->trans_lock);
> + kmem_cache_free(btrfs_transaction_cachep, cur_trans);
> + return root->fs_info->trans_aborted;
> }
>
> atomic_set(&cur_trans->num_writers, 1);
> @@ -1209,6 +1221,8 @@ static void cleanup_transaction(struct btrfs_trans_handle *trans,
> root->fs_info->running_transaction = NULL;
> root->fs_info->trans_no_join = 0;
> }
> + if (!root->fs_info->trans_aborted)
> + root->fs_info->trans_aborted = -EROFS;
> spin_unlock(&root->fs_info->trans_lock);
>
> btrfs_cleanup_one_transaction(trans->transaction, root);
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH 2/4] Btrfs: track transaction aborted across the fs_info
2012-06-01 1:22 ` Liu Bo
@ 2012-06-01 12:46 ` Josef Bacik
0 siblings, 0 replies; 6+ messages in thread
From: Josef Bacik @ 2012-06-01 12:46 UTC (permalink / raw)
To: Liu Bo; +Cc: Josef Bacik, linux-btrfs
On Fri, Jun 01, 2012 at 09:22:53AM +0800, Liu Bo wrote:
> On 06/01/2012 04:04 AM, Josef Bacik wrote:
>
> > If we abort a transaction during the commit phase we can have people who
> > start new transactions because we nave no way of signaling to them that
> > something went wrong. So add a trans_aborted flag to the fs_info so the
> > start transaction code can be notified that the last transaction was aborted
> > and it needs to return an error. Thanks,
> >
>
>
> We already have a (root->fs_info->fs_state & BTRFS_SUPER_FLAG_ERROR),
> why don't just use that?
>
Ah I missed all of that, will do, thanks,
Josef
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 3/4] Btrfs: do not reset the eb ref count to 1 when aborting the transaction
2012-05-31 20:04 [PATCH 1/4] Btrfs: wake up transaction waiters when aborting a transaction Josef Bacik
2012-05-31 20:04 ` [PATCH 2/4] Btrfs: track transaction aborted across the fs_info Josef Bacik
@ 2012-05-31 20:04 ` Josef Bacik
2012-05-31 20:04 ` [PATCH 4/4] Btrfs: unlock everything properly in the error case for nocow Josef Bacik
2 siblings, 0 replies; 6+ messages in thread
From: Josef Bacik @ 2012-05-31 20:04 UTC (permalink / raw)
To: linux-btrfs
This screws lots of things up and is just plain unnecessary, everything will
get cleaned up properly on it's own. Most notably this was causing root
eb's to be evicted forcefully which meant that any readers still doing
anything would block trying to grab the root eb since it had a ref count of
0. Thanks,
Signed-off-by: Josef Bacik <josef@redhat.com>
---
fs/btrfs/disk-io.c | 4 +---
1 files changed, 1 insertions(+), 3 deletions(-)
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 050db9b..b147a86 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -3519,11 +3519,9 @@ static int btrfs_destroy_marked_extents(struct btrfs_root *root,
&(&BTRFS_I(page->mapping->host)->io_tree)->buffer,
offset >> PAGE_CACHE_SHIFT);
spin_unlock(&dirty_pages->buffer_lock);
- if (eb) {
+ if (eb)
ret = test_and_clear_bit(EXTENT_BUFFER_DIRTY,
&eb->bflags);
- atomic_set(&eb->refs, 1);
- }
if (PageWriteback(page))
end_page_writeback(page);
--
1.7.7.6
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 4/4] Btrfs: unlock everything properly in the error case for nocow
2012-05-31 20:04 [PATCH 1/4] Btrfs: wake up transaction waiters when aborting a transaction Josef Bacik
2012-05-31 20:04 ` [PATCH 2/4] Btrfs: track transaction aborted across the fs_info Josef Bacik
2012-05-31 20:04 ` [PATCH 3/4] Btrfs: do not reset the eb ref count to 1 when aborting the transaction Josef Bacik
@ 2012-05-31 20:04 ` Josef Bacik
2 siblings, 0 replies; 6+ messages in thread
From: Josef Bacik @ 2012-05-31 20:04 UTC (permalink / raw)
To: linux-btrfs
I was getting hung on umount when a transaction was aborted because a range
of one of the free space inodes was still locked. This is because the nocow
stuff doesn't unlock anything on error. This fixed the problem and I
verified that is what was happening. Thanks,
Signed-off-by: Josef Bacik <josef@redhat.com>
---
fs/btrfs/inode.c | 37 +++++++++++++++++++++++++++++++++++--
1 files changed, 35 insertions(+), 2 deletions(-)
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index e91f985..96b841d 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -1136,8 +1136,18 @@ static noinline int run_delalloc_nocow(struct inode *inode,
u64 ino = btrfs_ino(inode);
path = btrfs_alloc_path();
- if (!path)
+ if (!path) {
+ extent_clear_unlock_delalloc(inode,
+ &BTRFS_I(inode)->io_tree,
+ start, end, locked_page,
+ EXTENT_CLEAR_UNLOCK_PAGE |
+ EXTENT_CLEAR_UNLOCK |
+ EXTENT_CLEAR_DELALLOC |
+ EXTENT_CLEAR_DIRTY |
+ EXTENT_SET_WRITEBACK |
+ EXTENT_END_WRITEBACK);
return -ENOMEM;
+ }
nolock = btrfs_is_free_space_inode(root, inode);
@@ -1147,6 +1157,15 @@ static noinline int run_delalloc_nocow(struct inode *inode,
trans = btrfs_join_transaction(root);
if (IS_ERR(trans)) {
+ extent_clear_unlock_delalloc(inode,
+ &BTRFS_I(inode)->io_tree,
+ start, end, locked_page,
+ EXTENT_CLEAR_UNLOCK_PAGE |
+ EXTENT_CLEAR_UNLOCK |
+ EXTENT_CLEAR_DELALLOC |
+ EXTENT_CLEAR_DIRTY |
+ EXTENT_SET_WRITEBACK |
+ EXTENT_END_WRITEBACK);
btrfs_free_path(path);
return PTR_ERR(trans);
}
@@ -1327,8 +1346,11 @@ out_check:
}
btrfs_release_path(path);
- if (cur_offset <= end && cow_start == (u64)-1)
+ if (cur_offset <= end && cow_start == (u64)-1) {
cow_start = cur_offset;
+ cur_offset = end;
+ }
+
if (cow_start != (u64)-1) {
ret = cow_file_range(inode, locked_page, cow_start, end,
page_started, nr_written, 1);
@@ -1347,6 +1369,17 @@ error:
if (!ret)
ret = err;
+ if (ret && cur_offset < end)
+ extent_clear_unlock_delalloc(inode,
+ &BTRFS_I(inode)->io_tree,
+ cur_offset, end, locked_page,
+ EXTENT_CLEAR_UNLOCK_PAGE |
+ EXTENT_CLEAR_UNLOCK |
+ EXTENT_CLEAR_DELALLOC |
+ EXTENT_CLEAR_DIRTY |
+ EXTENT_SET_WRITEBACK |
+ EXTENT_END_WRITEBACK);
+
btrfs_free_path(path);
return ret;
}
--
1.7.7.6
^ permalink raw reply related [flat|nested] 6+ messages in thread