* [PATCH v2 1/2] btrfs: print the block rsv type when we fail our reservation
2020-10-26 20:57 [PATCH v2 0/2] Some block rsv fixes Josef Bacik
@ 2020-10-26 20:57 ` Josef Bacik
2020-10-27 8:55 ` Nikolay Borisov
2020-10-26 20:57 ` [PATCH v2 2/2] btrfs: fix min reserved size calculation in merge_reloc_root Josef Bacik
2020-10-29 16:30 ` [PATCH v2 0/2] Some block rsv fixes David Sterba
2 siblings, 1 reply; 6+ messages in thread
From: Josef Bacik @ 2020-10-26 20:57 UTC (permalink / raw)
To: linux-btrfs, kernel-team
To help with debugging, print the type of the block rsv when we fail to
use our target block rsv in btrfs_use_block_rsv.
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
fs/btrfs/block-rsv.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/block-rsv.c b/fs/btrfs/block-rsv.c
index 4651f8bf890b..04a6226e0388 100644
--- a/fs/btrfs/block-rsv.c
+++ b/fs/btrfs/block-rsv.c
@@ -519,7 +519,8 @@ struct btrfs_block_rsv *btrfs_use_block_rsv(struct btrfs_trans_handle *trans,
/*DEFAULT_RATELIMIT_BURST*/ 1);
if (__ratelimit(&_rs))
WARN(1, KERN_DEBUG
- "BTRFS: block rsv returned %d\n", ret);
+ "BTRFS: block rsv %d returned %d\n",
+ block_rsv->type, ret);
}
try_reserve:
ret = btrfs_reserve_metadata_bytes(root, block_rsv, blocksize,
--
2.26.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] btrfs: print the block rsv type when we fail our reservation
2020-10-26 20:57 ` [PATCH v2 1/2] btrfs: print the block rsv type when we fail our reservation Josef Bacik
@ 2020-10-27 8:55 ` Nikolay Borisov
0 siblings, 0 replies; 6+ messages in thread
From: Nikolay Borisov @ 2020-10-27 8:55 UTC (permalink / raw)
To: Josef Bacik, linux-btrfs, kernel-team
On 26.10.20 г. 22:57 ч., Josef Bacik wrote:
> To help with debugging, print the type of the block rsv when we fail to
> use our target block rsv in btrfs_use_block_rsv.
>
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Yeah, this now produces:
[ 544.672035] BTRFS: block rsv 1 returned -28
which is still cryptic without consulting the enum in block-rsv.h but I
guess it's better than nothing.
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
> ---
> fs/btrfs/block-rsv.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/block-rsv.c b/fs/btrfs/block-rsv.c
> index 4651f8bf890b..04a6226e0388 100644
> --- a/fs/btrfs/block-rsv.c
> +++ b/fs/btrfs/block-rsv.c
> @@ -519,7 +519,8 @@ struct btrfs_block_rsv *btrfs_use_block_rsv(struct btrfs_trans_handle *trans,
> /*DEFAULT_RATELIMIT_BURST*/ 1);
> if (__ratelimit(&_rs))
> WARN(1, KERN_DEBUG
> - "BTRFS: block rsv returned %d\n", ret);
> + "BTRFS: block rsv %d returned %d\n",
> + block_rsv->type, ret);
> }
> try_reserve:
> ret = btrfs_reserve_metadata_bytes(root, block_rsv, blocksize,
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 2/2] btrfs: fix min reserved size calculation in merge_reloc_root
2020-10-26 20:57 [PATCH v2 0/2] Some block rsv fixes Josef Bacik
2020-10-26 20:57 ` [PATCH v2 1/2] btrfs: print the block rsv type when we fail our reservation Josef Bacik
@ 2020-10-26 20:57 ` Josef Bacik
2020-10-27 8:53 ` Nikolay Borisov
2020-10-29 16:30 ` [PATCH v2 0/2] Some block rsv fixes David Sterba
2 siblings, 1 reply; 6+ messages in thread
From: Josef Bacik @ 2020-10-26 20:57 UTC (permalink / raw)
To: linux-btrfs, kernel-team
The minimum reserve size was adjusted to take into account the height of
the tree we are merging, however we can have a root with a level == 0.
What we want is root_level + 1 to get the number of nodes we may have to
cow. This fixes the enospc_debug warning pops with btrfs/101.
44d354abf33e ("btrfs: relocation: review the call sites which can be interrupted by signal")
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
fs/btrfs/relocation.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
index 3602806d71bd..9ba92d86da0b 100644
--- a/fs/btrfs/relocation.c
+++ b/fs/btrfs/relocation.c
@@ -1648,6 +1648,7 @@ static noinline_for_stack int merge_reloc_root(struct reloc_control *rc,
struct btrfs_root_item *root_item;
struct btrfs_path *path;
struct extent_buffer *leaf;
+ int reserve_level;
int level;
int max_level;
int replaced = 0;
@@ -1696,7 +1697,8 @@ static noinline_for_stack int merge_reloc_root(struct reloc_control *rc,
* Thus the needed metadata size is at most root_level * nodesize,
* and * 2 since we have two trees to COW.
*/
- min_reserved = fs_info->nodesize * btrfs_root_level(root_item) * 2;
+ reserve_level = max_t(int, 1, btrfs_root_level(root_item));
+ min_reserved = fs_info->nodesize * reserve_level * 2;
memset(&next_key, 0, sizeof(next_key));
while (1) {
--
2.26.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2 2/2] btrfs: fix min reserved size calculation in merge_reloc_root
2020-10-26 20:57 ` [PATCH v2 2/2] btrfs: fix min reserved size calculation in merge_reloc_root Josef Bacik
@ 2020-10-27 8:53 ` Nikolay Borisov
0 siblings, 0 replies; 6+ messages in thread
From: Nikolay Borisov @ 2020-10-27 8:53 UTC (permalink / raw)
To: Josef Bacik, linux-btrfs, kernel-team
On 26.10.20 г. 22:57 ч., Josef Bacik wrote:
> The minimum reserve size was adjusted to take into account the height of
> the tree we are merging, however we can have a root with a level == 0.
> What we want is root_level + 1 to get the number of nodes we may have to
> cow. This fixes the enospc_debug warning pops with btrfs/101.
>
> 44d354abf33e ("btrfs: relocation: review the call sites which can be interrupted by signal")
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
FWIW this fixes failures on btrfs/060 btrfs/062 btrfs/063 and btrfs/195 That I was seeing, the call trace was:
[ 3680.515564] ------------[ cut here ]------------
[ 3680.515566] BTRFS: block rsv returned -28
[ 3680.515585] WARNING: CPU: 2 PID: 8339 at fs/btrfs/block-rsv.c:521 btrfs_use_block_rsv+0x162/0x180
[ 3680.515587] Modules linked in:
[ 3680.515591] CPU: 2 PID: 8339 Comm: btrfs Tainted: G W 5.9.0-rc8-default #95
[ 3680.515593] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.13.0-1ubuntu1 04/01/2014
[ 3680.515595] RIP: 0010:btrfs_use_block_rsv+0x162/0x180
[ 3680.515600] RSP: 0018:ffffa01ac9753910 EFLAGS: 00010282
[ 3680.515602] RAX: 0000000000000000 RBX: ffff984b34200000 RCX: 0000000000000027
[ 3680.515604] RDX: 0000000000000027 RSI: 0000000000000000 RDI: ffff984b3bd19e28
[ 3680.515606] RBP: 0000000000004000 R08: ffff984b3bd19e20 R09: 0000000000000001
[ 3680.515608] R10: 0000000000000004 R11: 0000000000000046 R12: ffff984b264fdc00
[ 3680.515609] R13: ffff984b13149000 R14: 00000000ffffffe4 R15: ffff984b34200000
[ 3680.515613] FS: 00007f4e2912b8c0(0000) GS:ffff984b3bd00000(0000) knlGS:0000000000000000
[ 3680.515615] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 3680.515617] CR2: 00007fab87122150 CR3: 0000000118e42000 CR4: 00000000000006e0
[ 3680.515620] Call Trace:
[ 3680.515627] btrfs_alloc_tree_block+0x8b/0x340
[ 3680.515633] ? __lock_acquire+0x51a/0xac0
[ 3680.515646] alloc_tree_block_no_bg_flush+0x4f/0x60
[ 3680.515651] __btrfs_cow_block+0x14e/0x7e0
[ 3680.515662] btrfs_cow_block+0x144/0x2c0
[ 3680.515670] merge_reloc_root+0x4d4/0x610
[ 3680.515675] ? btrfs_lookup_fs_root+0x78/0x90
[ 3680.515686] merge_reloc_roots+0xee/0x280
[ 3680.515695] relocate_block_group+0x2ce/0x5e0
[ 3680.515704] btrfs_relocate_block_group+0x16e/0x310
[ 3680.515711] btrfs_relocate_chunk+0x38/0xf0
[ 3680.515716] btrfs_shrink_device+0x200/0x560
[ 3680.515728] btrfs_rm_device+0x1ae/0x6a6
[ 3680.515744] ? _copy_from_user+0x6e/0xb0
[ 3680.515750] btrfs_ioctl+0x1afe/0x28c0
[ 3680.515755] ? find_held_lock+0x2b/0x80
[ 3680.515760] ? do_user_addr_fault+0x1f8/0x418
[ 3680.515773] ? __x64_sys_ioctl+0x77/0xb0
[ 3680.515775] __x64_sys_ioctl+0x77/0xb0
[ 3680.515781] do_syscall_64+0x31/0x70
[ 3680.515785] entry_SYSCALL_64_after_hwframe+0x44/0xa9
Reviewed-by: Nikolay Borisov <nborisov@suse.com>
Tested-by: Nikolay Borisov <nborisov@suse.com>
> ---
> fs/btrfs/relocation.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/fs/btrfs/relocation.c b/fs/btrfs/relocation.c
> index 3602806d71bd..9ba92d86da0b 100644
> --- a/fs/btrfs/relocation.c
> +++ b/fs/btrfs/relocation.c
> @@ -1648,6 +1648,7 @@ static noinline_for_stack int merge_reloc_root(struct reloc_control *rc,
> struct btrfs_root_item *root_item;
> struct btrfs_path *path;
> struct extent_buffer *leaf;
> + int reserve_level;
> int level;
> int max_level;
> int replaced = 0;
> @@ -1696,7 +1697,8 @@ static noinline_for_stack int merge_reloc_root(struct reloc_control *rc,
> * Thus the needed metadata size is at most root_level * nodesize,
> * and * 2 since we have two trees to COW.
> */
> - min_reserved = fs_info->nodesize * btrfs_root_level(root_item) * 2;
> + reserve_level = max_t(int, 1, btrfs_root_level(root_item));
> + min_reserved = fs_info->nodesize * reserve_level * 2;
> memset(&next_key, 0, sizeof(next_key));
>
> while (1) {
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 0/2] Some block rsv fixes
2020-10-26 20:57 [PATCH v2 0/2] Some block rsv fixes Josef Bacik
2020-10-26 20:57 ` [PATCH v2 1/2] btrfs: print the block rsv type when we fail our reservation Josef Bacik
2020-10-26 20:57 ` [PATCH v2 2/2] btrfs: fix min reserved size calculation in merge_reloc_root Josef Bacik
@ 2020-10-29 16:30 ` David Sterba
2 siblings, 0 replies; 6+ messages in thread
From: David Sterba @ 2020-10-29 16:30 UTC (permalink / raw)
To: Josef Bacik; +Cc: linux-btrfs, kernel-team
On Mon, Oct 26, 2020 at 04:57:25PM -0400, Josef Bacik wrote:
> v1->v2:
> - change the logic to max(1, root level), as generally we do not walk down into
> level 0 for the merge, with the exception for root level == 0.
>
> --- Original email ---
>
> Hello,
>
> Nikolay has noticed that -o enospc_debug was getting some warnings in
> btrfs_use_block_rsv() on some tests. I dug into them and one class is easy to
> fix as it's a straight regression. The other one is going to require some more
> debugging, so in the meantime here's the two patches I have so far that can be
> merged. The first is just to make my life easier when debugging these problems,
> and the second is the actual regression fix. It should probably be tagged for
> stable as well since the regression was backported to stable. Thanks,
Yeah, for stable 5.4+.
> Josef
>
> Josef Bacik (2):
> btrfs: print the block rsv type when we fail our reservation
> btrfs: fix min reserved size calculation in merge_reloc_root
Added to misc-next, thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread