* [PATCH 01/24] btrfs: pass a bool to btrfs_block_rsv_migrate() at evict_refill_and_join()
2023-03-21 11:13 [PATCH 00/24] btrfs: cleanups and small fixes mostly around block reserves fdmanana
@ 2023-03-21 11:13 ` fdmanana
2023-03-21 11:40 ` Anand Jain
2023-03-21 12:18 ` Johannes Thumshirn
2023-03-21 11:13 ` [PATCH 02/24] btrfs: pass a bool size update argument to btrfs_block_rsv_add_bytes() fdmanana
` (24 subsequent siblings)
25 siblings, 2 replies; 47+ messages in thread
From: fdmanana @ 2023-03-21 11:13 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
The last argument of btrfs_block_rsv_migrate() is a boolean, but we are
passing an integer, with a value of 1, to it at evict_refill_and_join().
While this is not a bug, due to type conversion, it's a lot more clear to
simply pass the boolean true value instead. So just do that.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/inode.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 76d93b9e94a9..7bae75973a4d 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -5268,7 +5268,7 @@ static struct btrfs_trans_handle *evict_refill_and_join(struct btrfs_root *root,
trans->block_rsv = &fs_info->trans_block_rsv;
trans->bytes_reserved = delayed_refs_extra;
btrfs_block_rsv_migrate(rsv, trans->block_rsv,
- delayed_refs_extra, 1);
+ delayed_refs_extra, true);
}
return trans;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH 02/24] btrfs: pass a bool size update argument to btrfs_block_rsv_add_bytes()
2023-03-21 11:13 [PATCH 00/24] btrfs: cleanups and small fixes mostly around block reserves fdmanana
2023-03-21 11:13 ` [PATCH 01/24] btrfs: pass a bool to btrfs_block_rsv_migrate() at evict_refill_and_join() fdmanana
@ 2023-03-21 11:13 ` fdmanana
2023-03-21 11:43 ` Anand Jain
2023-03-21 12:18 ` Johannes Thumshirn
2023-03-21 11:13 ` [PATCH 03/24] btrfs: remove check for NULL block reserve at btrfs_block_rsv_check() fdmanana
` (23 subsequent siblings)
25 siblings, 2 replies; 47+ messages in thread
From: fdmanana @ 2023-03-21 11:13 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
At btrfs_delayed_refs_rsv_refill(), we are passing a value of 0 to the
'update_size' argument of btrfs_block_rsv_add_bytes(), which is defined
as a boolean. Functionally this is fine because a 0 is, implicitly,
converted to a boolean false value. However it's easier to read an
explicit 'false' value, so just pass 'false' instead of 0.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/delayed-ref.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/btrfs/delayed-ref.c b/fs/btrfs/delayed-ref.c
index 886ffb232eac..83e1e1d0ec6a 100644
--- a/fs/btrfs/delayed-ref.c
+++ b/fs/btrfs/delayed-ref.c
@@ -217,7 +217,7 @@ int btrfs_delayed_refs_rsv_refill(struct btrfs_fs_info *fs_info,
ret = btrfs_reserve_metadata_bytes(fs_info, block_rsv, num_bytes, flush);
if (ret)
return ret;
- btrfs_block_rsv_add_bytes(block_rsv, num_bytes, 0);
+ btrfs_block_rsv_add_bytes(block_rsv, num_bytes, false);
trace_btrfs_space_reservation(fs_info, "delayed_refs_rsv",
0, num_bytes, 1);
return 0;
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH 03/24] btrfs: remove check for NULL block reserve at btrfs_block_rsv_check()
2023-03-21 11:13 [PATCH 00/24] btrfs: cleanups and small fixes mostly around block reserves fdmanana
2023-03-21 11:13 ` [PATCH 01/24] btrfs: pass a bool to btrfs_block_rsv_migrate() at evict_refill_and_join() fdmanana
2023-03-21 11:13 ` [PATCH 02/24] btrfs: pass a bool size update argument to btrfs_block_rsv_add_bytes() fdmanana
@ 2023-03-21 11:13 ` fdmanana
2023-03-21 11:48 ` Anand Jain
` (2 more replies)
2023-03-21 11:13 ` [PATCH 04/24] btrfs: update documentation for BTRFS_RESERVE_FLUSH_EVICT flush method fdmanana
` (22 subsequent siblings)
25 siblings, 3 replies; 47+ messages in thread
From: fdmanana @ 2023-03-21 11:13 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
The block reserve passed to btrfs_block_rsv_check() is never NULL, so
remove the check. In case it can ever become NULL in the future, then
we'll get a pretty obvious and clear NULL pointer dereference crash and
stack trace.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/block-rsv.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/fs/btrfs/block-rsv.c b/fs/btrfs/block-rsv.c
index 5367a14d44d2..364a3d11bf88 100644
--- a/fs/btrfs/block-rsv.c
+++ b/fs/btrfs/block-rsv.c
@@ -232,9 +232,6 @@ int btrfs_block_rsv_check(struct btrfs_block_rsv *block_rsv, int min_percent)
u64 num_bytes = 0;
int ret = -ENOSPC;
- if (!block_rsv)
- return 0;
-
spin_lock(&block_rsv->lock);
num_bytes = mult_perc(block_rsv->size, min_percent);
if (block_rsv->reserved >= num_bytes)
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [PATCH 03/24] btrfs: remove check for NULL block reserve at btrfs_block_rsv_check()
2023-03-21 11:13 ` [PATCH 03/24] btrfs: remove check for NULL block reserve at btrfs_block_rsv_check() fdmanana
@ 2023-03-21 11:48 ` Anand Jain
2023-03-21 11:54 ` Anand Jain
2023-03-21 12:19 ` Johannes Thumshirn
2 siblings, 0 replies; 47+ messages in thread
From: Anand Jain @ 2023-03-21 11:48 UTC (permalink / raw)
To: fdmanana, linux-btrfs
LGTM
Reviewed-by: Anand Jain <anand.jain@oracle.com>
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 03/24] btrfs: remove check for NULL block reserve at btrfs_block_rsv_check()
2023-03-21 11:13 ` [PATCH 03/24] btrfs: remove check for NULL block reserve at btrfs_block_rsv_check() fdmanana
2023-03-21 11:48 ` Anand Jain
@ 2023-03-21 11:54 ` Anand Jain
2023-03-21 12:19 ` Johannes Thumshirn
2 siblings, 0 replies; 47+ messages in thread
From: Anand Jain @ 2023-03-21 11:54 UTC (permalink / raw)
To: fdmanana, linux-btrfs
On 3/21/23 19:13, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
>
> The block reserve passed to btrfs_block_rsv_check() is never NULL, so
> remove the check. In case it can ever become NULL in the future, then
> we'll get a pretty obvious and clear NULL pointer dereference crash and
> stack trace.
>
> Signed-off-by: Filipe Manana <fdmanana@suse.com>
> ---
> fs/btrfs/block-rsv.c | 3 ---
> 1 file changed, 3 deletions(-)
>
> diff --git a/fs/btrfs/block-rsv.c b/fs/btrfs/block-rsv.c
> index 5367a14d44d2..364a3d11bf88 100644
> --- a/fs/btrfs/block-rsv.c
> +++ b/fs/btrfs/block-rsv.c
> @@ -232,9 +232,6 @@ int btrfs_block_rsv_check(struct btrfs_block_rsv *block_rsv, int min_percent)
I havn't checked the whole series. Is there a need for
btrfs_block_rsv_check() to return an integer? It seems that only
parent, should_end_transaction(), does not care about the error codes.
So return bool shall suffice.
Thanks, Anand
> u64 num_bytes = 0;
> int ret = -ENOSPC;
>
> - if (!block_rsv)
> - return 0;
> -
> spin_lock(&block_rsv->lock);
> num_bytes = mult_perc(block_rsv->size, min_percent);
> if (block_rsv->reserved >= num_bytes)
^ permalink raw reply [flat|nested] 47+ messages in thread
* Re: [PATCH 03/24] btrfs: remove check for NULL block reserve at btrfs_block_rsv_check()
2023-03-21 11:13 ` [PATCH 03/24] btrfs: remove check for NULL block reserve at btrfs_block_rsv_check() fdmanana
2023-03-21 11:48 ` Anand Jain
2023-03-21 11:54 ` Anand Jain
@ 2023-03-21 12:19 ` Johannes Thumshirn
2 siblings, 0 replies; 47+ messages in thread
From: Johannes Thumshirn @ 2023-03-21 12:19 UTC (permalink / raw)
To: fdmanana, linux-btrfs
Looks good,
Reviewed-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
^ permalink raw reply [flat|nested] 47+ messages in thread
* [PATCH 04/24] btrfs: update documentation for BTRFS_RESERVE_FLUSH_EVICT flush method
2023-03-21 11:13 [PATCH 00/24] btrfs: cleanups and small fixes mostly around block reserves fdmanana
` (2 preceding siblings ...)
2023-03-21 11:13 ` [PATCH 03/24] btrfs: remove check for NULL block reserve at btrfs_block_rsv_check() fdmanana
@ 2023-03-21 11:13 ` fdmanana
2023-03-21 12:19 ` Johannes Thumshirn
2023-03-21 11:13 ` [PATCH 05/24] btrfs: update flush method assertion when reserving space fdmanana
` (21 subsequent siblings)
25 siblings, 1 reply; 47+ messages in thread
From: fdmanana @ 2023-03-21 11:13 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
The BTRFS_RESERVE_FLUSH_EVICT flush method can also commit transactions,
see the definition of the evict_flush_states const array at space-info.c,
but the documentation for it at space-info.h does not mention it.
So update the documentation.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/space-info.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/fs/btrfs/space-info.h b/fs/btrfs/space-info.h
index 2033b71b18ce..0bb9d14e60a8 100644
--- a/fs/btrfs/space-info.h
+++ b/fs/btrfs/space-info.h
@@ -27,6 +27,7 @@ enum btrfs_reserve_flush_enum {
* - Running delayed refs
* - Running delalloc and waiting for ordered extents
* - Allocating a new chunk
+ * - Committing transaction
*/
BTRFS_RESERVE_FLUSH_EVICT,
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH 05/24] btrfs: update flush method assertion when reserving space
2023-03-21 11:13 [PATCH 00/24] btrfs: cleanups and small fixes mostly around block reserves fdmanana
` (3 preceding siblings ...)
2023-03-21 11:13 ` [PATCH 04/24] btrfs: update documentation for BTRFS_RESERVE_FLUSH_EVICT flush method fdmanana
@ 2023-03-21 11:13 ` fdmanana
2023-03-21 11:13 ` [PATCH 06/24] btrfs: initialize ret to -ENOSPC at __reserve_bytes() fdmanana
` (20 subsequent siblings)
25 siblings, 0 replies; 47+ messages in thread
From: fdmanana @ 2023-03-21 11:13 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
When reserving space, at space-info.c:__reserve_bytes(), we assert that
either the current task is not holding a transacion handle, or, if it is,
that the flush method is not BTRFS_RESERVE_FLUSH_ALL. This is because that
flush method can trigger transaction commits, and therefore could lead to
a deadlock.
However there are other 2 flush methods that can trigger transaction
commits:
1) BTRFS_RESERVE_FLUSH_ALL_STEAL
2) BTRFS_RESERVE_FLUSH_EVICT
So update the assertion to check the flush method is also not one those
two methods if the current task is holding a transaction handle.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/space-info.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c
index 3eecce86f63f..379a0e778dfb 100644
--- a/fs/btrfs/space-info.c
+++ b/fs/btrfs/space-info.c
@@ -1603,7 +1603,18 @@ static int __reserve_bytes(struct btrfs_fs_info *fs_info,
bool pending_tickets;
ASSERT(orig_bytes);
- ASSERT(!current->journal_info || flush != BTRFS_RESERVE_FLUSH_ALL);
+ /*
+ * If have a transaction handle (current->journal_info != NULL), then
+ * the flush method can not be neither BTRFS_RESERVE_FLUSH_ALL* nor
+ * BTRFS_RESERVE_FLUSH_EVICT, as we could deadlock because those
+ * flushing methods can trigger transaction commits.
+ */
+ if (current->journal_info) {
+ /* One assert per line for easier debugging. */
+ ASSERT(flush != BTRFS_RESERVE_FLUSH_ALL);
+ ASSERT(flush != BTRFS_RESERVE_FLUSH_ALL_STEAL);
+ ASSERT(flush != BTRFS_RESERVE_FLUSH_EVICT);
+ }
if (flush == BTRFS_RESERVE_FLUSH_DATA)
async_work = &fs_info->async_data_reclaim_work;
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH 06/24] btrfs: initialize ret to -ENOSPC at __reserve_bytes()
2023-03-21 11:13 [PATCH 00/24] btrfs: cleanups and small fixes mostly around block reserves fdmanana
` (4 preceding siblings ...)
2023-03-21 11:13 ` [PATCH 05/24] btrfs: update flush method assertion when reserving space fdmanana
@ 2023-03-21 11:13 ` fdmanana
2023-03-21 12:24 ` Johannes Thumshirn
2023-03-21 12:43 ` Anand Jain
2023-03-21 11:13 ` [PATCH 07/24] btrfs: simplify btrfs_should_throttle_delayed_refs() fdmanana
` (19 subsequent siblings)
25 siblings, 2 replies; 47+ messages in thread
From: fdmanana @ 2023-03-21 11:13 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
At space-info.c:__reserve_bytes(), instead of initializing 'ret' to 0 when
it's declared and then shortly after set it to -ENOSPC under the space
info's spinlock, initialize it to -ENOSPC when declaring it.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/space-info.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c
index 379a0e778dfb..5eb161d96e35 100644
--- a/fs/btrfs/space-info.c
+++ b/fs/btrfs/space-info.c
@@ -1599,7 +1599,7 @@ static int __reserve_bytes(struct btrfs_fs_info *fs_info,
struct reserve_ticket ticket;
u64 start_ns = 0;
u64 used;
- int ret = 0;
+ int ret = -ENOSPC;
bool pending_tickets;
ASSERT(orig_bytes);
@@ -1622,7 +1622,6 @@ static int __reserve_bytes(struct btrfs_fs_info *fs_info,
async_work = &fs_info->async_reclaim_work;
spin_lock(&space_info->lock);
- ret = -ENOSPC;
used = btrfs_space_info_used(space_info, true);
/*
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH 07/24] btrfs: simplify btrfs_should_throttle_delayed_refs()
2023-03-21 11:13 [PATCH 00/24] btrfs: cleanups and small fixes mostly around block reserves fdmanana
` (5 preceding siblings ...)
2023-03-21 11:13 ` [PATCH 06/24] btrfs: initialize ret to -ENOSPC at __reserve_bytes() fdmanana
@ 2023-03-21 11:13 ` fdmanana
2023-03-21 12:52 ` Anand Jain
2023-03-21 11:13 ` [PATCH 08/24] btrfs: collapse should_end_transaction() into btrfs_should_end_transaction() fdmanana
` (18 subsequent siblings)
25 siblings, 1 reply; 47+ messages in thread
From: fdmanana @ 2023-03-21 11:13 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
Currently btrfs_should_throttle_delayed_refs() returns 1 or 2 in case the
delayed refs should be throttled, however the only caller (inode eviction
and truncation path) does not care about those two different conditions,
it treats the return value as a boolean. This allows us to remove one of
the conditions in btrfs_should_throttle_delayed_refs() and change its
return value from 'int' to 'bool'. So just do that.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/delayed-ref.c | 6 ++----
fs/btrfs/delayed-ref.h | 2 +-
2 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/fs/btrfs/delayed-ref.c b/fs/btrfs/delayed-ref.c
index 83e1e1d0ec6a..3fdee91d8079 100644
--- a/fs/btrfs/delayed-ref.c
+++ b/fs/btrfs/delayed-ref.c
@@ -53,7 +53,7 @@ bool btrfs_check_space_for_delayed_refs(struct btrfs_fs_info *fs_info)
return ret;
}
-int btrfs_should_throttle_delayed_refs(struct btrfs_trans_handle *trans)
+bool btrfs_should_throttle_delayed_refs(struct btrfs_trans_handle *trans)
{
u64 num_entries =
atomic_read(&trans->transaction->delayed_refs.num_entries);
@@ -63,10 +63,8 @@ int btrfs_should_throttle_delayed_refs(struct btrfs_trans_handle *trans)
smp_mb();
avg_runtime = trans->fs_info->avg_delayed_ref_runtime;
val = num_entries * avg_runtime;
- if (val >= NSEC_PER_SEC)
- return 1;
if (val >= NSEC_PER_SEC / 2)
- return 2;
+ return true;
return btrfs_check_space_for_delayed_refs(trans->fs_info);
}
diff --git a/fs/btrfs/delayed-ref.h b/fs/btrfs/delayed-ref.h
index 2eb34abf700f..316fed159d49 100644
--- a/fs/btrfs/delayed-ref.h
+++ b/fs/btrfs/delayed-ref.h
@@ -385,7 +385,7 @@ int btrfs_delayed_refs_rsv_refill(struct btrfs_fs_info *fs_info,
void btrfs_migrate_to_delayed_refs_rsv(struct btrfs_fs_info *fs_info,
struct btrfs_block_rsv *src,
u64 num_bytes);
-int btrfs_should_throttle_delayed_refs(struct btrfs_trans_handle *trans);
+bool btrfs_should_throttle_delayed_refs(struct btrfs_trans_handle *trans);
bool btrfs_check_space_for_delayed_refs(struct btrfs_fs_info *fs_info);
/*
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH 08/24] btrfs: collapse should_end_transaction() into btrfs_should_end_transaction()
2023-03-21 11:13 [PATCH 00/24] btrfs: cleanups and small fixes mostly around block reserves fdmanana
` (6 preceding siblings ...)
2023-03-21 11:13 ` [PATCH 07/24] btrfs: simplify btrfs_should_throttle_delayed_refs() fdmanana
@ 2023-03-21 11:13 ` fdmanana
2023-03-21 13:00 ` Anand Jain
2023-03-21 11:13 ` [PATCH 09/24] btrfs: remove bytes_used argument from btrfs_make_block_group() fdmanana
` (17 subsequent siblings)
25 siblings, 1 reply; 47+ messages in thread
From: fdmanana @ 2023-03-21 11:13 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
The function should_end_transaction() is very short and only has one
caller, which is btrfs_should_end_transaction(). So move the code from
should_end_transaction() into btrfs_should_end_transaction().
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/transaction.c | 15 ++++-----------
1 file changed, 4 insertions(+), 11 deletions(-)
diff --git a/fs/btrfs/transaction.c b/fs/btrfs/transaction.c
index 18329ebcb1cb..c47b6838754e 100644
--- a/fs/btrfs/transaction.c
+++ b/fs/btrfs/transaction.c
@@ -942,16 +942,6 @@ void btrfs_throttle(struct btrfs_fs_info *fs_info)
wait_current_trans(fs_info);
}
-static bool should_end_transaction(struct btrfs_trans_handle *trans)
-{
- struct btrfs_fs_info *fs_info = trans->fs_info;
-
- if (btrfs_check_space_for_delayed_refs(fs_info))
- return true;
-
- return !!btrfs_block_rsv_check(&fs_info->global_block_rsv, 50);
-}
-
bool btrfs_should_end_transaction(struct btrfs_trans_handle *trans)
{
struct btrfs_transaction *cur_trans = trans->transaction;
@@ -960,7 +950,10 @@ bool btrfs_should_end_transaction(struct btrfs_trans_handle *trans)
test_bit(BTRFS_DELAYED_REFS_FLUSHING, &cur_trans->delayed_refs.flags))
return true;
- return should_end_transaction(trans);
+ if (btrfs_check_space_for_delayed_refs(trans->fs_info))
+ return true;
+
+ return !!btrfs_block_rsv_check(&trans->fs_info->global_block_rsv, 50);
}
static void btrfs_trans_release_metadata(struct btrfs_trans_handle *trans)
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH 09/24] btrfs: remove bytes_used argument from btrfs_make_block_group()
2023-03-21 11:13 [PATCH 00/24] btrfs: cleanups and small fixes mostly around block reserves fdmanana
` (7 preceding siblings ...)
2023-03-21 11:13 ` [PATCH 08/24] btrfs: collapse should_end_transaction() into btrfs_should_end_transaction() fdmanana
@ 2023-03-21 11:13 ` fdmanana
2023-03-21 13:05 ` Anand Jain
2023-03-21 11:13 ` [PATCH 10/24] btrfs: count extents before taking inode's spinlock when reserving metadata fdmanana
` (16 subsequent siblings)
25 siblings, 1 reply; 47+ messages in thread
From: fdmanana @ 2023-03-21 11:13 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
The only caller of btrfs_make_block_group() always passes 0 as the value
for the bytes_used argument, so remove it.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/block-group.c | 7 ++-----
fs/btrfs/block-group.h | 2 +-
fs/btrfs/volumes.c | 2 +-
3 files changed, 4 insertions(+), 7 deletions(-)
diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index 46a8ca24afaa..bb6024c17db4 100644
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c
@@ -2672,7 +2672,7 @@ static u64 calculate_global_root_id(struct btrfs_fs_info *fs_info, u64 offset)
}
struct btrfs_block_group *btrfs_make_block_group(struct btrfs_trans_handle *trans,
- u64 bytes_used, u64 type,
+ u64 type,
u64 chunk_offset, u64 size)
{
struct btrfs_fs_info *fs_info = trans->fs_info;
@@ -2687,7 +2687,6 @@ struct btrfs_block_group *btrfs_make_block_group(struct btrfs_trans_handle *tran
cache->length = size;
set_free_space_tree_thresholds(cache);
- cache->used = bytes_used;
cache->flags = type;
cache->cached = BTRFS_CACHE_FINISHED;
cache->global_root_id = calculate_global_root_id(fs_info, cache->start);
@@ -2738,9 +2737,7 @@ struct btrfs_block_group *btrfs_make_block_group(struct btrfs_trans_handle *tran
#ifdef CONFIG_BTRFS_DEBUG
if (btrfs_should_fragment_free_space(cache)) {
- u64 new_bytes_used = size - bytes_used;
-
- cache->space_info->bytes_used += new_bytes_used >> 1;
+ cache->space_info->bytes_used += size >> 1;
fragment_free_space(cache);
}
#endif
diff --git a/fs/btrfs/block-group.h b/fs/btrfs/block-group.h
index 6e4a0b429ac3..db729ad7315b 100644
--- a/fs/btrfs/block-group.h
+++ b/fs/btrfs/block-group.h
@@ -302,7 +302,7 @@ void btrfs_reclaim_bgs(struct btrfs_fs_info *fs_info);
void btrfs_mark_bg_to_reclaim(struct btrfs_block_group *bg);
int btrfs_read_block_groups(struct btrfs_fs_info *info);
struct btrfs_block_group *btrfs_make_block_group(struct btrfs_trans_handle *trans,
- u64 bytes_used, u64 type,
+ u64 type,
u64 chunk_offset, u64 size);
void btrfs_create_pending_block_groups(struct btrfs_trans_handle *trans);
int btrfs_inc_block_group_ro(struct btrfs_block_group *cache,
diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 93bc45001e68..5da6f5167046 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -5421,7 +5421,7 @@ static struct btrfs_block_group *create_chunk(struct btrfs_trans_handle *trans,
}
write_unlock(&em_tree->lock);
- block_group = btrfs_make_block_group(trans, 0, type, start, ctl->chunk_size);
+ block_group = btrfs_make_block_group(trans, type, start, ctl->chunk_size);
if (IS_ERR(block_group))
goto error_del_extent;
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH 10/24] btrfs: count extents before taking inode's spinlock when reserving metadata
2023-03-21 11:13 [PATCH 00/24] btrfs: cleanups and small fixes mostly around block reserves fdmanana
` (8 preceding siblings ...)
2023-03-21 11:13 ` [PATCH 09/24] btrfs: remove bytes_used argument from btrfs_make_block_group() fdmanana
@ 2023-03-21 11:13 ` fdmanana
2023-03-21 13:26 ` Anand Jain
2023-03-21 11:13 ` [PATCH 11/24] btrfs: remove redundant counter check at btrfs_truncate_inode_items() fdmanana
` (15 subsequent siblings)
25 siblings, 1 reply; 47+ messages in thread
From: fdmanana @ 2023-03-21 11:13 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
When reserving metadata space for delalloc (and direct IO too), at
btrfs_delalloc_reserve_metadata(), there's no need to count the number of
extents while holding the inode's spinlock, since that does not require
access to any field of the inode.
This section of code can be called concurrently, when we have direct IO
writes against different file ranges that don't increase the inode's
i_size, so it's beneficial to shorten the critical section by counting
the number of extents before taking the inode's spinlock.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/delalloc-space.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/btrfs/delalloc-space.c b/fs/btrfs/delalloc-space.c
index 7ddb1d104e8e..427abaf608b8 100644
--- a/fs/btrfs/delalloc-space.c
+++ b/fs/btrfs/delalloc-space.c
@@ -358,8 +358,8 @@ int btrfs_delalloc_reserve_metadata(struct btrfs_inode *inode, u64 num_bytes,
* racing with an ordered completion or some such that would think it
* needs to free the reservation we just made.
*/
- spin_lock(&inode->lock);
nr_extents = count_max_extents(fs_info, num_bytes);
+ spin_lock(&inode->lock);
btrfs_mod_outstanding_extents(inode, nr_extents);
inode->csum_bytes += disk_num_bytes;
btrfs_calculate_inode_block_rsv_size(fs_info, inode);
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH 11/24] btrfs: remove redundant counter check at btrfs_truncate_inode_items()
2023-03-21 11:13 [PATCH 00/24] btrfs: cleanups and small fixes mostly around block reserves fdmanana
` (9 preceding siblings ...)
2023-03-21 11:13 ` [PATCH 10/24] btrfs: count extents before taking inode's spinlock when reserving metadata fdmanana
@ 2023-03-21 11:13 ` fdmanana
2023-03-21 13:31 ` Anand Jain
2023-03-21 11:13 ` [PATCH 12/24] btrfs: simplify btrfs_block_rsv_refill() fdmanana
` (14 subsequent siblings)
25 siblings, 1 reply; 47+ messages in thread
From: fdmanana @ 2023-03-21 11:13 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
At btrfs_truncate_inode_items(), in the while loop when we decide that we
are going to delete an item, it's pointless to check that 'pending_del_nr'
is non-zero in an else clause because the corresponding if statement is
checking if 'pending_del_nr' has a value of zero. So just remove that
condition from the else clause.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/inode-item.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/fs/btrfs/inode-item.c b/fs/btrfs/inode-item.c
index b65c45b5d681..b27c2c560083 100644
--- a/fs/btrfs/inode-item.c
+++ b/fs/btrfs/inode-item.c
@@ -660,8 +660,7 @@ int btrfs_truncate_inode_items(struct btrfs_trans_handle *trans,
/* No pending yet, add ourselves */
pending_del_slot = path->slots[0];
pending_del_nr = 1;
- } else if (pending_del_nr &&
- path->slots[0] + 1 == pending_del_slot) {
+ } else if (path->slots[0] + 1 == pending_del_slot) {
/* Hop on the pending chunk */
pending_del_nr++;
pending_del_slot = path->slots[0];
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH 12/24] btrfs: simplify btrfs_block_rsv_refill()
2023-03-21 11:13 [PATCH 00/24] btrfs: cleanups and small fixes mostly around block reserves fdmanana
` (10 preceding siblings ...)
2023-03-21 11:13 ` [PATCH 11/24] btrfs: remove redundant counter check at btrfs_truncate_inode_items() fdmanana
@ 2023-03-21 11:13 ` fdmanana
2023-03-21 13:40 ` Anand Jain
2023-03-21 11:13 ` [PATCH 13/24] btrfs: remove obsolete delayed ref throttling logic when truncating items fdmanana
` (13 subsequent siblings)
25 siblings, 1 reply; 47+ messages in thread
From: fdmanana @ 2023-03-21 11:13 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
At btrfs_block_rsv_refill(), there's no point in initializing the
'num_bytes' variable to 0 and then, after taking the block reserve's
spinlock, initializing it to the value of the 'min_reserved' parameter.
So just get rid of the 'num_bytes' local variable and rename the
'min_reserved' parameter to 'num_bytes'.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/block-rsv.c | 4 +---
fs/btrfs/block-rsv.h | 2 +-
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/fs/btrfs/block-rsv.c b/fs/btrfs/block-rsv.c
index 364a3d11bf88..90b8088e8fac 100644
--- a/fs/btrfs/block-rsv.c
+++ b/fs/btrfs/block-rsv.c
@@ -242,17 +242,15 @@ int btrfs_block_rsv_check(struct btrfs_block_rsv *block_rsv, int min_percent)
}
int btrfs_block_rsv_refill(struct btrfs_fs_info *fs_info,
- struct btrfs_block_rsv *block_rsv, u64 min_reserved,
+ struct btrfs_block_rsv *block_rsv, u64 num_bytes,
enum btrfs_reserve_flush_enum flush)
{
- u64 num_bytes = 0;
int ret = -ENOSPC;
if (!block_rsv)
return 0;
spin_lock(&block_rsv->lock);
- num_bytes = min_reserved;
if (block_rsv->reserved >= num_bytes)
ret = 0;
else
diff --git a/fs/btrfs/block-rsv.h b/fs/btrfs/block-rsv.h
index 4cc41c9aaa82..6dc781709aca 100644
--- a/fs/btrfs/block-rsv.h
+++ b/fs/btrfs/block-rsv.h
@@ -65,7 +65,7 @@ int btrfs_block_rsv_add(struct btrfs_fs_info *fs_info,
enum btrfs_reserve_flush_enum flush);
int btrfs_block_rsv_check(struct btrfs_block_rsv *block_rsv, int min_percent);
int btrfs_block_rsv_refill(struct btrfs_fs_info *fs_info,
- struct btrfs_block_rsv *block_rsv, u64 min_reserved,
+ struct btrfs_block_rsv *block_rsv, u64 num_bytes,
enum btrfs_reserve_flush_enum flush);
int btrfs_block_rsv_migrate(struct btrfs_block_rsv *src_rsv,
struct btrfs_block_rsv *dst_rsv, u64 num_bytes,
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH 13/24] btrfs: remove obsolete delayed ref throttling logic when truncating items
2023-03-21 11:13 [PATCH 00/24] btrfs: cleanups and small fixes mostly around block reserves fdmanana
` (11 preceding siblings ...)
2023-03-21 11:13 ` [PATCH 12/24] btrfs: simplify btrfs_block_rsv_refill() fdmanana
@ 2023-03-21 11:13 ` fdmanana
2023-03-21 11:13 ` [PATCH 14/24] btrfs: don't throttle on delayed items when evicting deleted inode fdmanana
` (12 subsequent siblings)
25 siblings, 0 replies; 47+ messages in thread
From: fdmanana @ 2023-03-21 11:13 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
We have this logic encapsulated in btrfs_should_throttle_delayed_refs()
where we try to estimate if running the current amount of delayed
references we have will take more than half a second, and if so, the
caller btrfs_should_throttle_delayed_refs() should do something to
prevent more and more delayed refs from being accumulated.
This logic was added in commit 0a2b2a844af6 ("Btrfs: throttle delayed
refs better") and then further refined in commit a79b7d4b3e81 ("Btrfs:
async delayed refs"). The idea back then was that the caller of
btrfs_should_throttle_delayed_refs() would release its transaction
handle (by calling btrfs_end_transaction()) when that function returned
true, then btrfs_end_transaction() would trigger an async job to run
delayed references in a workqueue, and later start/join a transaction
again and do more work.
However we don't run delayed references asynchronously anymore, that
was removed in commit db2462a6ad3d ("btrfs: don't run delayed refs in
the end transaction logic"). That makes the logic that tries to estimate
how long we will take to run our current delayed references, at
btrfs_should_throttle_delayed_refs(), pointless as we don't take any
action to run delayed references anymore. We do have other type of
throttling, which consists of checking the size and reserved space of
the delayed and global block reserves, as well as if fluhsing delayed
references for the current transaction was already started, etc - this
is all done by btrfs_should_end_transaction(), and the only user of
btrfs_should_throttle_delayed_refs() does periodically call
btrfs_should_end_transaction().
So remove btrfs_should_throttle_delayed_refs() and the infrastructure
that keeps track of the average time used for running delayed references,
as well as adapting btrfs_truncate_inode_items() to call
btrfs_check_space_for_delayed_refs() instead.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/delayed-ref.c | 16 ----------------
fs/btrfs/delayed-ref.h | 1 -
fs/btrfs/disk-io.c | 1 -
fs/btrfs/extent-tree.c | 27 ++-------------------------
fs/btrfs/fs.h | 1 -
fs/btrfs/inode-item.c | 12 +++++-------
6 files changed, 7 insertions(+), 51 deletions(-)
diff --git a/fs/btrfs/delayed-ref.c b/fs/btrfs/delayed-ref.c
index 3fdee91d8079..bf2ce51e5086 100644
--- a/fs/btrfs/delayed-ref.c
+++ b/fs/btrfs/delayed-ref.c
@@ -53,22 +53,6 @@ bool btrfs_check_space_for_delayed_refs(struct btrfs_fs_info *fs_info)
return ret;
}
-bool btrfs_should_throttle_delayed_refs(struct btrfs_trans_handle *trans)
-{
- u64 num_entries =
- atomic_read(&trans->transaction->delayed_refs.num_entries);
- u64 avg_runtime;
- u64 val;
-
- smp_mb();
- avg_runtime = trans->fs_info->avg_delayed_ref_runtime;
- val = num_entries * avg_runtime;
- if (val >= NSEC_PER_SEC / 2)
- return true;
-
- return btrfs_check_space_for_delayed_refs(trans->fs_info);
-}
-
/*
* Release a ref head's reservation.
*
diff --git a/fs/btrfs/delayed-ref.h b/fs/btrfs/delayed-ref.h
index 316fed159d49..6cf1adc9a01a 100644
--- a/fs/btrfs/delayed-ref.h
+++ b/fs/btrfs/delayed-ref.h
@@ -385,7 +385,6 @@ int btrfs_delayed_refs_rsv_refill(struct btrfs_fs_info *fs_info,
void btrfs_migrate_to_delayed_refs_rsv(struct btrfs_fs_info *fs_info,
struct btrfs_block_rsv *src,
u64 num_bytes);
-bool btrfs_should_throttle_delayed_refs(struct btrfs_trans_handle *trans);
bool btrfs_check_space_for_delayed_refs(struct btrfs_fs_info *fs_info);
/*
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index bb864cf2eed6..b638e27468a7 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -2951,7 +2951,6 @@ void btrfs_init_fs_info(struct btrfs_fs_info *fs_info)
atomic64_set(&fs_info->free_chunk_space, 0);
fs_info->tree_mod_log = RB_ROOT;
fs_info->commit_interval = BTRFS_DEFAULT_COMMIT_INTERVAL;
- fs_info->avg_delayed_ref_runtime = NSEC_PER_SEC >> 6; /* div by 64 */
btrfs_init_ref_verify(fs_info);
fs_info->thread_pool_size = min_t(unsigned long,
diff --git a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c
index 6b6c59e6805c..5cd289de4e92 100644
--- a/fs/btrfs/extent-tree.c
+++ b/fs/btrfs/extent-tree.c
@@ -1894,8 +1894,7 @@ static struct btrfs_delayed_ref_head *btrfs_obtain_ref_head(
}
static int btrfs_run_delayed_refs_for_head(struct btrfs_trans_handle *trans,
- struct btrfs_delayed_ref_head *locked_ref,
- unsigned long *run_refs)
+ struct btrfs_delayed_ref_head *locked_ref)
{
struct btrfs_fs_info *fs_info = trans->fs_info;
struct btrfs_delayed_ref_root *delayed_refs;
@@ -1917,7 +1916,6 @@ static int btrfs_run_delayed_refs_for_head(struct btrfs_trans_handle *trans,
return -EAGAIN;
}
- (*run_refs)++;
ref->in_tree = 0;
rb_erase_cached(&ref->ref_node, &locked_ref->ref_tree);
RB_CLEAR_NODE(&ref->ref_node);
@@ -1981,10 +1979,8 @@ static noinline int __btrfs_run_delayed_refs(struct btrfs_trans_handle *trans,
struct btrfs_fs_info *fs_info = trans->fs_info;
struct btrfs_delayed_ref_root *delayed_refs;
struct btrfs_delayed_ref_head *locked_ref = NULL;
- ktime_t start = ktime_get();
int ret;
unsigned long count = 0;
- unsigned long actual_count = 0;
delayed_refs = &trans->transaction->delayed_refs;
do {
@@ -2014,8 +2010,7 @@ static noinline int __btrfs_run_delayed_refs(struct btrfs_trans_handle *trans,
spin_lock(&locked_ref->lock);
btrfs_merge_delayed_refs(fs_info, delayed_refs, locked_ref);
- ret = btrfs_run_delayed_refs_for_head(trans, locked_ref,
- &actual_count);
+ ret = btrfs_run_delayed_refs_for_head(trans, locked_ref);
if (ret < 0 && ret != -EAGAIN) {
/*
* Error, btrfs_run_delayed_refs_for_head already
@@ -2046,24 +2041,6 @@ static noinline int __btrfs_run_delayed_refs(struct btrfs_trans_handle *trans,
cond_resched();
} while ((nr != -1 && count < nr) || locked_ref);
- /*
- * We don't want to include ref heads since we can have empty ref heads
- * and those will drastically skew our runtime down since we just do
- * accounting, no actual extent tree updates.
- */
- if (actual_count > 0) {
- u64 runtime = ktime_to_ns(ktime_sub(ktime_get(), start));
- u64 avg;
-
- /*
- * We weigh the current average higher than our current runtime
- * to avoid large swings in the average.
- */
- spin_lock(&delayed_refs->lock);
- avg = fs_info->avg_delayed_ref_runtime * 3 + runtime;
- fs_info->avg_delayed_ref_runtime = avg >> 2; /* div by 4 */
- spin_unlock(&delayed_refs->lock);
- }
return 0;
}
diff --git a/fs/btrfs/fs.h b/fs/btrfs/fs.h
index abe5b3475a9d..492436e1a59e 100644
--- a/fs/btrfs/fs.h
+++ b/fs/btrfs/fs.h
@@ -407,7 +407,6 @@ struct btrfs_fs_info {
* Must be written and read while holding btrfs_fs_info::commit_root_sem.
*/
u64 last_reloc_trans;
- u64 avg_delayed_ref_runtime;
/*
* This is updated to the current trans every time a full commit is
diff --git a/fs/btrfs/inode-item.c b/fs/btrfs/inode-item.c
index b27c2c560083..4c322b720a80 100644
--- a/fs/btrfs/inode-item.c
+++ b/fs/btrfs/inode-item.c
@@ -527,7 +527,7 @@ int btrfs_truncate_inode_items(struct btrfs_trans_handle *trans,
while (1) {
u64 clear_start = 0, clear_len = 0, extent_start = 0;
- bool should_throttle = false;
+ bool refill_delayed_refs_rsv = false;
fi = NULL;
leaf = path->nodes[0];
@@ -685,10 +685,8 @@ int btrfs_truncate_inode_items(struct btrfs_trans_handle *trans,
btrfs_abort_transaction(trans, ret);
break;
}
- if (be_nice) {
- if (btrfs_should_throttle_delayed_refs(trans))
- should_throttle = true;
- }
+ if (be_nice && btrfs_check_space_for_delayed_refs(fs_info))
+ refill_delayed_refs_rsv = true;
}
if (found_type == BTRFS_INODE_ITEM_KEY)
@@ -696,7 +694,7 @@ int btrfs_truncate_inode_items(struct btrfs_trans_handle *trans,
if (path->slots[0] == 0 ||
path->slots[0] != pending_del_slot ||
- should_throttle) {
+ refill_delayed_refs_rsv) {
if (pending_del_nr) {
ret = btrfs_del_items(trans, root, path,
pending_del_slot,
@@ -719,7 +717,7 @@ int btrfs_truncate_inode_items(struct btrfs_trans_handle *trans,
* actually allocate, so just bail if we're short and
* let the normal reservation dance happen higher up.
*/
- if (should_throttle) {
+ if (refill_delayed_refs_rsv) {
ret = btrfs_delayed_refs_rsv_refill(fs_info,
BTRFS_RESERVE_NO_FLUSH);
if (ret) {
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH 14/24] btrfs: don't throttle on delayed items when evicting deleted inode
2023-03-21 11:13 [PATCH 00/24] btrfs: cleanups and small fixes mostly around block reserves fdmanana
` (12 preceding siblings ...)
2023-03-21 11:13 ` [PATCH 13/24] btrfs: remove obsolete delayed ref throttling logic when truncating items fdmanana
@ 2023-03-21 11:13 ` fdmanana
2023-03-21 11:13 ` [PATCH 15/24] btrfs: calculate the right space for a single delayed ref when refilling fdmanana
` (11 subsequent siblings)
25 siblings, 0 replies; 47+ messages in thread
From: fdmanana @ 2023-03-21 11:13 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
During inode eviction, if we are truncating a deleted inode, we don't add
delayed items for our inode, so there's no need to throttle on delayed
items on each iteration of the loop that truncates inode items from its
subvolume tree. But we dirty extent buffers from its subvolume tree, so
we only need to throttle on btree inode dirty pages.
So use btrfs_btree_balance_dirty_nodelay() in the loop that truncates
inode items.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/inode.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 7bae75973a4d..912d5f4aafbc 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -5350,7 +5350,12 @@ void btrfs_evict_inode(struct inode *inode)
ret = btrfs_truncate_inode_items(trans, root, &control);
trans->block_rsv = &fs_info->trans_block_rsv;
btrfs_end_transaction(trans);
- btrfs_btree_balance_dirty(fs_info);
+ /*
+ * We have not added new delayed items for our inode after we
+ * have flushed its delayed items, so no need to throttle on
+ * delayed items. However we have modified extent buffers.
+ */
+ btrfs_btree_balance_dirty_nodelay(fs_info);
if (ret && ret != -ENOSPC && ret != -EAGAIN)
goto free_rsv;
else if (!ret)
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH 15/24] btrfs: calculate the right space for a single delayed ref when refilling
2023-03-21 11:13 [PATCH 00/24] btrfs: cleanups and small fixes mostly around block reserves fdmanana
` (13 preceding siblings ...)
2023-03-21 11:13 ` [PATCH 14/24] btrfs: don't throttle on delayed items when evicting deleted inode fdmanana
@ 2023-03-21 11:13 ` fdmanana
2023-03-21 13:59 ` Anand Jain
2023-03-21 11:13 ` [PATCH 16/24] btrfs: accurately calculate number of delayed refs when flushing fdmanana
` (10 subsequent siblings)
25 siblings, 1 reply; 47+ messages in thread
From: fdmanana @ 2023-03-21 11:13 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
When refilling the delayed block reserve we are incorrectly computing the
amount of bytes for a single delayed reference if the free space tree is
being used. In that case we should double the calculated amount.
Everywhere else we compute the correct amount, like when updating the
delayed block reserve, at btrfs_update_delayed_refs_rsv(), or when
releasing space from the delayed block reserve, at
btrfs_delayed_refs_rsv_release().
So fix btrfs_delayed_refs_rsv_refill() to multiply the amount of bytes for
a single delayed reference by two in case the free space tree is used.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/delayed-ref.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/fs/btrfs/delayed-ref.c b/fs/btrfs/delayed-ref.c
index bf2ce51e5086..b58a7e30d37c 100644
--- a/fs/btrfs/delayed-ref.c
+++ b/fs/btrfs/delayed-ref.c
@@ -186,6 +186,17 @@ int btrfs_delayed_refs_rsv_refill(struct btrfs_fs_info *fs_info,
u64 num_bytes = 0;
int ret = -ENOSPC;
+ /*
+ * We have to check the mount option here because we could be enabling
+ * the free space tree for the first time and don't have the compat_ro
+ * option set yet.
+ *
+ * We need extra reservations if we have the free space tree because
+ * we'll have to modify that tree as well.
+ */
+ if (btrfs_test_opt(fs_info, FREE_SPACE_TREE))
+ limit *= 2;
+
spin_lock(&block_rsv->lock);
if (block_rsv->reserved < block_rsv->size) {
num_bytes = block_rsv->size - block_rsv->reserved;
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH 16/24] btrfs: accurately calculate number of delayed refs when flushing
2023-03-21 11:13 [PATCH 00/24] btrfs: cleanups and small fixes mostly around block reserves fdmanana
` (14 preceding siblings ...)
2023-03-21 11:13 ` [PATCH 15/24] btrfs: calculate the right space for a single delayed ref when refilling fdmanana
@ 2023-03-21 11:13 ` fdmanana
2023-03-21 11:13 ` [PATCH 17/24] btrfs: constify fs_info argument of the metadata size calculation helpers fdmanana
` (9 subsequent siblings)
25 siblings, 0 replies; 47+ messages in thread
From: fdmanana @ 2023-03-21 11:13 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
When flushing a limited number of delayed references (FLUSH_DELAYED_REFS_NR
state), we are assuming each delayed reference is holding a number of bytes
matching the needed space for inserting for a single metadata item (the
result of btrfs_calc_insert_metadata_size()). That is not correct when
using the free space tree, as in that case we have to multiply that value
by 2 since we need to touch the free space tree as well. This is the same
computation as we do at btrfs_update_delayed_refs_rsv() and at
btrfs_delayed_refs_rsv_release().
So correct the computation for the amount of delayed references we need to
flush in case we have the free space tree. This does not fix a functional
issue, instead it makes the flush code flush less delayed references, only
the minimum necessary to satisfy a ticket.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/space-info.c | 26 +++++++++++++++++++++++++-
1 file changed, 25 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c
index 5eb161d96e35..f36b16ee0a02 100644
--- a/fs/btrfs/space-info.c
+++ b/fs/btrfs/space-info.c
@@ -550,6 +550,30 @@ static inline u64 calc_reclaim_items_nr(struct btrfs_fs_info *fs_info,
return nr;
}
+static inline u64 calc_delayed_refs_nr(struct btrfs_fs_info *fs_info,
+ u64 to_reclaim)
+{
+ u64 bytes;
+ u64 nr;
+
+ bytes = btrfs_calc_insert_metadata_size(fs_info, 1);
+ /*
+ * We have to check the mount option here because we could be enabling
+ * the free space tree for the first time and don't have the compat_ro
+ * option set yet.
+ *
+ * We need extra reservations if we have the free space tree because
+ * we'll have to modify that tree as well.
+ */
+ if (btrfs_test_opt(fs_info, FREE_SPACE_TREE))
+ bytes *= 2;
+
+ nr = div64_u64(to_reclaim, bytes);
+ if (!nr)
+ nr = 1;
+ return nr;
+}
+
#define EXTENT_SIZE_PER_ITEM SZ_256K
/*
@@ -727,7 +751,7 @@ static void flush_space(struct btrfs_fs_info *fs_info,
break;
}
if (state == FLUSH_DELAYED_REFS_NR)
- nr = calc_reclaim_items_nr(fs_info, num_bytes);
+ nr = calc_delayed_refs_nr(fs_info, num_bytes);
else
nr = 0;
btrfs_run_delayed_refs(trans, nr);
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH 17/24] btrfs: constify fs_info argument of the metadata size calculation helpers
2023-03-21 11:13 [PATCH 00/24] btrfs: cleanups and small fixes mostly around block reserves fdmanana
` (15 preceding siblings ...)
2023-03-21 11:13 ` [PATCH 16/24] btrfs: accurately calculate number of delayed refs when flushing fdmanana
@ 2023-03-21 11:13 ` fdmanana
2023-03-21 15:09 ` Anand Jain
2023-03-21 11:13 ` [PATCH 18/24] btrfs: constify fs_info argument for the reclaim items " fdmanana
` (8 subsequent siblings)
25 siblings, 1 reply; 47+ messages in thread
From: fdmanana @ 2023-03-21 11:13 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
The fs_info argument of the helpers btrfs_calc_insert_metadata_size() and
btrfs_calc_metadata_size() is not modified so it can be const. This will
also allow a new helper function in one of the next patches to have its
fs_info argument as const.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/fs.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/btrfs/fs.h b/fs/btrfs/fs.h
index 492436e1a59e..0ce43318ac0e 100644
--- a/fs/btrfs/fs.h
+++ b/fs/btrfs/fs.h
@@ -822,7 +822,7 @@ static inline u64 btrfs_csum_bytes_to_leaves(
* Use this if we would be adding new items, as we could split nodes as we cow
* down the tree.
*/
-static inline u64 btrfs_calc_insert_metadata_size(struct btrfs_fs_info *fs_info,
+static inline u64 btrfs_calc_insert_metadata_size(const struct btrfs_fs_info *fs_info,
unsigned num_items)
{
return (u64)fs_info->nodesize * BTRFS_MAX_LEVEL * 2 * num_items;
@@ -832,7 +832,7 @@ static inline u64 btrfs_calc_insert_metadata_size(struct btrfs_fs_info *fs_info,
* Doing a truncate or a modification won't result in new nodes or leaves, just
* what we need for COW.
*/
-static inline u64 btrfs_calc_metadata_size(struct btrfs_fs_info *fs_info,
+static inline u64 btrfs_calc_metadata_size(const struct btrfs_fs_info *fs_info,
unsigned num_items)
{
return (u64)fs_info->nodesize * BTRFS_MAX_LEVEL * num_items;
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH 18/24] btrfs: constify fs_info argument for the reclaim items calculation helpers
2023-03-21 11:13 [PATCH 00/24] btrfs: cleanups and small fixes mostly around block reserves fdmanana
` (16 preceding siblings ...)
2023-03-21 11:13 ` [PATCH 17/24] btrfs: constify fs_info argument of the metadata size calculation helpers fdmanana
@ 2023-03-21 11:13 ` fdmanana
2023-03-21 14:24 ` Anand Jain
2023-03-21 11:13 ` [PATCH 19/24] btrfs: add helper to calculate space for delayed references fdmanana
` (7 subsequent siblings)
25 siblings, 1 reply; 47+ messages in thread
From: fdmanana @ 2023-03-21 11:13 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
Now that btrfs_calc_insert_metadata_size() can take a const fs_info
argument, make the fs_info argument of calc_reclaim_items_nr() and of
calc_delayed_refs_nr() const as well.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/space-info.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c
index f36b16ee0a02..a2e14c410416 100644
--- a/fs/btrfs/space-info.c
+++ b/fs/btrfs/space-info.c
@@ -537,7 +537,7 @@ void btrfs_dump_space_info(struct btrfs_fs_info *fs_info,
up_read(&info->groups_sem);
}
-static inline u64 calc_reclaim_items_nr(struct btrfs_fs_info *fs_info,
+static inline u64 calc_reclaim_items_nr(const struct btrfs_fs_info *fs_info,
u64 to_reclaim)
{
u64 bytes;
@@ -550,7 +550,7 @@ static inline u64 calc_reclaim_items_nr(struct btrfs_fs_info *fs_info,
return nr;
}
-static inline u64 calc_delayed_refs_nr(struct btrfs_fs_info *fs_info,
+static inline u64 calc_delayed_refs_nr(const struct btrfs_fs_info *fs_info,
u64 to_reclaim)
{
u64 bytes;
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH 19/24] btrfs: add helper to calculate space for delayed references
2023-03-21 11:13 [PATCH 00/24] btrfs: cleanups and small fixes mostly around block reserves fdmanana
` (17 preceding siblings ...)
2023-03-21 11:13 ` [PATCH 18/24] btrfs: constify fs_info argument for the reclaim items " fdmanana
@ 2023-03-21 11:13 ` fdmanana
2023-03-21 11:13 ` [PATCH 20/24] btrfs: calculate correct amount of space for delayed reference when evicting fdmanana
` (6 subsequent siblings)
25 siblings, 0 replies; 47+ messages in thread
From: fdmanana @ 2023-03-21 11:13 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
Instead of duplicating the logic for calculating how much space is
required for a given number of delayed references, add an inline helper
to encapsulate that logic and use it everywhere we are calculating the
space required.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/delayed-ref.c | 40 ++++------------------------------------
fs/btrfs/delayed-ref.h | 21 +++++++++++++++++++++
fs/btrfs/space-info.c | 14 +-------------
3 files changed, 26 insertions(+), 49 deletions(-)
diff --git a/fs/btrfs/delayed-ref.c b/fs/btrfs/delayed-ref.c
index b58a7e30d37c..0b32432d7d56 100644
--- a/fs/btrfs/delayed-ref.c
+++ b/fs/btrfs/delayed-ref.c
@@ -65,20 +65,9 @@ bool btrfs_check_space_for_delayed_refs(struct btrfs_fs_info *fs_info)
void btrfs_delayed_refs_rsv_release(struct btrfs_fs_info *fs_info, int nr)
{
struct btrfs_block_rsv *block_rsv = &fs_info->delayed_refs_rsv;
- u64 num_bytes = btrfs_calc_insert_metadata_size(fs_info, nr);
+ const u64 num_bytes = btrfs_calc_delayed_ref_bytes(fs_info, nr);
u64 released = 0;
- /*
- * We have to check the mount option here because we could be enabling
- * the free space tree for the first time and don't have the compat_ro
- * option set yet.
- *
- * We need extra reservations if we have the free space tree because
- * we'll have to modify that tree as well.
- */
- if (btrfs_test_opt(fs_info, FREE_SPACE_TREE))
- num_bytes *= 2;
-
released = btrfs_block_rsv_release(fs_info, block_rsv, num_bytes, NULL);
if (released)
trace_btrfs_space_reservation(fs_info, "delayed_refs_rsv",
@@ -100,18 +89,8 @@ void btrfs_update_delayed_refs_rsv(struct btrfs_trans_handle *trans)
if (!trans->delayed_ref_updates)
return;
- num_bytes = btrfs_calc_insert_metadata_size(fs_info,
- trans->delayed_ref_updates);
- /*
- * We have to check the mount option here because we could be enabling
- * the free space tree for the first time and don't have the compat_ro
- * option set yet.
- *
- * We need extra reservations if we have the free space tree because
- * we'll have to modify that tree as well.
- */
- if (btrfs_test_opt(fs_info, FREE_SPACE_TREE))
- num_bytes *= 2;
+ num_bytes = btrfs_calc_delayed_ref_bytes(fs_info,
+ trans->delayed_ref_updates);
spin_lock(&delayed_rsv->lock);
delayed_rsv->size += num_bytes;
@@ -182,21 +161,10 @@ int btrfs_delayed_refs_rsv_refill(struct btrfs_fs_info *fs_info,
enum btrfs_reserve_flush_enum flush)
{
struct btrfs_block_rsv *block_rsv = &fs_info->delayed_refs_rsv;
- u64 limit = btrfs_calc_insert_metadata_size(fs_info, 1);
+ u64 limit = btrfs_calc_delayed_ref_bytes(fs_info, 1);
u64 num_bytes = 0;
int ret = -ENOSPC;
- /*
- * We have to check the mount option here because we could be enabling
- * the free space tree for the first time and don't have the compat_ro
- * option set yet.
- *
- * We need extra reservations if we have the free space tree because
- * we'll have to modify that tree as well.
- */
- if (btrfs_test_opt(fs_info, FREE_SPACE_TREE))
- limit *= 2;
-
spin_lock(&block_rsv->lock);
if (block_rsv->reserved < block_rsv->size) {
num_bytes = block_rsv->size - block_rsv->reserved;
diff --git a/fs/btrfs/delayed-ref.h b/fs/btrfs/delayed-ref.h
index 6cf1adc9a01a..b54261fe509b 100644
--- a/fs/btrfs/delayed-ref.h
+++ b/fs/btrfs/delayed-ref.h
@@ -253,6 +253,27 @@ extern struct kmem_cache *btrfs_delayed_extent_op_cachep;
int __init btrfs_delayed_ref_init(void);
void __cold btrfs_delayed_ref_exit(void);
+static inline u64 btrfs_calc_delayed_ref_bytes(const struct btrfs_fs_info *fs_info,
+ int num_delayed_refs)
+{
+ u64 num_bytes;
+
+ num_bytes = btrfs_calc_insert_metadata_size(fs_info, num_delayed_refs);
+
+ /*
+ * We have to check the mount option here because we could be enabling
+ * the free space tree for the first time and don't have the compat_ro
+ * option set yet.
+ *
+ * We need extra reservations if we have the free space tree because
+ * we'll have to modify that tree as well.
+ */
+ if (btrfs_test_opt(fs_info, FREE_SPACE_TREE))
+ num_bytes *= 2;
+
+ return num_bytes;
+}
+
static inline void btrfs_init_generic_ref(struct btrfs_ref *generic_ref,
int action, u64 bytenr, u64 len, u64 parent)
{
diff --git a/fs/btrfs/space-info.c b/fs/btrfs/space-info.c
index a2e14c410416..75e7fa337e66 100644
--- a/fs/btrfs/space-info.c
+++ b/fs/btrfs/space-info.c
@@ -553,21 +553,9 @@ static inline u64 calc_reclaim_items_nr(const struct btrfs_fs_info *fs_info,
static inline u64 calc_delayed_refs_nr(const struct btrfs_fs_info *fs_info,
u64 to_reclaim)
{
- u64 bytes;
+ const u64 bytes = btrfs_calc_delayed_ref_bytes(fs_info, 1);
u64 nr;
- bytes = btrfs_calc_insert_metadata_size(fs_info, 1);
- /*
- * We have to check the mount option here because we could be enabling
- * the free space tree for the first time and don't have the compat_ro
- * option set yet.
- *
- * We need extra reservations if we have the free space tree because
- * we'll have to modify that tree as well.
- */
- if (btrfs_test_opt(fs_info, FREE_SPACE_TREE))
- bytes *= 2;
-
nr = div64_u64(to_reclaim, bytes);
if (!nr)
nr = 1;
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH 20/24] btrfs: calculate correct amount of space for delayed reference when evicting
2023-03-21 11:13 [PATCH 00/24] btrfs: cleanups and small fixes mostly around block reserves fdmanana
` (18 preceding siblings ...)
2023-03-21 11:13 ` [PATCH 19/24] btrfs: add helper to calculate space for delayed references fdmanana
@ 2023-03-21 11:13 ` fdmanana
2023-03-21 11:13 ` [PATCH 21/24] btrfs: fix calculation of the global block reserve's size fdmanana
` (5 subsequent siblings)
25 siblings, 0 replies; 47+ messages in thread
From: fdmanana @ 2023-03-21 11:13 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
When evicting an inode, we are incorrectly calculating the amount of space
required for a single delayed reference in case the free space tree is
enabled. We have to multiply by 2 the result of
btrfs_calc_insert_metadata_size(). We should be calculating according to
the size update and space release of the delayed block reserve logic at
btrfs_update_delayed_refs_rsv() and btrfs_delayed_refs_rsv_release().
Fix this by using the btrfs_calc_delayed_ref_bytes() helper at
evict_refill_and_join() instead of btrfs_calc_insert_metadata_size().
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/inode.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 912d5f4aafbc..2e181a0a6f37 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -5230,7 +5230,7 @@ static struct btrfs_trans_handle *evict_refill_and_join(struct btrfs_root *root,
{
struct btrfs_fs_info *fs_info = root->fs_info;
struct btrfs_trans_handle *trans;
- u64 delayed_refs_extra = btrfs_calc_insert_metadata_size(fs_info, 1);
+ u64 delayed_refs_extra = btrfs_calc_delayed_ref_bytes(fs_info, 1);
int ret;
/*
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH 21/24] btrfs: fix calculation of the global block reserve's size
2023-03-21 11:13 [PATCH 00/24] btrfs: cleanups and small fixes mostly around block reserves fdmanana
` (19 preceding siblings ...)
2023-03-21 11:13 ` [PATCH 20/24] btrfs: calculate correct amount of space for delayed reference when evicting fdmanana
@ 2023-03-21 11:13 ` fdmanana
2023-03-21 11:13 ` [PATCH 22/24] btrfs: use a constant for the number of metadata units needed for an unlink fdmanana
` (4 subsequent siblings)
25 siblings, 0 replies; 47+ messages in thread
From: fdmanana @ 2023-03-21 11:13 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
At btrfs_update_global_block_rsv(), we are assuming an unlink operation
uses 5 metadata units, but that's not true anymore, it uses 6 since the
commit bca4ad7c0b54 ("btrfs: reserve correct number of items for unlink
and rmdir"). So update the code and comments to consider 6 units.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/block-rsv.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/fs/btrfs/block-rsv.c b/fs/btrfs/block-rsv.c
index 90b8088e8fac..6edcb32ed4c9 100644
--- a/fs/btrfs/block-rsv.c
+++ b/fs/btrfs/block-rsv.c
@@ -350,14 +350,14 @@ void btrfs_update_global_block_rsv(struct btrfs_fs_info *fs_info)
/*
* But we also want to reserve enough space so we can do the fallback
- * global reserve for an unlink, which is an additional 5 items (see the
+ * global reserve for an unlink, which is an additional 6 items (see the
* comment in __unlink_start_trans for what we're modifying.)
*
* But we also need space for the delayed ref updates from the unlink,
- * so its 10, 5 for the actual operation, and 5 for the delayed ref
+ * so its 12, 6 for the actual operation, and 6 for the delayed ref
* updates.
*/
- min_items += 10;
+ min_items += 12;
num_bytes = max_t(u64, num_bytes,
btrfs_calc_insert_metadata_size(fs_info, min_items));
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH 22/24] btrfs: use a constant for the number of metadata units needed for an unlink
2023-03-21 11:13 [PATCH 00/24] btrfs: cleanups and small fixes mostly around block reserves fdmanana
` (20 preceding siblings ...)
2023-03-21 11:13 ` [PATCH 21/24] btrfs: fix calculation of the global block reserve's size fdmanana
@ 2023-03-21 11:13 ` fdmanana
2023-03-21 14:37 ` Anand Jain
2023-03-21 11:13 ` [PATCH 23/24] btrfs: calculate the right space for delayed refs when updating global reserve fdmanana
` (3 subsequent siblings)
25 siblings, 1 reply; 47+ messages in thread
From: fdmanana @ 2023-03-21 11:13 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
Instead of hard coding the number of metadata units for an unlink operation
in a couple places, define a macro and use it instead. This eliminates the
problem of one place getting out of sync with the other, such as recently
fixed by the previous patch in the series ("btrfs: fix calculation of the
global block reserve's size").
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/block-rsv.c | 11 ++++++-----
fs/btrfs/fs.h | 12 ++++++++++++
fs/btrfs/inode.c | 11 ++---------
3 files changed, 20 insertions(+), 14 deletions(-)
diff --git a/fs/btrfs/block-rsv.c b/fs/btrfs/block-rsv.c
index 6edcb32ed4c9..b4a1f1bc340f 100644
--- a/fs/btrfs/block-rsv.c
+++ b/fs/btrfs/block-rsv.c
@@ -350,14 +350,15 @@ void btrfs_update_global_block_rsv(struct btrfs_fs_info *fs_info)
/*
* But we also want to reserve enough space so we can do the fallback
- * global reserve for an unlink, which is an additional 6 items (see the
- * comment in __unlink_start_trans for what we're modifying.)
+ * global reserve for an unlink, which is an additional
+ * BTRFS_UNLINK_METADATA_UNITS items.
*
* But we also need space for the delayed ref updates from the unlink,
- * so its 12, 6 for the actual operation, and 6 for the delayed ref
- * updates.
+ * so it's BTRFS_UNLINK_METADATA_UNITS * 2, BTRFS_UNLINK_METADATA_UNITS
+ * for the actual operation, and BTRFS_UNLINK_METADATA_UNITS more for
+ * the delayed ref updates.
*/
- min_items += 12;
+ min_items += BTRFS_UNLINK_METADATA_UNITS * 2;
num_bytes = max_t(u64, num_bytes,
btrfs_calc_insert_metadata_size(fs_info, min_items));
diff --git a/fs/btrfs/fs.h b/fs/btrfs/fs.h
index 0ce43318ac0e..ca17a7fc3ac3 100644
--- a/fs/btrfs/fs.h
+++ b/fs/btrfs/fs.h
@@ -24,6 +24,18 @@
#define BTRFS_SUPER_INFO_SIZE 4096
static_assert(sizeof(struct btrfs_super_block) == BTRFS_SUPER_INFO_SIZE);
+/*
+ * Number of metadata items necessary for an unlink operation:
+ *
+ * 1 for the possible orphan item
+ * 1 for the dir item
+ * 1 for the dir index
+ * 1 for the inode ref
+ * 1 for the inode
+ * 1 for the parent inode
+ */
+#define BTRFS_UNLINK_METADATA_UNITS 6
+
/*
* The reserved space at the beginning of each device. It covers the primary
* super block and leaves space for potential use by other tools like
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 2e181a0a6f37..0b3710d47dd0 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -4248,15 +4248,8 @@ static struct btrfs_trans_handle *__unlink_start_trans(struct btrfs_inode *dir)
{
struct btrfs_root *root = dir->root;
- /*
- * 1 for the possible orphan item
- * 1 for the dir item
- * 1 for the dir index
- * 1 for the inode ref
- * 1 for the inode
- * 1 for the parent inode
- */
- return btrfs_start_transaction_fallback_global_rsv(root, 6);
+ return btrfs_start_transaction_fallback_global_rsv(root,
+ BTRFS_UNLINK_METADATA_UNITS);
}
static int btrfs_unlink(struct inode *dir, struct dentry *dentry)
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH 23/24] btrfs: calculate the right space for delayed refs when updating global reserve
2023-03-21 11:13 [PATCH 00/24] btrfs: cleanups and small fixes mostly around block reserves fdmanana
` (21 preceding siblings ...)
2023-03-21 11:13 ` [PATCH 22/24] btrfs: use a constant for the number of metadata units needed for an unlink fdmanana
@ 2023-03-21 11:13 ` fdmanana
2023-03-21 11:14 ` [PATCH 24/24] btrfs: simplify exit paths of btrfs_evict_inode() fdmanana
` (2 subsequent siblings)
25 siblings, 0 replies; 47+ messages in thread
From: fdmanana @ 2023-03-21 11:13 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
When updating the global block reserve, we account for the 6 items needed
by an unlink operation and the 6 delayed references for each one of those
items. However the calculation for the delayed references is not correct
in case we have the free space tree enabled, as in that case we need to
touch the free space tree as well and therefore need twice the number of
bytes. So use the btrfs_calc_delayed_ref_bytes() helper to calculate the
number of bytes need for the delayed references at
btrfs_update_global_block_rsv().
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/block-rsv.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/fs/btrfs/block-rsv.c b/fs/btrfs/block-rsv.c
index b4a1f1bc340f..3ab707e26fa2 100644
--- a/fs/btrfs/block-rsv.c
+++ b/fs/btrfs/block-rsv.c
@@ -354,14 +354,15 @@ void btrfs_update_global_block_rsv(struct btrfs_fs_info *fs_info)
* BTRFS_UNLINK_METADATA_UNITS items.
*
* But we also need space for the delayed ref updates from the unlink,
- * so it's BTRFS_UNLINK_METADATA_UNITS * 2, BTRFS_UNLINK_METADATA_UNITS
- * for the actual operation, and BTRFS_UNLINK_METADATA_UNITS more for
- * the delayed ref updates.
+ * so add BTRFS_UNLINK_METADATA_UNITS units for delayed refs, one for
+ * each unlink metadata item.
*/
- min_items += BTRFS_UNLINK_METADATA_UNITS * 2;
+ min_items += BTRFS_UNLINK_METADATA_UNITS;
num_bytes = max_t(u64, num_bytes,
- btrfs_calc_insert_metadata_size(fs_info, min_items));
+ btrfs_calc_insert_metadata_size(fs_info, min_items) +
+ btrfs_calc_delayed_ref_bytes(fs_info,
+ BTRFS_UNLINK_METADATA_UNITS));
spin_lock(&sinfo->lock);
spin_lock(&block_rsv->lock);
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* [PATCH 24/24] btrfs: simplify exit paths of btrfs_evict_inode()
2023-03-21 11:13 [PATCH 00/24] btrfs: cleanups and small fixes mostly around block reserves fdmanana
` (22 preceding siblings ...)
2023-03-21 11:13 ` [PATCH 23/24] btrfs: calculate the right space for delayed refs when updating global reserve fdmanana
@ 2023-03-21 11:14 ` fdmanana
2023-03-22 14:37 ` [PATCH 00/24] btrfs: cleanups and small fixes mostly around block reserves Josef Bacik
2023-03-23 19:35 ` David Sterba
25 siblings, 0 replies; 47+ messages in thread
From: fdmanana @ 2023-03-21 11:14 UTC (permalink / raw)
To: linux-btrfs
From: Filipe Manana <fdmanana@suse.com>
Instead of using two labels at btrfs_evict_inode() for exiting depending
on whether we need to delete the inode items and orphan or some error
happened, we can use a single exit label if we initialize the block
reserve to NULL, since btrfs_free_block_rsv() ignores a NULL block reserve
pointer. So just do that. It will also make an upcoming change simpler by
avoiding one extra error label.
Signed-off-by: Filipe Manana <fdmanana@suse.com>
---
fs/btrfs/inode.c | 21 ++++++++++-----------
1 file changed, 10 insertions(+), 11 deletions(-)
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 0b3710d47dd0..865d56ff2ce1 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -5271,7 +5271,7 @@ void btrfs_evict_inode(struct inode *inode)
struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
struct btrfs_trans_handle *trans;
struct btrfs_root *root = BTRFS_I(inode)->root;
- struct btrfs_block_rsv *rsv;
+ struct btrfs_block_rsv *rsv = NULL;
int ret;
trace_btrfs_inode_evict(inode);
@@ -5288,18 +5288,18 @@ void btrfs_evict_inode(struct inode *inode)
((btrfs_root_refs(&root->root_item) != 0 &&
root->root_key.objectid != BTRFS_ROOT_TREE_OBJECTID) ||
btrfs_is_free_space_inode(BTRFS_I(inode))))
- goto no_delete;
+ goto out;
if (is_bad_inode(inode))
- goto no_delete;
+ goto out;
if (test_bit(BTRFS_FS_LOG_RECOVERING, &fs_info->flags))
- goto no_delete;
+ goto out;
if (inode->i_nlink > 0) {
BUG_ON(btrfs_root_refs(&root->root_item) != 0 &&
root->root_key.objectid != BTRFS_ROOT_TREE_OBJECTID);
- goto no_delete;
+ goto out;
}
/*
@@ -5308,7 +5308,7 @@ void btrfs_evict_inode(struct inode *inode)
*/
ret = btrfs_commit_inode_delayed_inode(BTRFS_I(inode));
if (ret)
- goto no_delete;
+ goto out;
/*
* This drops any pending insert or delete operations we have for this
@@ -5320,7 +5320,7 @@ void btrfs_evict_inode(struct inode *inode)
rsv = btrfs_alloc_block_rsv(fs_info, BTRFS_BLOCK_RSV_TEMP);
if (!rsv)
- goto no_delete;
+ goto out;
rsv->size = btrfs_calc_metadata_size(fs_info, 1);
rsv->failfast = true;
@@ -5336,7 +5336,7 @@ void btrfs_evict_inode(struct inode *inode)
trans = evict_refill_and_join(root, rsv);
if (IS_ERR(trans))
- goto free_rsv;
+ goto out;
trans->block_rsv = rsv;
@@ -5350,7 +5350,7 @@ void btrfs_evict_inode(struct inode *inode)
*/
btrfs_btree_balance_dirty_nodelay(fs_info);
if (ret && ret != -ENOSPC && ret != -EAGAIN)
- goto free_rsv;
+ goto out;
else if (!ret)
break;
}
@@ -5372,9 +5372,8 @@ void btrfs_evict_inode(struct inode *inode)
btrfs_end_transaction(trans);
}
-free_rsv:
+out:
btrfs_free_block_rsv(fs_info, rsv);
-no_delete:
/*
* If we didn't successfully delete, the orphan item will still be in
* the tree and we'll retry on the next mount. Again, we might also want
--
2.34.1
^ permalink raw reply related [flat|nested] 47+ messages in thread* Re: [PATCH 00/24] btrfs: cleanups and small fixes mostly around block reserves
2023-03-21 11:13 [PATCH 00/24] btrfs: cleanups and small fixes mostly around block reserves fdmanana
` (23 preceding siblings ...)
2023-03-21 11:14 ` [PATCH 24/24] btrfs: simplify exit paths of btrfs_evict_inode() fdmanana
@ 2023-03-22 14:37 ` Josef Bacik
2023-03-23 19:35 ` David Sterba
25 siblings, 0 replies; 47+ messages in thread
From: Josef Bacik @ 2023-03-22 14:37 UTC (permalink / raw)
To: fdmanana; +Cc: linux-btrfs
On Tue, Mar 21, 2023 at 11:13:36AM +0000, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
>
> A set of cleanups and small fixes that started as part of a larger work,
> mostly around block reserves and space reservation, but as they are mostly
> trivial and independent of the rest of that work, I'm sending them out
> separately. More details on the individual changelogs.
>
> Filipe Manana (24):
> btrfs: pass a bool to btrfs_block_rsv_migrate() at evict_refill_and_join()
> btrfs: pass a bool size update argument to btrfs_block_rsv_add_bytes()
> btrfs: remove check for NULL block reserve at btrfs_block_rsv_check()
> btrfs: update documentation for BTRFS_RESERVE_FLUSH_EVICT flush method
> btrfs: update flush method assertion when reserving space
> btrfs: initialize ret to -ENOSPC at __reserve_bytes()
> btrfs: simplify btrfs_should_throttle_delayed_refs()
> btrfs: collapse should_end_transaction() into btrfs_should_end_transaction()
> btrfs: remove bytes_used argument from btrfs_make_block_group()
> btrfs: count extents before taking inode's spinlock when reserving metadata
> btrfs: remove redundant counter check at btrfs_truncate_inode_items()
> btrfs: simplify btrfs_block_rsv_refill()
> btrfs: remove obsolete delayed ref throttling logic when truncating items
> btrfs: don't throttle on delayed items when evicting deleted inode
> btrfs: calculate the right space for a single delayed ref when refilling
> btrfs: accurately calculate number of delayed refs when flushing
> btrfs: constify fs_info argument of the metadata size calculation helpers
> btrfs: constify fs_info argument for the reclaim items calculation helpers
> btrfs: add helper to calculate space for delayed references
> btrfs: calculate correct amount of space for delayed reference when evicting
> btrfs: fix calculation of the global block reserve's size
> btrfs: use a constant for the number of metadata units needed for an unlink
> btrfs: calculate the right space for delayed refs when updating global reserve
> btrfs: simplify exit paths of btrfs_evict_inode()
>
> fs/btrfs/block-group.c | 7 ++----
> fs/btrfs/block-group.h | 2 +-
> fs/btrfs/block-rsv.c | 21 +++++++----------
> fs/btrfs/block-rsv.h | 2 +-
> fs/btrfs/delalloc-space.c | 2 +-
> fs/btrfs/delayed-ref.c | 49 ++++-----------------------------------
> fs/btrfs/delayed-ref.h | 22 +++++++++++++++++-
> fs/btrfs/disk-io.c | 1 -
> fs/btrfs/extent-tree.c | 27 ++-------------------
> fs/btrfs/fs.h | 17 +++++++++++---
> fs/btrfs/inode-item.c | 15 +++++-------
> fs/btrfs/inode.c | 43 ++++++++++++++++------------------
> fs/btrfs/space-info.c | 32 +++++++++++++++++++++----
> fs/btrfs/space-info.h | 1 +
> fs/btrfs/transaction.c | 15 ++++--------
> fs/btrfs/volumes.c | 2 +-
> 16 files changed, 115 insertions(+), 143 deletions(-)
I reviewed this last night but I don't see my reply, so apologies if this is the
second email. You can add
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
to this series, thanks,
Josef
^ permalink raw reply [flat|nested] 47+ messages in thread* Re: [PATCH 00/24] btrfs: cleanups and small fixes mostly around block reserves
2023-03-21 11:13 [PATCH 00/24] btrfs: cleanups and small fixes mostly around block reserves fdmanana
` (24 preceding siblings ...)
2023-03-22 14:37 ` [PATCH 00/24] btrfs: cleanups and small fixes mostly around block reserves Josef Bacik
@ 2023-03-23 19:35 ` David Sterba
25 siblings, 0 replies; 47+ messages in thread
From: David Sterba @ 2023-03-23 19:35 UTC (permalink / raw)
To: fdmanana; +Cc: linux-btrfs
On Tue, Mar 21, 2023 at 11:13:36AM +0000, fdmanana@kernel.org wrote:
> From: Filipe Manana <fdmanana@suse.com>
>
> A set of cleanups and small fixes that started as part of a larger work,
> mostly around block reserves and space reservation, but as they are mostly
> trivial and independent of the rest of that work, I'm sending them out
> separately. More details on the individual changelogs.
>
> Filipe Manana (24):
> btrfs: pass a bool to btrfs_block_rsv_migrate() at evict_refill_and_join()
> btrfs: pass a bool size update argument to btrfs_block_rsv_add_bytes()
> btrfs: remove check for NULL block reserve at btrfs_block_rsv_check()
> btrfs: update documentation for BTRFS_RESERVE_FLUSH_EVICT flush method
> btrfs: update flush method assertion when reserving space
> btrfs: initialize ret to -ENOSPC at __reserve_bytes()
> btrfs: simplify btrfs_should_throttle_delayed_refs()
> btrfs: collapse should_end_transaction() into btrfs_should_end_transaction()
> btrfs: remove bytes_used argument from btrfs_make_block_group()
> btrfs: count extents before taking inode's spinlock when reserving metadata
> btrfs: remove redundant counter check at btrfs_truncate_inode_items()
> btrfs: simplify btrfs_block_rsv_refill()
> btrfs: remove obsolete delayed ref throttling logic when truncating items
> btrfs: don't throttle on delayed items when evicting deleted inode
> btrfs: calculate the right space for a single delayed ref when refilling
> btrfs: accurately calculate number of delayed refs when flushing
> btrfs: constify fs_info argument of the metadata size calculation helpers
> btrfs: constify fs_info argument for the reclaim items calculation helpers
> btrfs: add helper to calculate space for delayed references
> btrfs: calculate correct amount of space for delayed reference when evicting
> btrfs: fix calculation of the global block reserve's size
> btrfs: use a constant for the number of metadata units needed for an unlink
> btrfs: calculate the right space for delayed refs when updating global reserve
> btrfs: simplify exit paths of btrfs_evict_inode()
Added to misc-next, thanks.
^ permalink raw reply [flat|nested] 47+ messages in thread