Skip to contentSkip to conversation

Need some help with a backref problem

DISCUSSION Josef Bacik 2 participants 5 messages Oct 16, 2008 – Oct 17, 2008 Btrfs file system

lore
Share & tools
lore
#0 lore raw

Hello,

Its the end of the day here and I haven't figured this out, so hopefully Yan you can figure this out and I can come in tomorrow and keep working on taking alloc_mutex out :). What is happening is I'm getting -ENOENT from lookup_extent_backref in finish_current_insert() when extent_op->type == PENDING_BACKREF_UPDATE. The way I have locking is that the only way this can happen is if we delete the extent backref completely, and then do btrfs_update_ref. I put a lookup_extent_backref in __btrfs_update_extent_ref and did a BUG_ON(ret), and it gave me this backtrace

[<ffffffffa035ecac>] ? btrfs_update_ref+0x2ce/0x322 [btrfs] [<ffffffffa034f859>] ? insert_ptr+0x176/0x184 [btrfs] [<ffffffffa0354615>] ? split_node+0x54a/0x5b3 [btrfs] [<ffffffffa03555af>] ? btrfs_search_slot+0x4ef/0x7aa [btrfs] [<ffffffff8109a1ad>] ? check_bytes_and_report+0x37/0xc9 [<ffffffff8109a1ad>] ? check_bytes_and_report+0x37/0xc9 [<ffffffffa0355da7>] ? btrfs_insert_empty_items+0x7d/0x43b [btrfs] [<ffffffffa03561b4>] ? btrfs_insert_item+0x4f/0xa4 [btrfs] [<ffffffffa0358ef6>] ? finish_current_insert+0xfc/0x2b5 [btrfs] [<ffffffff8109a832>] ? init_object+0x27/0x6e [<ffffffffa035a77b>] ? __btrfs_alloc_reserved_extent+0x37d/0x3dc [btrfs] [<ffffffffa035aa26>] ? btrfs_alloc_reserved_extent+0x2b/0x5b [btrfs] [<ffffffffa036accf>] ? btrfs_finish_ordered_io+0x21b/0x344 [btrfs] [<ffffffffa037a782>] ? end_bio_extent_writepage+0x9b/0x172 [btrfs] [<ffffffffa037fe51>] ? worker_loop+0x42/0x125 [btrfs] [<ffffffffa037fe0f>] ? worker_loop+0x0/0x125 [btrfs] [<ffffffff81046721>] ? kthread+0x47/0x76 [<ffffffff8100cd59>] ? child_rip+0xa/0x11 [<ffffffff810466da>] ? kthread+0x0/0x76
[<ffffffff8100cd4f>] ? child_rip+0x0/0x11

And I also put in some printk's to figure out when exactly this was happening, and it happens in split_node() when c == root->node, so we do an insert_new_root. My first reaction was to put a c = path->nodes[level] after the insert_new_root, but looking at it thats just going to give me the same thing back. I can't figure out if I'm doing something wrong or if there is something wonky with the backref stuff, and what is even more worriesome is that I can't figure out why having alloc_mutex in there kept this problem from happenign before, since the way this happens doesn't have anything to do with alloc_mutex. All help is appreciated, even random thinking outloud, hopefully we can figure out what is going on and I can finish ripping alloc_mutex out. Thanks,

Josef

#1 lore raw
Quoted 32 lines · Josef Bacik

Hello,

Its the end of the day here and I haven't figured this out, so hopefully Yan you can figure this out and I can come in tomorrow and keep working on taking alloc_mutex out :). What is happening is I'm getting -ENOENT from lookup_extent_backref in finish_current_insert() when extent_op->type == PENDING_BACKREF_UPDATE. The way I have locking is that the only way this can happen is if we delete the extent backref completely, and then do btrfs_update_ref. I put a lookup_extent_backref in __btrfs_update_extent_ref and did a BUG_ON(ret), and it gave me this backtrace

