* [PATCH 1/2] btrfs: kill update_block_group_flags
@ 2020-06-30 18:17 Josef Bacik
2020-06-30 18:17 ` [PATCH 2/2] btrfs: if we're restriping, use the target restripe profile Josef Bacik
2020-07-22 16:03 ` [PATCH 1/2] btrfs: kill update_block_group_flags David Sterba
0 siblings, 2 replies; 4+ messages in thread
From: Josef Bacik @ 2020-06-30 18:17 UTC (permalink / raw)
To: linux-btrfs, kernel-team, holger
btrfs/061 has been failing consistently for me recently with a
transaction abort. We run out of space in the system chunk array, which
means we've allocated way too many system chunks than we need.
Chris added this a long time ago for balance as a poor mans restriping.
If you had a single disk and then added another disk and then did a
balance, update_block_group_flags would then figure out which RAID level
you needed.
Fast forward to today and we have restriping behavior, so we can
explicitly tell the fs that we're trying to change the raid level. This
is accomplished through the normal get_alloc_profile path.
Furthermore this code actually causes btrfs/061 to fail, because we do
things like mkfs -m dup -d single with multiple devices. This trips
this check
alloc_flags = update_block_group_flags(fs_info, cache->flags);
if (alloc_flags != cache->flags) {
ret = btrfs_chunk_alloc(trans, alloc_flags, CHUNK_ALLOC_FORCE);
in btrfs_inc_block_group_ro. Because we're balancing and scrubbing, but
not actually restriping, we keep forcing chunk allocation of RAID1
chunks. This eventually causes us to run out of system space and the
file system aborts and flips read only.
We don't need this poor mans restriping any more, simply use the normal
get_alloc_profile helper, which will get the correct alloc_flags and
thus make the right decision for chunk allocation. This keeps us from
allocating a billion system chunks and falling over.
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
fs/btrfs/block-group.c | 52 ++----------------------------------------
1 file changed, 2 insertions(+), 50 deletions(-)
diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index 09b796a081dd..b111885482e5 100644
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c
@@ -2242,54 +2242,6 @@ int btrfs_make_block_group(struct btrfs_trans_handle *trans, u64 bytes_used,
return 0;
}
-static u64 update_block_group_flags(struct btrfs_fs_info *fs_info, u64 flags)
-{
- u64 num_devices;
- u64 stripped;
-
- /*
- * if restripe for this chunk_type is on pick target profile and
- * return, otherwise do the usual balance
- */
- stripped = get_restripe_target(fs_info, flags);
- if (stripped)
- return extended_to_chunk(stripped);
-
- num_devices = fs_info->fs_devices->rw_devices;
-
- stripped = BTRFS_BLOCK_GROUP_RAID0 | BTRFS_BLOCK_GROUP_RAID56_MASK |
- BTRFS_BLOCK_GROUP_RAID1_MASK | BTRFS_BLOCK_GROUP_RAID10;
-
- if (num_devices == 1) {
- stripped |= BTRFS_BLOCK_GROUP_DUP;
- stripped = flags & ~stripped;
-
- /* turn raid0 into single device chunks */
- if (flags & BTRFS_BLOCK_GROUP_RAID0)
- return stripped;
-
- /* turn mirroring into duplication */
- if (flags & (BTRFS_BLOCK_GROUP_RAID1_MASK |
- BTRFS_BLOCK_GROUP_RAID10))
- return stripped | BTRFS_BLOCK_GROUP_DUP;
- } else {
- /* they already had raid on here, just return */
- if (flags & stripped)
- return flags;
-
- stripped |= BTRFS_BLOCK_GROUP_DUP;
- stripped = flags & ~stripped;
-
- /* switch duplicated blocks with raid1 */
- if (flags & BTRFS_BLOCK_GROUP_DUP)
- return stripped | BTRFS_BLOCK_GROUP_RAID1;
-
- /* this is drive concat, leave it alone */
- }
-
- return flags;
-}
-
/*
* Mark one block group RO, can be called several times for the same block
* group.
@@ -2335,7 +2287,7 @@ int btrfs_inc_block_group_ro(struct btrfs_block_group *cache,
* If we are changing raid levels, try to allocate a
* corresponding block group with the new raid level.
*/
- alloc_flags = update_block_group_flags(fs_info, cache->flags);
+ alloc_flags = btrfs_get_alloc_profile(fs_info, cache->flags);
if (alloc_flags != cache->flags) {
ret = btrfs_chunk_alloc(trans, alloc_flags,
CHUNK_ALLOC_FORCE);
@@ -2362,7 +2314,7 @@ int btrfs_inc_block_group_ro(struct btrfs_block_group *cache,
ret = inc_block_group_ro(cache, 0);
out:
if (cache->flags & BTRFS_BLOCK_GROUP_SYSTEM) {
- alloc_flags = update_block_group_flags(fs_info, cache->flags);
+ alloc_flags = btrfs_get_alloc_profile(fs_info, cache->flags);
mutex_lock(&fs_info->chunk_mutex);
check_system_chunk(trans, alloc_flags);
mutex_unlock(&fs_info->chunk_mutex);
--
2.24.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH 2/2] btrfs: if we're restriping, use the target restripe profile
2020-06-30 18:17 [PATCH 1/2] btrfs: kill update_block_group_flags Josef Bacik
@ 2020-06-30 18:17 ` Josef Bacik
2020-07-22 16:03 ` [PATCH 1/2] btrfs: kill update_block_group_flags David Sterba
1 sibling, 0 replies; 4+ messages in thread
From: Josef Bacik @ 2020-06-30 18:17 UTC (permalink / raw)
To: linux-btrfs, kernel-team, holger
Previously we depended on some weird behavior in our chunk allocator to
force the allocation of new stripes, so by the time we got to doing the
reduce we would usually already have a chunk with the proper target.
However that behavior causes other problems and needs to be removed.
First however we need to remove this check to only restripe if we
already have those available profiles, because if we're allocating our
first chunk it obviously will not be available. Simply use the target
as specified, and if that fails it'll be because we're out of space.
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
fs/btrfs/block-group.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index b111885482e5..1907ab9a093a 100644
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c
@@ -65,11 +65,8 @@ static u64 btrfs_reduce_alloc_profile(struct btrfs_fs_info *fs_info, u64 flags)
spin_lock(&fs_info->balance_lock);
target = get_restripe_target(fs_info, flags);
if (target) {
- /* Pick target profile only if it's already available */
- if ((flags & target) & BTRFS_EXTENDED_PROFILE_MASK) {
- spin_unlock(&fs_info->balance_lock);
- return extended_to_chunk(target);
- }
+ spin_unlock(&fs_info->balance_lock);
+ return extended_to_chunk(target);
}
spin_unlock(&fs_info->balance_lock);
--
2.24.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH 1/2] btrfs: kill update_block_group_flags
2020-06-30 18:17 [PATCH 1/2] btrfs: kill update_block_group_flags Josef Bacik
2020-06-30 18:17 ` [PATCH 2/2] btrfs: if we're restriping, use the target restripe profile Josef Bacik
@ 2020-07-22 16:03 ` David Sterba
1 sibling, 0 replies; 4+ messages in thread
From: David Sterba @ 2020-07-22 16:03 UTC (permalink / raw)
To: Josef Bacik; +Cc: linux-btrfs, kernel-team, holger
On Tue, Jun 30, 2020 at 02:17:18PM -0400, Josef Bacik wrote:
> btrfs/061 has been failing consistently for me recently with a
> transaction abort. We run out of space in the system chunk array, which
> means we've allocated way too many system chunks than we need.
>
> Chris added this a long time ago for balance as a poor mans restriping.
> If you had a single disk and then added another disk and then did a
> balance, update_block_group_flags would then figure out which RAID level
> you needed.
>
> Fast forward to today and we have restriping behavior, so we can
> explicitly tell the fs that we're trying to change the raid level. This
> is accomplished through the normal get_alloc_profile path.
>
> Furthermore this code actually causes btrfs/061 to fail, because we do
> things like mkfs -m dup -d single with multiple devices. This trips
> this check
>
> alloc_flags = update_block_group_flags(fs_info, cache->flags);
> if (alloc_flags != cache->flags) {
> ret = btrfs_chunk_alloc(trans, alloc_flags, CHUNK_ALLOC_FORCE);
>
> in btrfs_inc_block_group_ro. Because we're balancing and scrubbing, but
> not actually restriping, we keep forcing chunk allocation of RAID1
> chunks. This eventually causes us to run out of system space and the
> file system aborts and flips read only.
>
> We don't need this poor mans restriping any more, simply use the normal
> get_alloc_profile helper, which will get the correct alloc_flags and
> thus make the right decision for chunk allocation. This keeps us from
> allocating a billion system chunks and falling over.
>
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
1 and 2 added to misc-next. I haven't found time to verify the raid1c34
case, but Holger reported it had fixed some problems for him I take
that as testing.
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 0/2][RESEND] Fix how we do block group flags
@ 2020-07-21 14:48 Josef Bacik
2020-07-21 14:48 ` [PATCH 2/2] btrfs: if we're restriping, use the target restripe profile Josef Bacik
0 siblings, 1 reply; 4+ messages in thread
From: Josef Bacik @ 2020-07-21 14:48 UTC (permalink / raw)
To: linux-btrfs, kernel-team
These two patches address some wonkiness with how we do block group flags.
We've had long standing failures here, this makes things more consistent and
fixes some weird corner cases where we end up with lots of chunks we cannot use.
Thanks,
Josef
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 2/2] btrfs: if we're restriping, use the target restripe profile
2020-07-21 14:48 [PATCH 0/2][RESEND] Fix how we do block group flags Josef Bacik
@ 2020-07-21 14:48 ` Josef Bacik
0 siblings, 0 replies; 4+ messages in thread
From: Josef Bacik @ 2020-07-21 14:48 UTC (permalink / raw)
To: linux-btrfs, kernel-team
Previously we depended on some weird behavior in our chunk allocator to
force the allocation of new stripes, so by the time we got to doing the
reduce we would usually already have a chunk with the proper target.
However that behavior causes other problems and needs to be removed.
First however we need to remove this check to only restripe if we
already have those available profiles, because if we're allocating our
first chunk it obviously will not be available. Simply use the target
as specified, and if that fails it'll be because we're out of space.
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
fs/btrfs/block-group.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/fs/btrfs/block-group.c b/fs/btrfs/block-group.c
index 652b35d5a773..613920c17ac1 100644
--- a/fs/btrfs/block-group.c
+++ b/fs/btrfs/block-group.c
@@ -65,11 +65,8 @@ static u64 btrfs_reduce_alloc_profile(struct btrfs_fs_info *fs_info, u64 flags)
spin_lock(&fs_info->balance_lock);
target = get_restripe_target(fs_info, flags);
if (target) {
- /* Pick target profile only if it's already available */
- if ((flags & target) & BTRFS_EXTENDED_PROFILE_MASK) {
- spin_unlock(&fs_info->balance_lock);
- return extended_to_chunk(target);
- }
+ spin_unlock(&fs_info->balance_lock);
+ return extended_to_chunk(target);
}
spin_unlock(&fs_info->balance_lock);
--
2.24.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-07-22 16:03 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-30 18:17 [PATCH 1/2] btrfs: kill update_block_group_flags Josef Bacik
2020-06-30 18:17 ` [PATCH 2/2] btrfs: if we're restriping, use the target restripe profile Josef Bacik
2020-07-22 16:03 ` [PATCH 1/2] btrfs: kill update_block_group_flags David Sterba
2020-07-21 14:48 [PATCH 0/2][RESEND] Fix how we do block group flags Josef Bacik
2020-07-21 14:48 ` [PATCH 2/2] btrfs: if we're restriping, use the target restripe profile Josef Bacik
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).