All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/2] Some block rsv fixes
@ 2020-10-26 20:57 Josef Bacik
  2020-10-26 20:57 ` [PATCH v2 1/2] btrfs: print the block rsv type when we fail our reservation Josef Bacik
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Josef Bacik @ 2020-10-26 20:57 UTC (permalink / raw)
  To: linux-btrfs, kernel-team

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,

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

 fs/btrfs/block-rsv.c  | 3 ++-
 fs/btrfs/relocation.c | 4 +++-
 2 files changed, 5 insertions(+), 2 deletions(-)

-- 
2.26.2


^ permalink raw reply	[flat|nested] 6+ messages in thread
* [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
* [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 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
* 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
end of thread, other threads:[~2020-10-29 16:34 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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-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-27  8:53   ` Nikolay Borisov
2020-10-29 16:30 ` [PATCH v2 0/2] Some block rsv fixes David Sterba

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.