[<ffffffffa035ecac>] ? btrfs_update_ref+0x2ce/0x322 [btrfs] [<ffffffffa034f859>] ? insert_ptr+0x176/0x184 [btrfs] [<ffffffffa0354615>] ? split_node+0x54a/0x5b3 [btrfs] [<ffffffffa03555af>] ? btrfs_search_slot+0x4ef/0x7aa [btrfs] [<ffffffff8109a1ad>] ? check_bytes_and_report+0x37/0xc9 [<ffffffff8109a1ad>] ? check_bytes_and_report+0x37/0xc9 [<ffffffffa0355da7>] ? btrfs_insert_empty_items+0x7d/0x43b [btrfs] [<ffffffffa03561b4>] ? btrfs_insert_item+0x4f/0xa4 [btrfs] [<ffffffffa0358ef6>] ? finish_current_insert+0xfc/0x2b5 [btrfs] [<ffffffff8109a832>] ? init_object+0x27/0x6e [<ffffffffa035a77b>] ? __btrfs_alloc_reserved_extent+0x37d/0x3dc [btrfs] [<ffffffffa035aa26>] ? btrfs_alloc_reserved_extent+0x2b/0x5b [btrfs] [<ffffffffa036accf>] ? btrfs_finish_ordered_io+0x21b/0x344 [btrfs] [<ffffffffa037a782>] ? end_bio_extent_writepage+0x9b/0x172 [btrfs] [<ffffffffa037fe51>] ? worker_loop+0x42/0x125 [btrfs] [<ffffffffa037fe0f>] ? worker_loop+0x0/0x125 [btrfs] [<ffffffff81046721>] ? kthread+0x47/0x76 [<ffffffff8100cd59>] ? child_rip+0xa/0x11 [<ffffffff810466da>] ? kthread+0x0/0x76
[<ffffffff8100cd4f>] ? child_rip+0x0/0x11

Quoted 12 lines

And I also put in some printk's to figure out when exactly this was happening, and it happens in split_node() when c == root->node, so we do an insert_new_root. My first reaction was to put a c = path->nodes[level] after the insert_new_root, but looking at it thats just going to give me the same thing back. I can't figure out if I'm doing something wrong or if there is something wonky with the backref stuff, and what is even more worriesome is that I can't figure out why having alloc_mutex in there kept this problem from happenign before, since the way this happens doesn't have anything to do with alloc_mutex. All help is appreciated, even random thinking outloud, hopefully we can figure out what is going on and I can finish ripping alloc_mutex out. Thanks,

Ok I think I figured it out, we need to have a

c = root->node;

after the insert_new_root, since we will have free'd the old extent buffer and replaced it with a new one. Does that sound right? Thanks,

Josef

#2 lore raw

2008/10/17 Josef Bacik <[email protected]>:

Quoted 12 lines · Josef Bacik
Quoted 11 lines

Hello,

Its the end of the day here and I haven't figured this out, so hopefully Yan you can figure this out and I can come in tomorrow and keep working on taking alloc_mutex out :). What is happening is I'm getting -ENOENT from lookup_extent_backref in finish_current_insert() when extent_op->type == PENDING_BACKREF_UPDATE. The way I have locking is that the only way this can happen is if we delete the extent backref completely, and then do btrfs_update_ref. I put a lookup_extent_backref in __btrfs_update_extent_ref and did a BUG_ON(ret), and it gave me this backtrace

I guess there are two or more threads running finish_current_insert at the same time. (find_first_extent_bit vs clear_extent_bits race)

Quoted 34 lines
Quoted 21 lines

[<ffffffffa035ecac>] ? btrfs_update_ref+0x2ce/0x322 [btrfs] [<ffffffffa034f859>] ? insert_ptr+0x176/0x184 [btrfs] [<ffffffffa0354615>] ? split_node+0x54a/0x5b3 [btrfs] [<ffffffffa03555af>] ? btrfs_search_slot+0x4ef/0x7aa [btrfs] [<ffffffff8109a1ad>] ? check_bytes_and_report+0x37/0xc9 [<ffffffff8109a1ad>] ? check_bytes_and_report+0x37/0xc9 [<ffffffffa0355da7>] ? btrfs_insert_empty_items+0x7d/0x43b [btrfs] [<ffffffffa03561b4>] ? btrfs_insert_item+0x4f/0xa4 [btrfs] [<ffffffffa0358ef6>] ? finish_current_insert+0xfc/0x2b5 [btrfs] [<ffffffff8109a832>] ? init_object+0x27/0x6e [<ffffffffa035a77b>] ? __btrfs_alloc_reserved_extent+0x37d/0x3dc [btrfs] [<ffffffffa035aa26>] ? btrfs_alloc_reserved_extent+0x2b/0x5b [btrfs] [<ffffffffa036accf>] ? btrfs_finish_ordered_io+0x21b/0x344 [btrfs] [<ffffffffa037a782>] ? end_bio_extent_writepage+0x9b/0x172 [btrfs] [<ffffffffa037fe51>] ? worker_loop+0x42/0x125 [btrfs] [<ffffffffa037fe0f>] ? worker_loop+0x0/0x125 [btrfs] [<ffffffff81046721>] ? kthread+0x47/0x76 [<ffffffff8100cd59>] ? child_rip+0xa/0x11 [<ffffffff810466da>] ? kthread+0x0/0x76
[<ffffffff8100cd4f>] ? child_rip+0x0/0x11

Quoted 12 lines

And I also put in some printk's to figure out when exactly this was happening, and it happens in split_node() when c == root->node, so we do an insert_new_root. My first reaction was to put a c = path->nodes[level] after the insert_new_root, but looking at it thats just going to give me the same thing back. I can't figure out if I'm doing something wrong or if there is something wonky with the backref stuff, and what is even more worriesome is that I can't figure out why having alloc_mutex in there kept this problem from happenign before, since the way this happens doesn't have anything to do with alloc_mutex. All help is appreciated, even random thinking outloud, hopefully we can figure out what is going on and I can finish ripping alloc_mutex out. Thanks,

Quoted 7 lines

Ok I think I figured it out, we need to have a

c = root->node;

after the insert_new_root, since we will have free'd the old extent buffer and replaced it with a new one. Does that sound right? Thanks,

I don't think so. If we do this, we will end up spliting the new root.

Regards
Yan Zheng

#3 lore raw
Quoted 14 lines · Yan Zheng

2008/10/17 Josef Bacik <[email protected]>:

Quoted 12 lines · Josef Bacik
Quoted 11 lines

Hello,

Its the end of the day here and I haven't figured this out, so hopefully Yan you can figure this out and I can come in tomorrow and keep working on taking alloc_mutex out :). What is happening is I'm getting -ENOENT from lookup_extent_backref in finish_current_insert() when extent_op->type == PENDING_BACKREF_UPDATE. The way I have locking is that the only way this can happen is if we delete the extent backref completely, and then do btrfs_update_ref. I put a lookup_extent_backref in __btrfs_update_extent_ref and did a BUG_ON(ret), and it gave me this backtrace

Quoted 3 lines

I guess there are two or more threads running finish_current_insert at the same time. (find_first_extent_bit vs clear_extent_bits race)

This can't happen, its gaurded by a new mutex that is responsible for the extent_ins/del_pending/pinned_extents extent io trees.

Quoted 36 lines
Quoted 36 lines
Quoted 21 lines

[<ffffffffa035ecac>] ? btrfs_update_ref+0x2ce/0x322 [btrfs] [<ffffffffa034f859>] ? insert_ptr+0x176/0x184 [btrfs] [<ffffffffa0354615>] ? split_node+0x54a/0x5b3 [btrfs] [<ffffffffa03555af>] ? btrfs_search_slot+0x4ef/0x7aa [btrfs] [<ffffffff8109a1ad>] ? check_bytes_and_report+0x37/0xc9 [<ffffffff8109a1ad>] ? check_bytes_and_report+0x37/0xc9 [<ffffffffa0355da7>] ? btrfs_insert_empty_items+0x7d/0x43b [btrfs] [<ffffffffa03561b4>] ? btrfs_insert_item+0x4f/0xa4 [btrfs] [<ffffffffa0358ef6>] ? finish_current_insert+0xfc/0x2b5 [btrfs] [<ffffffff8109a832>] ? init_object+0x27/0x6e [<ffffffffa035a77b>] ? __btrfs_alloc_reserved_extent+0x37d/0x3dc [btrfs] [<ffffffffa035aa26>] ? btrfs_alloc_reserved_extent+0x2b/0x5b [btrfs] [<ffffffffa036accf>] ? btrfs_finish_ordered_io+0x21b/0x344 [btrfs] [<ffffffffa037a782>] ? end_bio_extent_writepage+0x9b/0x172 [btrfs] [<ffffffffa037fe51>] ? worker_loop+0x42/0x125 [btrfs] [<ffffffffa037fe0f>] ? worker_loop+0x0/0x125 [btrfs] [<ffffffff81046721>] ? kthread+0x47/0x76 [<ffffffff8100cd59>] ? child_rip+0xa/0x11 [<ffffffff810466da>] ? kthread+0x0/0x76
[<ffffffff8100cd4f>] ? child_rip+0x0/0x11

Quoted 12 lines

