Re: [PATCH 1/2] Btrfs: fix a deadlock in aborting transaction due to ENOSPC
On mon, 19 Nov 2012 15:58:55 +0800, Liu Bo wrote: > On Fri, Nov 16, 2012 at 03:19:49PM +0100, David Sterba wrote: >> On Mon, Nov 05, 2012 at 08:42:08PM +0800, Liu Bo wrote: >>> When committing a transaction, we may bail out of running delayed refs >>> due to ENOSPC, and then abort the current transaction to flip into readonly. >>> >>> But we'll hit a deadlock on ref head's lock since we forget to release >>> its lock and other cleanup stuff. >> >> Miao sent a patch for this on Tue, 08 May 2012 >> >> http://www.mail-archive.com/linux-btrfs@vger.kernel.org/msg16361.html >> >> and it does more things than just the unlock, I can't tell which one of >> the patches is the right one, only letting you know. >> > > Thanks for pointing that, David. Yes, that patch will work. > > Miao, can you send it here again? > [PATCH] Btrfs: fix deadlock when the process of delayed refs fails OK, I will send it out again. Thanks Miao > > thanks, > liubo > > >> david >> -- >> 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 > -- 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
[PATCH] Btrfs-progs: Fix a segmentation fault in btrfstune when is invalid.
When open_ctree(device, ...) failed, the return value should be checked.
Signed-off-by: Cheng Yang
---
btrfstune.c |2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/btrfstune.c b/btrfstune.c
index 47830c5..07648d5 100644
--- a/btrfstune.c
+++ b/btrfstune.c
@@ -107,6 +107,8 @@ int main(int argc, char *argv[])
}
root = open_ctree(device, 0, 1);
+ if (!root)
+ return 1;
if (seeding_flag) {
ret = update_seeding_flag(root, seeding_value);
--
1.7.7.6
--
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
[PATCH RESEND] Btrfs: fix deadlock when the process of delayed refs fails
Delayed ref mutexes are taken inside btrfs_commit_transaction. A later call
fails and jumps to the cleanup_transaction label with these mutexes still held
causing deadlock when they are reacquired.
Fix this problem by unlocking those mutexes at the suitable place.
Reported-by: Daniel J Blueman
Signed-off-by: Miao Xie
---
fs/btrfs/delayed-ref.c | 8
fs/btrfs/delayed-ref.h | 6 ++
fs/btrfs/extent-tree.c | 25 ++---
3 files changed, 32 insertions(+), 7 deletions(-)
diff --git a/fs/btrfs/delayed-ref.c b/fs/btrfs/delayed-ref.c
index ae94117..364cb90 100644
--- a/fs/btrfs/delayed-ref.c
+++ b/fs/btrfs/delayed-ref.c
@@ -422,6 +422,14 @@ again:
return 1;
}
+void btrfs_release_ref_cluster(struct list_head *cluster)
+{
+ struct list_head *pos, *q;
+
+ list_for_each_safe(pos, q, cluster)
+ list_del_init(pos);
+}
+
/*
* helper function to update an extent delayed ref in the
* rbtree. existing and update must both have the same
diff --git a/fs/btrfs/delayed-ref.h b/fs/btrfs/delayed-ref.h
index ab53005..fcc5a29 100644
--- a/fs/btrfs/delayed-ref.h
+++ b/fs/btrfs/delayed-ref.h
@@ -176,8 +176,14 @@ struct btrfs_delayed_ref_head *
btrfs_find_delayed_ref_head(struct btrfs_trans_handle *trans, u64 bytenr);
int btrfs_delayed_ref_lock(struct btrfs_trans_handle *trans,
struct btrfs_delayed_ref_head *head);
+static inline void btrfs_delayed_ref_unlock(struct btrfs_delayed_ref_head
*head)
+{
+ mutex_unlock(&head->mutex);
+}
+
int btrfs_find_ref_cluster(struct btrfs_trans_handle *trans,
struct list_head *cluster, u64 search_start);
+void btrfs_release_ref_cluster(struct list_head *cluster);
int btrfs_check_delayed_seq(struct btrfs_fs_info *fs_info,
struct btrfs_delayed_ref_root *delayed_refs,
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 3d3e2c1..43a58cb 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -2160,7 +2160,6 @@ static int run_one_delayed_ref(struct btrfs_trans_handle
*trans,
node->num_bytes);
}
}
- mutex_unlock(&head->mutex);
return ret;
}
@@ -2275,7 +2274,7 @@ static noinline int run_clustered_refs(struct
btrfs_trans_handle *trans,
* process of being added. Don't run this ref yet.
*/
list_del_init(&locked_ref->cluster);
- mutex_unlock(&locked_ref->mutex);
+ btrfs_delayed_ref_unlock(locked_ref);
locked_ref = NULL;
delayed_refs->num_heads_ready++;
spin_unlock(&delayed_refs->lock);
@@ -2316,14 +2315,12 @@ static noinline int run_clustered_refs(struct
btrfs_trans_handle *trans,
if (ret) {
printk(KERN_DEBUG "btrfs:
run_delayed_extent_op returned %d\n", ret);
spin_lock(&delayed_refs->lock);
+ btrfs_delayed_ref_unlock(locked_ref);
return ret;
}
goto next;
}
-
- list_del_init(&locked_ref->cluster);
- locked_ref = NULL;
}
ref->in_tree = 0;
@@ -2350,11 +2347,24 @@ static noinline int run_clustered_refs(struct
btrfs_trans_handle *trans,
ret = run_one_delayed_ref(trans, root, ref, extent_op,
must_insert_reserved);
-
- btrfs_put_delayed_ref(ref);
kfree(extent_op);
count++;
+ /*
+* If this node is a head, we will pick the next head to deal
+* with. If there is something wrong when we process the
+* delayed ref, we will end our operation. So in these two
+* cases, we have to unlock the head and drop it from the
+* cluster list before we release it though the code is ugly.
+*/
+ if (btrfs_delayed_ref_is_head(ref) || ret) {
+ list_del_init(&locked_ref->cluster);
+ btrfs_delayed_ref_unlock(locked_ref);
+ locked_ref = NULL;
+ }
+
+ btrfs_put_delayed_ref(ref);
+
if (ret) {
printk(KERN_DEBUG "btrfs: run_one_delayed_ref returned
%d\n", ret);
spin_lock(&delayed_refs->lock);
@@ -2510,6 +2520,7 @@ again:
ret = run_clustered_refs(trans, root, &cluster);
if (ret < 0) {
+ btrfs_release_ref_cluster(&cluster);
Re: [PATCH RESEND] Btrfs: fix deadlock when the process of delayed refs fails
On Mon, Nov 19, 2012 at 05:20:06PM +0800, Miao Xie wrote:
> Delayed ref mutexes are taken inside btrfs_commit_transaction. A later call
> fails and jumps to the cleanup_transaction label with these mutexes still held
> causing deadlock when they are reacquired.
>
> Fix this problem by unlocking those mutexes at the suitable place.
>
> Reported-by: Daniel J Blueman
> Signed-off-by: Miao Xie
> ---
> fs/btrfs/delayed-ref.c | 8
> fs/btrfs/delayed-ref.h | 6 ++
> fs/btrfs/extent-tree.c | 25 ++---
> 3 files changed, 32 insertions(+), 7 deletions(-)
>
> diff --git a/fs/btrfs/delayed-ref.c b/fs/btrfs/delayed-ref.c
> index ae94117..364cb90 100644
> --- a/fs/btrfs/delayed-ref.c
> +++ b/fs/btrfs/delayed-ref.c
> @@ -422,6 +422,14 @@ again:
> return 1;
> }
>
> +void btrfs_release_ref_cluster(struct list_head *cluster)
> +{
> + struct list_head *pos, *q;
> +
> + list_for_each_safe(pos, q, cluster)
> + list_del_init(pos);
> +}
> +
> /*
> * helper function to update an extent delayed ref in the
> * rbtree. existing and update must both have the same
> diff --git a/fs/btrfs/delayed-ref.h b/fs/btrfs/delayed-ref.h
> index ab53005..fcc5a29 100644
> --- a/fs/btrfs/delayed-ref.h
> +++ b/fs/btrfs/delayed-ref.h
> @@ -176,8 +176,14 @@ struct btrfs_delayed_ref_head *
> btrfs_find_delayed_ref_head(struct btrfs_trans_handle *trans, u64 bytenr);
> int btrfs_delayed_ref_lock(struct btrfs_trans_handle *trans,
> struct btrfs_delayed_ref_head *head);
> +static inline void btrfs_delayed_ref_unlock(struct btrfs_delayed_ref_head
> *head)
> +{
> + mutex_unlock(&head->mutex);
> +}
> +
> int btrfs_find_ref_cluster(struct btrfs_trans_handle *trans,
> struct list_head *cluster, u64 search_start);
> +void btrfs_release_ref_cluster(struct list_head *cluster);
>
> int btrfs_check_delayed_seq(struct btrfs_fs_info *fs_info,
> struct btrfs_delayed_ref_root *delayed_refs,
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 3d3e2c1..43a58cb 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -2160,7 +2160,6 @@ static int run_one_delayed_ref(struct
> btrfs_trans_handle *trans,
> node->num_bytes);
> }
> }
> - mutex_unlock(&head->mutex);
I think code can be clean here if we keep this mutex_unlock(), see below,
please.
> return ret;
> }
>
> @@ -2275,7 +2274,7 @@ static noinline int run_clustered_refs(struct
> btrfs_trans_handle *trans,
>* process of being added. Don't run this ref yet.
>*/
> list_del_init(&locked_ref->cluster);
> - mutex_unlock(&locked_ref->mutex);
> + btrfs_delayed_ref_unlock(locked_ref);
> locked_ref = NULL;
> delayed_refs->num_heads_ready++;
> spin_unlock(&delayed_refs->lock);
> @@ -2316,14 +2315,12 @@ static noinline int run_clustered_refs(struct
> btrfs_trans_handle *trans,
> if (ret) {
> printk(KERN_DEBUG "btrfs:
> run_delayed_extent_op returned %d\n", ret);
> spin_lock(&delayed_refs->lock);
> + btrfs_delayed_ref_unlock(locked_ref);
> return ret;
> }
>
> goto next;
> }
> -
> - list_del_init(&locked_ref->cluster);
> - locked_ref = NULL;
> }
>
> ref->in_tree = 0;
> @@ -2350,11 +2347,24 @@ static noinline int run_clustered_refs(struct
> btrfs_trans_handle *trans,
>
> ret = run_one_delayed_ref(trans, root, ref, extent_op,
> must_insert_reserved);
> -
> - btrfs_put_delayed_ref(ref);
> kfree(extent_op);
> count++;
>
> + /*
> + * If this node is a head, we will pick the next head to deal
> + * with. If there is something wrong when we process the
> + * delayed ref, we will end our operation. So in these two
> + * cases, we have to unlock the head and drop it from the
> + * cluster list before we release it though the code is ugly.
> + */
> + if (btrfs_delayed_ref_is_head(ref) || ret) {
> + list_del_init(&locked_ref->cluster);
> + btrfs_delayed_ref_unlock(locked_ref);
> + locked_ref = NULL;
> + }
> +
In case that we don't remove mutex_unlock above,
if ret is non-zero, either
A)locked_ref is not NULL, or
B)locked_ref is NULL, and it has done list
Re: btrfs crash - Null dereference - 3.7.0-rc5-00068-gc5e35d6
On Mon, Nov 19, 2012 at 12:55:40AM -0200, Gustavo Padovan wrote: > Hi, > > my system suddenly crashed and gave me this dump: > > http://imgur.com/oO6S0 > > I checked and there is not btrfs commit in linus' tree since I compiled this > kernel. > Hi Gustavo, It's weird that NULL pointer oops happens here. Since you own the kernel, you may also have debuginfo left, can you please run 'gdb fs/btrfs/btrfs.ko' and 'list *block_rsv_release_bytes+0x21' to check which one is NULL pointer? thanks, liubo > Gustavo > -- > 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
Re: btrfs crash - Null dereference - 3.7.0-rc5-00068-gc5e35d6
Hi Liu,
* Liu Bo [2012-11-19 18:32:23 +0800]:
> On Mon, Nov 19, 2012 at 12:55:40AM -0200, Gustavo Padovan wrote:
> > Hi,
> >
> > my system suddenly crashed and gave me this dump:
> >
> > http://imgur.com/oO6S0
> >
> > I checked and there is not btrfs commit in linus' tree since I compiled this
> > kernel.
> >
>
> Hi Gustavo,
>
> It's weird that NULL pointer oops happens here.
>
> Since you own the kernel, you may also have debuginfo left,
>
> can you please run
> 'gdb fs/btrfs/btrfs.ko' and 'list *block_rsv_release_bytes+0x21' to
> check which one is NULL pointer?
(gdb) list *block_rsv_release_bytes+0x21
0x811a83c1 is in block_rsv_release_bytes
(fs/btrfs/extent-tree.c:4065).
4060
4061static void block_rsv_release_bytes(struct btrfs_fs_info *fs_info,
4062struct btrfs_block_rsv *block_rsv,
4063struct btrfs_block_rsv *dest, u64
num_bytes)
4064{
4065struct btrfs_space_info *space_info = block_rsv->space_info;
4066
4067spin_lock(&block_rsv->lock);
4068if (num_bytes == (u64)-1)
4069num_bytes = block_rsv->size;
(gdb)
Seems block_rsv is NULL here and looking to btrfs_block_rsv_release() it can
only be NULL at this point if global_rsv->full is true otherwise the crash
would happen there. I didn't go any further than this.
Gustavo
--
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
Re: btrfs crash - Null dereference - 3.7.0-rc5-00068-gc5e35d6
On Mon, Nov 19, 2012 at 11:07:52AM -0200, Gustavo Padovan wrote:
> > can you please run
> > 'gdb fs/btrfs/btrfs.ko' and 'list *block_rsv_release_bytes+0x21' to
> > check which one is NULL pointer?
>
>
> (gdb) list *block_rsv_release_bytes+0x21
> 0x811a83c1 is in block_rsv_release_bytes
> (fs/btrfs/extent-tree.c:4065).
> 4060
> 4061 static void block_rsv_release_bytes(struct btrfs_fs_info *fs_info,
> 4062 struct btrfs_block_rsv *block_rsv,
> 4063 struct btrfs_block_rsv *dest, u64
> num_bytes)
> 4064 {
> 4065 struct btrfs_space_info *space_info = block_rsv->space_info;
> 4066
> 4067 spin_lock(&block_rsv->lock);
> 4068 if (num_bytes == (u64)-1)
> 4069 num_bytes = block_rsv->size;
> (gdb)
>
> Seems block_rsv is NULL here and looking to btrfs_block_rsv_release() it can
> only be NULL at this point if global_rsv->full is true otherwise the crash
> would happen there. I didn't go any further than this.
>
> Gustavo
Can you check if the following commit is in your tree?
commit 321f0e70225abc792d74902a2bc4a60164265fd4
Author: Miao Xie
Btrfs: fix wrong orphan count of the fs/file tree
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 878116d..a6824bd 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -2228,7 +2228,7 @@ int btrfs_orphan_add(struct btrfs_trans_handle *trans,
struct inode *inode)
insert = 1;
#endif
insert = 1;
- atomic_dec(&root->orphan_inodes);
+ atomic_inc(&root->orphan_inodes);
}
if (!test_and_set_bit(BTRFS_INODE_ORPHAN_META_RESERVED,
thanks,
liubo
--
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
Re: btrfs crash - Null dereference - 3.7.0-rc5-00068-gc5e35d6
* Liu Bo [2012-11-19 23:27:53 +0800]:
> On Mon, Nov 19, 2012 at 11:07:52AM -0200, Gustavo Padovan wrote:
> > > can you please run
> > > 'gdb fs/btrfs/btrfs.ko' and 'list *block_rsv_release_bytes+0x21' to
> > > check which one is NULL pointer?
> >
> >
> > (gdb) list *block_rsv_release_bytes+0x21
> > 0x811a83c1 is in block_rsv_release_bytes
> > (fs/btrfs/extent-tree.c:4065).
> > 4060
> > 4061static void block_rsv_release_bytes(struct btrfs_fs_info
> > *fs_info,
> > 4062struct btrfs_block_rsv
> > *block_rsv,
> > 4063struct btrfs_block_rsv
> > *dest, u64
> > num_bytes)
> > 4064{
> > 4065struct btrfs_space_info *space_info =
> > block_rsv->space_info;
> > 4066
> > 4067spin_lock(&block_rsv->lock);
> > 4068if (num_bytes == (u64)-1)
> > 4069num_bytes = block_rsv->size;
> > (gdb)
> >
> > Seems block_rsv is NULL here and looking to btrfs_block_rsv_release() it can
> > only be NULL at this point if global_rsv->full is true otherwise the crash
> > would happen there. I didn't go any further than this.
> >
> > Gustavo
>
> Can you check if the following commit is in your tree?
>
> commit 321f0e70225abc792d74902a2bc4a60164265fd4
> Author: Miao Xie
>
> Btrfs: fix wrong orphan count of the fs/file tree
>
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 878116d..a6824bd 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -2228,7 +2228,7 @@ int btrfs_orphan_add(struct btrfs_trans_handle *trans,
> struct inode *inode)
> insert = 1;
> #endif
> insert = 1;
> - atomic_dec(&root->orphan_inodes);
> + atomic_inc(&root->orphan_inodes);
> }
>
> if (!test_and_set_bit(BTRFS_INODE_ORPHAN_META_RESERVED,
Yes, it is. I'm using linus tree from last week, head at c5e35d6.
Gustavo
--
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
[PATCH V2] Btrfs: fix deadlock when the process of delayed refs fails
Delayed ref mutexes are taken inside btrfs_commit_transaction. A later call
fails and jumps to the cleanup_transaction label with these mutexes still held
causing deadlock when they are reacquired.
Fix this problem by unlocking those mutexes at the suitable place.
Reported-by: Daniel J Blueman
Signed-off-by: Miao Xie
---
Changelog from v1 -> v2:
- split the code of the run_one_delayed_ref()'s error path from the normal path
---
fs/btrfs/delayed-ref.c | 8
fs/btrfs/delayed-ref.h | 6 ++
fs/btrfs/extent-tree.c | 36
3 files changed, 38 insertions(+), 12 deletions(-)
diff --git a/fs/btrfs/delayed-ref.c b/fs/btrfs/delayed-ref.c
index ae94117..364cb90 100644
--- a/fs/btrfs/delayed-ref.c
+++ b/fs/btrfs/delayed-ref.c
@@ -422,6 +422,14 @@ again:
return 1;
}
+void btrfs_release_ref_cluster(struct list_head *cluster)
+{
+ struct list_head *pos, *q;
+
+ list_for_each_safe(pos, q, cluster)
+ list_del_init(pos);
+}
+
/*
* helper function to update an extent delayed ref in the
* rbtree. existing and update must both have the same
diff --git a/fs/btrfs/delayed-ref.h b/fs/btrfs/delayed-ref.h
index ab53005..fcc5a29 100644
--- a/fs/btrfs/delayed-ref.h
+++ b/fs/btrfs/delayed-ref.h
@@ -176,8 +176,14 @@ struct btrfs_delayed_ref_head *
btrfs_find_delayed_ref_head(struct btrfs_trans_handle *trans, u64 bytenr);
int btrfs_delayed_ref_lock(struct btrfs_trans_handle *trans,
struct btrfs_delayed_ref_head *head);
+static inline void btrfs_delayed_ref_unlock(struct btrfs_delayed_ref_head
*head)
+{
+ mutex_unlock(&head->mutex);
+}
+
int btrfs_find_ref_cluster(struct btrfs_trans_handle *trans,
struct list_head *cluster, u64 search_start);
+void btrfs_release_ref_cluster(struct list_head *cluster);
int btrfs_check_delayed_seq(struct btrfs_fs_info *fs_info,
struct btrfs_delayed_ref_root *delayed_refs,
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 3d3e2c1..93faf5e 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -2160,7 +2160,6 @@ static int run_one_delayed_ref(struct btrfs_trans_handle
*trans,
node->num_bytes);
}
}
- mutex_unlock(&head->mutex);
return ret;
}
@@ -2275,7 +2274,7 @@ static noinline int run_clustered_refs(struct
btrfs_trans_handle *trans,
* process of being added. Don't run this ref yet.
*/
list_del_init(&locked_ref->cluster);
- mutex_unlock(&locked_ref->mutex);
+ btrfs_delayed_ref_unlock(locked_ref);
locked_ref = NULL;
delayed_refs->num_heads_ready++;
spin_unlock(&delayed_refs->lock);
@@ -2314,16 +2313,17 @@ static noinline int run_clustered_refs(struct
btrfs_trans_handle *trans,
kfree(extent_op);
if (ret) {
- printk(KERN_DEBUG "btrfs:
run_delayed_extent_op returned %d\n", ret);
+ printk(KERN_DEBUG
+ "btrfs: run_delayed_extent_op "
+ "returned %d\n",
+ ret);
spin_lock(&delayed_refs->lock);
+ btrfs_delayed_ref_unlock(locked_ref);
return ret;
}
goto next;
}
-
- list_del_init(&locked_ref->cluster);
- locked_ref = NULL;
}
ref->in_tree = 0;
@@ -2350,17 +2350,28 @@ static noinline int run_clustered_refs(struct
btrfs_trans_handle *trans,
ret = run_one_delayed_ref(trans, root, ref, extent_op,
must_insert_reserved);
-
- btrfs_put_delayed_ref(ref);
kfree(extent_op);
- count++;
-
if (ret) {
- printk(KERN_DEBUG "btrfs: run_one_delayed_ref returned
%d\n", ret);
+ btrfs_delayed_ref_unlock(locked_ref);
+ btrfs_put_delayed_ref(ref);
+ printk(KERN_DEBUG
+ "btrfs: run_one_delayed_ref returned %d\n", ret);
spin_lock(&delayed_refs->lock);
return ret;
}
-
+ /*
+* If this node is a head, that means all the refs in this head
+* have been dealt with, and we will pick the next head to deal
+
Re: [PATCH RESEND] Btrfs: fix deadlock when the process of delayed refs fails
On mon, 19 Nov 2012 18:18:48 +0800, Liu Bo wrote:
>> @@ -2316,14 +2315,12 @@ static noinline int run_clustered_refs(struct
>> btrfs_trans_handle *trans,
>> if (ret) {
>> printk(KERN_DEBUG "btrfs:
>> run_delayed_extent_op returned %d\n", ret);
>> spin_lock(&delayed_refs->lock);
>> +btrfs_delayed_ref_unlock(locked_ref);
>> return ret;
>> }
>>
>> goto next;
>> }
>> -
>> -list_del_init(&locked_ref->cluster);
>> -locked_ref = NULL;
>> }
>>
>> ref->in_tree = 0;
>> @@ -2350,11 +2347,24 @@ static noinline int run_clustered_refs(struct
>> btrfs_trans_handle *trans,
>>
>> ret = run_one_delayed_ref(trans, root, ref, extent_op,
>>must_insert_reserved);
>> -
>> -btrfs_put_delayed_ref(ref);
>> kfree(extent_op);
>> count++;
>>
>> +/*
>> + * If this node is a head, we will pick the next head to deal
>> + * with. If there is something wrong when we process the
>> + * delayed ref, we will end our operation. So in these two
>> + * cases, we have to unlock the head and drop it from the
>> + * cluster list before we release it though the code is ugly.
>> + */
>> +if (btrfs_delayed_ref_is_head(ref) || ret) {
>> +list_del_init(&locked_ref->cluster);
>> +btrfs_delayed_ref_unlock(locked_ref);
>> +locked_ref = NULL;
>> +}
>> +
>
> In case that we don't remove mutex_unlock above,
>
> if ret is non-zero, either
> A)locked_ref is not NULL, or
> B)locked_ref is NULL, and it has done list_del_init above and
> also done mutex_unlock in run_one_delayed_ref().
>
> So in the case A), it is ok to do list_del_init() and mutex_unlock(),
> while in the case B), we need to do nothing.
>
> Then the code can be clean as we wish,
> if (ret) {
> if (locked_ref) {
> list_del_init();
> mutex_unlock();
> }
> ...
> }
I think it is not good style that locking/unlocking a lock in the different
functions, because
it is error prone and the readability of the code is very bad, so I remove
mutex_unlock() in
run_one_delayed_ref().
Maybe I should not mix the code of the error path into the normal one, I will
send out a new patch
to make the code cleaner and more readable.
Thanks
Miao
--
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
Re: [PATCH V2] Btrfs: fix deadlock when the process of delayed refs fails
On Tue, Nov 20, 2012 at 10:52:36AM +0800, Miao Xie wrote:
> Delayed ref mutexes are taken inside btrfs_commit_transaction. A later call
> fails and jumps to the cleanup_transaction label with these mutexes still held
> causing deadlock when they are reacquired.
>
> Fix this problem by unlocking those mutexes at the suitable place.
>
> Reported-by: Daniel J Blueman
> Signed-off-by: Miao Xie
> ---
> Changelog from v1 -> v2:
> - split the code of the run_one_delayed_ref()'s error path from the normal
> path
> ---
> fs/btrfs/delayed-ref.c | 8
> fs/btrfs/delayed-ref.h | 6 ++
> fs/btrfs/extent-tree.c | 36
> 3 files changed, 38 insertions(+), 12 deletions(-)
>
> diff --git a/fs/btrfs/delayed-ref.c b/fs/btrfs/delayed-ref.c
> index ae94117..364cb90 100644
> --- a/fs/btrfs/delayed-ref.c
> +++ b/fs/btrfs/delayed-ref.c
> @@ -422,6 +422,14 @@ again:
> return 1;
> }
>
> +void btrfs_release_ref_cluster(struct list_head *cluster)
> +{
> + struct list_head *pos, *q;
> +
> + list_for_each_safe(pos, q, cluster)
> + list_del_init(pos);
> +}
> +
> /*
> * helper function to update an extent delayed ref in the
> * rbtree. existing and update must both have the same
> diff --git a/fs/btrfs/delayed-ref.h b/fs/btrfs/delayed-ref.h
> index ab53005..fcc5a29 100644
> --- a/fs/btrfs/delayed-ref.h
> +++ b/fs/btrfs/delayed-ref.h
> @@ -176,8 +176,14 @@ struct btrfs_delayed_ref_head *
> btrfs_find_delayed_ref_head(struct btrfs_trans_handle *trans, u64 bytenr);
> int btrfs_delayed_ref_lock(struct btrfs_trans_handle *trans,
> struct btrfs_delayed_ref_head *head);
> +static inline void btrfs_delayed_ref_unlock(struct btrfs_delayed_ref_head
> *head)
> +{
> + mutex_unlock(&head->mutex);
> +}
> +
> int btrfs_find_ref_cluster(struct btrfs_trans_handle *trans,
> struct list_head *cluster, u64 search_start);
> +void btrfs_release_ref_cluster(struct list_head *cluster);
>
> int btrfs_check_delayed_seq(struct btrfs_fs_info *fs_info,
> struct btrfs_delayed_ref_root *delayed_refs,
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 3d3e2c1..93faf5e 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -2160,7 +2160,6 @@ static int run_one_delayed_ref(struct
> btrfs_trans_handle *trans,
> node->num_bytes);
> }
> }
> - mutex_unlock(&head->mutex);
> return ret;
> }
>
> @@ -2275,7 +2274,7 @@ static noinline int run_clustered_refs(struct
> btrfs_trans_handle *trans,
>* process of being added. Don't run this ref yet.
>*/
> list_del_init(&locked_ref->cluster);
> - mutex_unlock(&locked_ref->mutex);
> + btrfs_delayed_ref_unlock(locked_ref);
> locked_ref = NULL;
> delayed_refs->num_heads_ready++;
> spin_unlock(&delayed_refs->lock);
> @@ -2314,16 +2313,17 @@ static noinline int run_clustered_refs(struct
> btrfs_trans_handle *trans,
> kfree(extent_op);
>
> if (ret) {
> - printk(KERN_DEBUG "btrfs:
> run_delayed_extent_op returned %d\n", ret);
> + printk(KERN_DEBUG
> +"btrfs: run_delayed_extent_op "
> +"returned %d\n",
> +ret);
> spin_lock(&delayed_refs->lock);
> + btrfs_delayed_ref_unlock(locked_ref);
> return ret;
> }
>
> goto next;
> }
> -
> - list_del_init(&locked_ref->cluster);
> - locked_ref = NULL;
> }
>
> ref->in_tree = 0;
> @@ -2350,17 +2350,28 @@ static noinline int run_clustered_refs(struct
> btrfs_trans_handle *trans,
>
> ret = run_one_delayed_ref(trans, root, ref, extent_op,
> must_insert_reserved);
> -
> - btrfs_put_delayed_ref(ref);
> kfree(extent_op);
> - count++;
> -
> if (ret) {
> - printk(KERN_DEBUG "btrfs: run_one_delayed_ref returned
> %d\n", ret);
> + btrfs_delayed_ref_unlock(locked_ref);
> + btrfs_put_delayed_ref(ref);
> + printk(KERN_DEBUG
> +"btrfs: run_one_delayed_ref returned %d\n", ret);
> spin_lock(&delayed_refs->lock);
> return ret;
>
[PATCH] btrfs-progs: print the right mount checking info in restore.c
The mount status is checked against the arg (argv[optind]),
not the . Correct the info printed on mounted.
Signed-off-by: Wang Sheng-Hui
---
restore.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/restore.c b/restore.c
index b9e5381..a7580f8 100644
--- a/restore.c
+++ b/restore.c
@@ -827,7 +827,7 @@ int main(int argc, char **argv)
strerror(ret));
return ret;
} else if (ret) {
- fprintf(stderr, "%s is currently mounted. Aborting.\n",
argv[optind + 1]);
+ fprintf(stderr, "%s is currently mounted. Aborting.\n",
argv[optind]);
return -EBUSY;
}
--
1.7.5.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
