diff mbox series

[v5,05/13] btrfs: delete stripe extent on extent deletion

Message ID 28ff4b0398d52b27c93fac93297be8f0e2a18fab.1675853489.git.johannes.thumshirn@wdc.com (mailing list archive)
State New, archived
Headers show
Series btrfs: introduce RAID stripe tree | expand

Commit Message

Johannes Thumshirn Feb. 8, 2023, 10:57 a.m. UTC 
As each stripe extent is tied to an extent item, delete the stripe extent
once the corresponding extent item is deleted.

Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 fs/btrfs/extent-tree.c      |  8 ++++++++
 fs/btrfs/raid-stripe-tree.c | 31 +++++++++++++++++++++++++++++++
 fs/btrfs/raid-stripe-tree.h |  2 ++
 3 files changed, 41 insertions(+)

Comments

Josef Bacik Feb. 8, 2023, 8 p.m. UTC | #1 
On Wed, Feb 08, 2023 at 02:57:42AM -0800, Johannes Thumshirn wrote:
> As each stripe extent is tied to an extent item, delete the stripe extent
> once the corresponding extent item is deleted.
> 
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> ---
>  fs/btrfs/extent-tree.c      |  8 ++++++++
>  fs/btrfs/raid-stripe-tree.c | 31 +++++++++++++++++++++++++++++++
>  fs/btrfs/raid-stripe-tree.h |  2 ++
>  3 files changed, 41 insertions(+)
> 
> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
> index 50b3a2c3c0dd..f08ee7d9211c 100644
> --- a/fs/btrfs/extent-tree.c
> +++ b/fs/btrfs/extent-tree.c
> @@ -3238,6 +3238,14 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans,
>  			}
>  		}
>  
> +		if (is_data) {
> +			ret = btrfs_delete_raid_extent(trans, bytenr, num_bytes);
> +			if (ret) {
> +				btrfs_abort_transaction(trans, ret);
> +				return ret;
> +			}
> +		}
> +

We're still holding the path open, so now we have a lockdep thing of extent root
-> RST, which will for sure bite us in the ass in the future.  Push this part
just under this part of the code, after the btrfs_release_path().

Also since we have a path here, add it to the arguments for
btrfs_delete_raid_extent() so we don't have to allocate a new path.  Thanks,

Josef
Johannes Thumshirn Feb. 13, 2023, 3:21 p.m. UTC | #2 
On 08.02.23 21:00, Josef Bacik wrote:
>> diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
>> index 50b3a2c3c0dd..f08ee7d9211c 100644
>> --- a/fs/btrfs/extent-tree.c
>> +++ b/fs/btrfs/extent-tree.c
>> @@ -3238,6 +3238,14 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans,
>>  			}
>>  		}
>>  
>> +		if (is_data) {
>> +			ret = btrfs_delete_raid_extent(trans, bytenr, num_bytes);
>> +			if (ret) {
>> +				btrfs_abort_transaction(trans, ret);
>> +				return ret;
>> +			}
>> +		}
>> +
> We're still holding the path open, so now we have a lockdep thing of extent root
> -> RST, which will for sure bite us in the ass in the future.  Push this part
> just under this part of the code, after the btrfs_release_path().
> 
> Also since we have a path here, add it to the arguments for
> btrfs_delete_raid_extent() so we don't have to allocate a new path. 

Not sure what you mean here.

With below change, I'm getting this splat:

bash-5.1# rm /mnt/test/test                                                                                                                                                           
[   46.519048] ------------[ cut here ]------------                                                                                                                                   
[   46.520012] WARNING: CPU: 0 PID: 80 at fs/btrfs/ctree.c:2050 btrfs_search_slot+0x99a/0xd50                                                                                         
[   46.521652] CPU: 0 PID: 80 Comm: kworker/u8:8 Not tainted 6.2.0-rc7+ #657                                                                                                          
[   46.522883] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.16.1-2.fc37 04/01/2014
[   46.524575] Workqueue: events_unbound btrfs_preempt_reclaim_metadata_space        
[   46.526016] RIP: 0010:btrfs_search_slot+0x99a/0xd50
[   46.526911] Code: 00 e9 63 fb ff ff c7 44 24 44 08 00 00 00 89 c8 c7 44 24 28 08 00 00 00 e9 60 f7 ff ff 0f 0b 49 83 3c 24 00 0f 84 e9 f6 ff ff <0f> 0b e9 e2 f6 ff ff 0f 0b e8 38 2c db ff 85 c0 0f 85 5f f7 ff ff
[   46.530527] RSP: 0018:ffffc90000b5faf8 EFLAGS: 00010286
[   46.531479] RAX: 0000000000000000 RBX: 00000000ffffffff RCX: ffff88810235b700
[   46.532817] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffffff81ec6182
[   46.534250] RBP: ffff8881056b8000 R08: 00000000ffffffff R09: 0000000000000001
[   46.535524] R10: 0000000000001000 R11: 00000000000000d0 R12: ffff88810a75e230
[   46.536968] R13: ffff88810524e000 R14: ffff88810a75e230 R15: 000000000000002d
[   46.538273] FS:  0000000000000000(0000) GS:ffff88842fc00000(0000) knlGS:0000000000000000
[   46.539963] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   46.541085] CR2: 000055aea08c9410 CR3: 00000001122fa000 CR4: 0000000000350eb0
[   46.542395] Call Trace:
[   46.542868]  <TASK>
[   46.543448]  ? set_extent_buffer_dirty+0x17/0x170
[   46.544409]  ? btrfs_mark_buffer_dirty+0xd7/0x100
[   46.545270]  ? btrfs_del_items+0x394/0x4c0
[   46.546031]  btrfs_delete_raid_extent+0x4a/0x80
[   46.547015]  __btrfs_free_extent+0x446/0x7e0
[   46.547812]  __btrfs_run_delayed_refs+0x331/0x1210
[   46.548794]  btrfs_run_delayed_refs+0x89/0x1b0
[   46.549661]  flush_space+0x392/0x5e0
[   46.550418]  ? _raw_spin_unlock+0x12/0x30
[   46.551156]  ? btrfs_get_alloc_profile+0xbd/0x1a0
[   46.552095]  btrfs_preempt_reclaim_metadata_space+0x96/0x1e0
[   46.553191]  process_one_work+0x1d9/0x3e0
[   46.554038]  worker_thread+0x4a/0x3c0
[   46.554726]  ? __pfx_worker_thread+0x10/0x10
[   46.555514]  kthread+0xf5/0x120
[   46.556173]  ? __pfx_kthread+0x10/0x10
[   46.557020]  ret_from_fork+0x2c/0x50
[   46.557695]  </TASK>
[   46.558107] ---[ end trace 0000000000000000 ]---




diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 10827b7db13c..c040c1c70075 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -3238,20 +3238,23 @@ static int __btrfs_free_extent(struct btrfs_trans_handle *trans,
 			}
 		}
 
-		if (is_data) {
-			ret = btrfs_delete_raid_extent(trans, bytenr, num_bytes);
-			if (ret) {
-				btrfs_abort_transaction(trans, ret);
-				return ret;
-			}
-		}
-
 		ret = btrfs_del_items(trans, extent_root, path, path->slots[0],
 				      num_to_del);
 		if (ret) {
 			btrfs_abort_transaction(trans, ret);
 			goto out;
 		}
+
+		if (is_data) {
+			ret = btrfs_delete_raid_extent(trans, bytenr, num_bytes,
+						path);
+			if (ret) {
+				btrfs_abort_transaction(trans, ret);
+				goto out;
+			}
+		}
+
+
 		btrfs_release_path(path);
 
 		ret = do_free_extent_accounting(trans, bytenr, num_bytes, is_data);