And I also put in some printk's to figure out when exactly this was happening, and it happens in split_node() when c == root->node, so we do an insert_new_root. My first reaction was to put a c = path->nodes[level] after the insert_new_root, but looking at it thats just going to give me the same thing back. I can't figure out if I'm doing something wrong or if there is something wonky with the backref stuff, and what is even more worriesome is that I can't figure out why having alloc_mutex in there kept this problem from happenign before, since the way this happens doesn't have anything to do with alloc_mutex. All help is appreciated, even random thinking outloud, hopefully we can figure out what is going on and I can finish ripping alloc_mutex out. Thanks,

Ok I think I figured it out, we need to have a

Quoted 8 lines
Quoted 5 lines

c = root->node;

after the insert_new_root, since we will have free'd the old extent buffer and replaced it with a new one. Does that sound right? Thanks,

I don't think so. If we do this, we will end up spliting the new root.

Ok, I think I understand this now, thanks,

Josef

#4 lore raw
In reply to Yan Zheng
Quoted 53 lines · Yan Zheng

2008/10/17 Josef Bacik <[email protected]>:

Quoted 12 lines · Josef Bacik
Quoted 11 lines

Hello,

Its the end of the day here and I haven't figured this out, so hopefully Yan you can figure this out and I can come in tomorrow and keep working on taking alloc_mutex out :). What is happening is I'm getting -ENOENT from lookup_extent_backref in finish_current_insert() when extent_op->type == PENDING_BACKREF_UPDATE. The way I have locking is that the only way this can happen is if we delete the extent backref completely, and then do btrfs_update_ref. I put a lookup_extent_backref in __btrfs_update_extent_ref and did a BUG_ON(ret), and it gave me this backtrace

I guess there are two or more threads running finish_current_insert at the same time. (find_first_extent_bit vs clear_extent_bits race)

Quoted 36 lines
Quoted 21 lines

[<ffffffffa035ecac>] ? btrfs_update_ref+0x2ce/0x322 [btrfs] [<ffffffffa034f859>] ? insert_ptr+0x176/0x184 [btrfs] [<ffffffffa0354615>] ? split_node+0x54a/0x5b3 [btrfs] [<ffffffffa03555af>] ? btrfs_search_slot+0x4ef/0x7aa [btrfs] [<ffffffff8109a1ad>] ? check_bytes_and_report+0x37/0xc9 [<ffffffff8109a1ad>] ? check_bytes_and_report+0x37/0xc9 [<ffffffffa0355da7>] ? btrfs_insert_empty_items+0x7d/0x43b [btrfs] [<ffffffffa03561b4>] ? btrfs_insert_item+0x4f/0xa4 [btrfs] [<ffffffffa0358ef6>] ? finish_current_insert+0xfc/0x2b5 [btrfs] [<ffffffff8109a832>] ? init_object+0x27/0x6e [<ffffffffa035a77b>] ? __btrfs_alloc_reserved_extent+0x37d/0x3dc [btrfs] [<ffffffffa035aa26>] ? btrfs_alloc_reserved_extent+0x2b/0x5b [btrfs] [<ffffffffa036accf>] ? btrfs_finish_ordered_io+0x21b/0x344 [btrfs] [<ffffffffa037a782>] ? end_bio_extent_writepage+0x9b/0x172 [btrfs] [<ffffffffa037fe51>] ? worker_loop+0x42/0x125 [btrfs] [<ffffffffa037fe0f>] ? worker_loop+0x0/0x125 [btrfs] [<ffffffff81046721>] ? kthread+0x47/0x76 [<ffffffff8100cd59>] ? child_rip+0xa/0x11 [<ffffffff810466da>] ? kthread+0x0/0x76
[<ffffffff8100cd4f>] ? child_rip+0x0/0x11

Quoted 12 lines

And I also put in some printk's to figure out when exactly this was happening, and it happens in split_node() when c == root->node, so we do an insert_new_root. My first reaction was to put a c = path->nodes[level] after the insert_new_root, but looking at it thats just going to give me the same thing back. I can't figure out if I'm doing something wrong or if there is something wonky with the backref stuff, and what is even more worriesome is that I can't figure out why having alloc_mutex in there kept this problem from happenign before, since the way this happens doesn't have anything to do with alloc_mutex. All help is appreciated, even random thinking outloud, hopefully we can figure out what is going on and I can finish ripping alloc_mutex out. Thanks,

Ok I think I figured it out, we need to have a

