Btrfs: fix regression in raid level conversion
PATCH 1/1 Btrfs: fix regression in raid level conversion
Commit 2f0810880f082fa8ba66ab2c33b02e4ff9770a5e changed btrfs_set_block_group_ro to avoid trying to allocate new chunks with the new raid profile during conversion. This fixed failures when there was no space on the drive to allocate a new chunk, but the metadata reserves were sufficient to continue the conversion.
But this ended up causing a regression when the drive had plenty of space to allocate new chunks, mostly because reduce_alloc_profile isn't using the new raid profile.
Fixing btrfs_reduce_alloc_profile is a bigger patch. For now, do a partial revert of 2f0810880, and don't error out if we hit ENOSPC.
--- fs/btrfs/extent-tree.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)
fs/btrfs/extent-tree.c +20 -0
@@ -8829,6 +8829,26 @@ again: goto again; } +/* +* if we are changing raid levels, try to allocate a corresponding +* block group with the new raid level. +*/ +if (!(cache->flags &BTRFS_BLOCK_GROUP_SYSTEM )) { + alloc_flags =update_block_group_flags (root, cache->flags); +if (alloc_flags != cache->flags) { + ret =do_chunk_alloc (trans, root, alloc_flags, +CHUNK_ALLOC_FORCE ); +/* +* ENOSPC is allowed here, we may have enough space +* already allocated at the new raid level to +* carry on +*/ +if (ret == -ENOSPC ) + ret =0 ; +if (ret <0 ) + goto out; + } + } ret =set_block_group_ro (cache,0 );if (!ret)
Signature · 1 line
2.4.0
Quoted 23 lines · Chris Mason
Commit
2f0810880f082fa8ba66ab2c33b02e4ff9770a5echangedbtrfs_set_block_group_roto avoid trying to allocate new chunks with the new raid profile during conversion. This fixed failures when there was no space on the drive to allocate a new chunk, but the metadata reserves were sufficient to continue the conversion.But this ended up causing a regression when the drive had plenty of space to allocate new chunks, mostly because
reduce_alloc_profileisn't using the new raid profile.Fixing
btrfs_reduce_alloc_profileis a bigger patch. For now, do a partial revert of2f0810880, and don't error out if we hit ENOSPC.Signed-off-by Chris Mason <[email protected]>--- fs/btrfs/extent-tree.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)fs/btrfs/extent-tree.c +0
-0
fs/btrfs/extent-tree.c again: · line 8836
Quoted 8 lines
fs/btrfs/extent-tree.c +5
-0@@ -8829,6 +8829,26 @@ again: goto again; } +/* +* if we are changing raid levels, try to allocate a corresponding +* block group with the new raid level. +*/ +if (!(cache->flags &BTRFS_BLOCK_GROUP_SYSTEM )) {
This prevents to switch the system chunk in all cases. What was the reason to do it?
If I remove the check, then the conversions work in all combinations. Eg.
* convert from ext (or use any single device fs)
* add second device
* balance data/metadata/system to raid1
* all fine
Quoted 27 lines · David Sterba
Quoted 26 lines · Chris Mason
Commit
2f0810880f082fa8ba66ab2c33b02e4ff9770a5echangedbtrfs_set_block_group_roto avoid trying to allocate new chunks with the new raid profile during conversion. This fixed failures when there was no space on the drive to allocate a new chunk, but the metadata reserves were sufficient to continue the conversion.But this ended up causing a regression when the drive had plenty of space to allocate new chunks, mostly because
reduce_alloc_profileisn't using the new raid profile.Fixing
btrfs_reduce_alloc_profileis a bigger patch. For now, do a partial revert of2f0810880, and don't error out if we hit ENOSPC.Signed-off-by Chris Mason <[email protected]>--- fs/btrfs/extent-tree.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+)fs/btrfs/extent-tree.c +0
-0@@ -8829,6 +8829,26 @@ again: goto again; }
Quoted 8 lines
fs/btrfs/extent-tree.c again: · line 8836
Quoted 5 lines
fs/btrfs/extent-tree.c +5
-0+/* +* if we are changing raid levels, try to allocate a corresponding +* block group with the new raid level. +*/ +if (!(cache->flags &BTRFS_BLOCK_GROUP_SYSTEM )) {This prevents to switch the system chunk in all cases. What was the reason to do it?
I thought the system chunk was being caught by check_system_chunk below, but no, its using the wrong profile. It did work when I tested, but I ran it a few times in a row and got inconsistent results.
Quoted 3 lines
If I remove the check, then the conversions work in all combinations. Eg.
Thanks, I'm pushing out an updated patch without the check.
-chris