diff --git a/fs/btrfs/raid-stripe-tree.c b/fs/btrfs/raid-stripe-tree.c
index 8799a7abaf38..a60cdfaa9359 100644
--- a/fs/btrfs/raid-stripe-tree.c
+++ b/fs/btrfs/raid-stripe-tree.c
@@ -157,11 +157,11 @@ void btrfs_put_ordered_stripe(struct btrfs_fs_info *fs_info,
 }
 
 int btrfs_delete_raid_extent(struct btrfs_trans_handle *trans, u64 start,
-			     u64 length)
+			u64 length, struct btrfs_path *path)
 {
 	struct btrfs_fs_info *fs_info = trans->fs_info;
 	struct btrfs_root *stripe_root = btrfs_stripe_tree_root(fs_info);
-	struct btrfs_path *path;
+	//	struct btrfs_path *path;
 	struct btrfs_key stripe_key;
 	int ret;
 
@@ -172,9 +172,9 @@ int btrfs_delete_raid_extent(struct btrfs_trans_handle *trans, u64 start,
 	stripe_key.type = BTRFS_RAID_STRIPE_KEY;
 	stripe_key.offset = length;
 
-	path = btrfs_alloc_path();
-	if (!path)
-		return -ENOMEM;
+	/* path = btrfs_alloc_path(); */
+	/* if (!path) */
+	/* 	return -ENOMEM; */
 
 	ret = btrfs_search_slot(trans, stripe_root, &stripe_key, path, -1, 1);
 	if (ret < 0)
@@ -182,7 +182,7 @@ int btrfs_delete_raid_extent(struct btrfs_trans_handle *trans, u64 start,
 
 	ret = btrfs_del_item(trans, stripe_root, path);
 out:
-	btrfs_free_path(path);
+	/* btrfs_free_path(path); */
 	return ret;
 
 }
diff --git a/fs/btrfs/raid-stripe-tree.h b/fs/btrfs/raid-stripe-tree.h
index 371409351d60..fc415ec77c36 100644
--- a/fs/btrfs/raid-stripe-tree.h
+++ b/fs/btrfs/raid-stripe-tree.h
@@ -27,7 +27,7 @@ int btrfs_get_raid_extent_offset(struct btrfs_fs_info *fs_info,
 				 u32 stripe_index,
 				 struct btrfs_io_stripe *stripe);
 int btrfs_delete_raid_extent(struct btrfs_trans_handle *trans, u64 start,
-			     u64 length);
+			u64 length, struct btrfs_path *path);
 int btrfs_insert_raid_extent(struct btrfs_trans_handle *trans,
 			     struct btrfs_ordered_stripe *stripe);
 int btrfs_insert_preallocated_raid_stripe(struct btrfs_fs_info *fs_info,
diff mbox series

Patch

diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 50b3a2c3c0dd..f08ee7d9211c 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -3238,6 +3238,14 @@  static int __btrfs_free_extent(struct btrfs_trans_handle *trans,
 			}
 		}
 
+		if (is_data) {
+			ret = btrfs_delete_raid_extent(trans, bytenr, num_bytes);
+			if (ret) {
+				btrfs_abort_transaction(trans, ret);
+				return ret;
+			}
+		}
+
 		ret = btrfs_del_items(trans, extent_root, path, path->slots[0],
 				      num_to_del);
 		if (ret) {
diff --git a/fs/btrfs/raid-stripe-tree.c b/fs/btrfs/raid-stripe-tree.c
index d184cd9dc96e..ff5787a19454 100644
--- a/fs/btrfs/raid-stripe-tree.c
+++ b/fs/btrfs/raid-stripe-tree.c
@@ -122,6 +122,37 @@  void btrfs_put_ordered_stripe(struct btrfs_fs_info *fs_info,
 	write_unlock(&fs_info->stripe_update_lock);
 }
 
+int btrfs_delete_raid_extent(struct btrfs_trans_handle *trans, u64 start,
+			     u64 length)
+{
+	struct btrfs_fs_info *fs_info = trans->fs_info;
+	struct btrfs_root *stripe_root = btrfs_stripe_tree_root(fs_info);
+	struct btrfs_path *path;
+	struct btrfs_key stripe_key;
+	int ret;
+
+	if (!stripe_root)
+		return 0;
+
+	stripe_key.objectid = start;
+	stripe_key.type = BTRFS_RAID_STRIPE_KEY;
+	stripe_key.offset = length;
+
+	path = btrfs_alloc_path();
+	if (!path)
+		return -ENOMEM;
+
+	ret = btrfs_search_slot(trans, stripe_root, &stripe_key, path, -1, 1);
+	if (ret < 0)
+		goto out;
+
+	ret = btrfs_del_item(trans, stripe_root, path);
+out:
+	btrfs_free_path(path);
+	return ret;
+
+}
+
 int btrfs_insert_preallocated_raid_stripe(struct btrfs_fs_info *fs_info,
 					  u64 start, u64 len)
 {
diff --git a/fs/btrfs/raid-stripe-tree.h b/fs/btrfs/raid-stripe-tree.h
index 60d3f8489cc9..12d2f588b22d 100644
--- a/fs/btrfs/raid-stripe-tree.h
+++ b/fs/btrfs/raid-stripe-tree.h
@@ -22,6 +22,8 @@  struct btrfs_ordered_stripe {
 	refcount_t ref;
 };
 
+int btrfs_delete_raid_extent(struct btrfs_trans_handle *trans, u64 start,
+			     u64 length);
 int btrfs_insert_raid_extent(struct btrfs_trans_handle *trans,
 			     struct btrfs_ordered_stripe *stripe);
 int btrfs_insert_preallocated_raid_stripe(struct btrfs_fs_info *fs_info,