Quoted 8 lines
Quoted 5 lines

c = root->node;

after the insert_new_root, since we will have free'd the old extent buffer and replaced it with a new one. Does that sound right? Thanks,

I don't think so. If we do this, we will end up spliting the new root.

here is my patch, its a bit of a mess right now, thanks,

Josef

fs/btrfs/ctree.c +2 -2
index 9caeb37..4f55552 100644
@@ -1390,8 +1390,7 @@ int btrfs_search_slot(struct btrfs_trans_handle *trans, struct btrfs_root
 	lowest_level = p->lowest_level;
 	WARN_ON(lowest_level && ins_len > 0);
 	WARN_ON(p->nodes[0] != NULL);
-	WARN_ON(cow && root == root->fs_info->extent_root &&
-		!mutex_is_locked(&root->fs_info->alloc_mutex));
+
 	if (ins_len < 0)
 		lowest_unlock = 2;
 
@@ -2051,6 +2050,7 @@ static noinline int split_node(struct btrfs_trans_handle *trans,
 	if (c == root->node) {
 		/* trying to split the root, lets make a new one */
 		ret = insert_new_root(trans, root, path, level + 1);
+		printk(KERN_ERR "splitting the root, %llu\n", c->start);
 		if (ret)
 			return ret;
 	} else {
fs/btrfs/ctree.h +7 -0
index fad58b9..d1e304f 100644
@@ -516,12 +516,14 @@ struct btrfs_free_space {
 	struct rb_node offset_index;
 	u64 offset;
 	u64 bytes;
+	unsigned long ip;
 };
 
 struct btrfs_block_group_cache {
 	struct btrfs_key key;
 	struct btrfs_block_group_item item;
 	spinlock_t lock;
+	struct mutex alloc_mutex;
 	u64 pinned;
 	u64 reserved;
 	u64 flags;
@@ -600,6 +602,7 @@ struct btrfs_fs_info {
 	struct mutex transaction_kthread_mutex;
 	struct mutex cleaner_mutex;
 	struct mutex alloc_mutex;
+	struct mutex extent_io_mutex;
 	struct mutex chunk_mutex;
 	struct mutex drop_mutex;
 	struct mutex volume_mutex;
@@ -1879,8 +1882,12 @@ int btrfs_acl_chmod(struct inode *inode);
 /* free-space-cache.c */
 int btrfs_add_free_space(struct btrfs_block_group_cache *block_group,
 			 u64 bytenr, u64 size);
+int btrfs_add_free_space_lock(struct btrfs_block_group_cache *block_group,
+			      u64 offset, u64 bytes);
 int btrfs_remove_free_space(struct btrfs_block_group_cache *block_group,
 			    u64 bytenr, u64 size);
+int btrfs_remove_free_space_lock(struct btrfs_block_group_cache *block_group,
+				 u64 offset, u64 bytes);
 void btrfs_remove_free_space_cache(struct btrfs_block_group_cache
 				   *block_group);
 struct btrfs_free_space *btrfs_find_free_space(struct btrfs_block_group_cache
fs/btrfs/disk-io.c +1 -0
index 0be044b..6da2345 100644
@@ -1458,6 +1458,7 @@ struct btrfs_root *open_ctree(struct super_block *sb,
 	mutex_init(&fs_info->tree_log_mutex);
 	mutex_init(&fs_info->drop_mutex);
 	mutex_init(&fs_info->alloc_mutex);
+	mutex_init(&fs_info->extent_io_mutex);
 	mutex_init(&fs_info->chunk_mutex);
 	mutex_init(&fs_info->transaction_kthread_mutex);
 	mutex_init(&fs_info->cleaner_mutex);
fs/btrfs/extent-tree.c +178 -74

714 lines not on this page · raw message

fs/btrfs/free-space-cache.c +68 -26

175 lines not on this page · raw message

fs/btrfs/tree-log.c +2 -0
index cf618cc..fc3a8d8 100644
@@ -272,8 +272,10 @@ static int process_one_buffer(struct btrfs_root *log,
 {
 	if (wc->pin) {
 		mutex_lock(&log->fs_info->alloc_mutex);
+		mutex_lock(&log->fs_info->extent_io_mutex);
 		btrfs_update_pinned_extents(log->fs_info->extent_root,
 					    eb->start, eb->len, 1);
+		mutex_unlock(&log->fs_info->extent_io_mutex);
 		mutex_unlock(&log->fs_info->alloc_mutex);
 	}