Btrfs: try to allocate new chunks with degenerated profile
RFC PATCH 1/2 Btrfs: try to allocate new chunks with degenerated profile
If there is no free space, the free space allocator will try to get space from the block group with the degenerated profile. For example, if there is no free space in the RAID1 block groups, the allocator will try to allocate space from the DUP block groups. And besides that, the space reservation has the similar
behaviour: if there is no enough space in the space cache to reserve, it will reserve the space according to the disk space, and it just take mirror storage into account, no RAID0, RAID1, or RAID10.
So we'd better make the behaviour of chunk allocation correspond with space reservation and free space allocation, if there is no enough disk space to allocate RAID(RAID0, RAID1, RAID10) chunks, we degenerate the profile and try to allocate chunks again. Otherwise, enospc will happen though we reserve the space successfully and BUG_ON() will be triggered.
Degenerating rule:
RAID10 -> RAID1 -> DUP
RAID0 -> SINGLE
--- fs/btrfs/extent-tree.c | 43 +++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 41 insertions(+), 2 deletions(-)
fs/btrfs/extent-tree.c +41 -2
@@ -3065,6 +3065,30 @@ u64 btrfs_reduce_alloc_profile(struct btrfs_root *root, u64 flags)return flags; } +/* +* Degenerate the alloc profile: +* RAID10 -> RAID1 -> DUP +* RAID0 -> SINGLE +* +* This is used when there is no enough disk space to do chunk allocation. +* After degenerating the profile, we will try to allocate new chunks again. +*/ +static u64 btrfs_degenerate_alloc_profile (u64 flags) +{ +if (flags &BTRFS_BLOCK_GROUP_RAID10 ) { + flags &= ~BTRFS_BLOCK_GROUP_RAID10 ; + flags |=BTRFS_BLOCK_GROUP_RAID1 ; + }else if (flags &BTRFS_BLOCK_GROUP_RAID1 ) { + flags &= ~BTRFS_BLOCK_GROUP_RAID1 ; + flags |=BTRFS_BLOCK_GROUP_DUP ; + }else if (flags &BTRFS_BLOCK_GROUP_RAID0 ) { + flags &= ~BTRFS_BLOCK_GROUP_RAID0 ; + }else + flags =ULLONG_MAX ; + +return flags; +} +static u64 get_alloc_profile (struct btrfs_root *root,u64 flags) {if (flags &BTRFS_BLOCK_GROUP_DATA )
@@ -3356,8 +3380,23 @@ again: } ret =btrfs_alloc_chunk (trans, extent_root, flags); -if (ret <0 && ret != -ENOSPC ) - goto out; +if (ret <0 ) { +if (ret != -ENOSPC ) + goto out; + +/* +* Degenerate the alloc profile: +* RAID10 -> RAID1 -> DUP +* RAID0 -> SINGLE +* then we will try to allocate new chunks again. By this way, +* we can utilize the whole disk spacem and make the behaviour +* of the chunk allocation correspond with the space reservation +* and the free space allocation. +*/ + flags =btrfs_degenerate_alloc_profile (flags); +if (flags !=ULLONG_MAX ) + goto again; + }spin_lock (&space_info->lock);if (ret)
Signature · 1 line
1.7.6.5
These two didn't make my first pull request just because I wanted to get something out the door. I'll definitely have them in the next pull.
-chris
Quoted 2 lines · Chris Mason
These two didn't make my first pull request just because I wanted to get something out the door. I'll definitely have them in the next pull.
Please, don't do that! You can't just degenerate to DUP when RAID1 is out of space, that's entirely different.
It's debatable whether degeneration from RAID0 to single is acceptable, but that again has different characteristics.
ENOSPC is the best choice for both in my opinon.
-Jan
Quoted 6 lines · Jan Schmidt
Quoted 2 lines · Chris Mason
These two didn't make my first pull request just because I wanted to get something out the door. I'll definitely have them in the next pull.
Please, don't do that! You can't just degenerate to DUP when RAID1 is out of space, that's entirely different.
Agreed. This isn't a good idea.
Quoted 2 lines
It's debatable whether degeneration from RAID0 to single is acceptable, but that again has different characteristics.
RAID-0 to single and RAID-10 to RAID-1 are less controversial, I
think, although we need to be clear about what the performance guarantees of the striping are (and how they would be affected by the move to fewer disks). Given that we already degrade the striping of RAID-0 down to "as many devices as we can fit right now", we're not really providing much in the way of performance guarantees.
(I'd definitely support some way of defining a fixed-width stripe,
but that's a whole separate question that we shouldn't get into now).
Quoted 3 lines
ENOSPC is the best choice for both in my opinon.
-Jan
Signature · 3 lines
=== Hugo Mills: hugo@... carfax.org.uk | darksatanic.net | lug.org.uk ===
PGP key: 515C238D from wwwkeys.eu.pgp.net or http://www.carfax.org.uk
--- What's a Nazgûl like you doing in a place like this? --- Quoted 3 lines · Hugo Mills
Quoted 2 lines · Jan Schmidt
Quoted 1 line · Chris Mason
These two didn't make my first pull request just because I wanted =
to get
Quoted 1 line
Quoted 1 line
Quoted 1 line
something out the door. =C2=A0I'll definitely have them in the nex=
t pull.
Quoted 2 lines
Quoted 2 lines
Please, don't do that! You can't just degenerate to DUP when RAID1 i=
s
Quoted 5 lines
Quoted 1 line
out of space, that's entirely different.
=C2=A0 Agreed. This isn't a good idea.
Quoted 1 line
It's debatable whether degeneration from RAID0 to single is acceptab=
le,
Quoted 3 lines
Quoted 1 line
but that again has different characteristics.
=C2=A0 RAID-0 to single and RAID-10 to RAID-1 are less controversial,=
I
Quoted 5 lines
think, although we need to be clear about what the performance guarantees of the striping are (and how they would be affected by the move to fewer disks). Given that we already degrade the striping of RAID-0 down to "as many devices as we can fit right now", we're not really providing much in the way of performance guarantees.
Definitely this degeneration is good idea, but it should be only optional and not default behavior of filesystem, default should be ENOSPC.
Quoted 2 lines
=C2=A0 (I'd definitely support some way of defining a fixed-width str=
ipe,
Quoted 8 lines
but that's a whole separate question that we shouldn't get into now).
Quoted 3 lines
ENOSPC is the best choice for both in my opinon.
-Jan
--
=3D=3D=3D Hugo Mills: hugo@... carfax.org.uk | darksatanic.net | lug.=
org.uk =3D=3D=3D
Quoted 1 line
=C2=A0PGP key: 515C238D from wwwkeys.eu.pgp.net or http://www.carfax.=
org.uk
Quoted 1 line
=C2=A0 =C2=A0 =C2=A0--- What's a Nazg=C3=BBl like you doing in a plac=
e like this? ---
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" = in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 11 lines · Jan Schmidt
Quoted 2 lines · Chris Mason
These two didn't make my first pull request just because I wanted to get something out the door. I'll definitely have them in the next pull.
Please, don't do that! You can't just degenerate to DUP when RAID1 is out of space, that's entirely different.
It's debatable whether degeneration from RAID0 to single is acceptable, but that again has different characteristics.
ENOSPC is the best choice for both in my opinon.
I understand what you said, but in fact, the free space allocator can degenerate the profile if it doesn't find enough free space. This patch just follows the rule which exists in the current code.
Maybe adding a new mount option is another good option.
Thanks
Miao
Quoted 7 lines
-Jan
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 8 lines · Miao Xie
Quoted 7 lines · Jan Schmidt
Quoted 2 lines · Chris Mason
These two didn't make my first pull request just because I wanted to get something out the door. I'll definitely have them in the next pull.
Please, don't do that! You can't just degenerate to DUP when RAID1 is out of space, that's entirely different.
Quoted 8 lines
Quoted 4 lines
It's debatable whether degeneration from RAID0 to single is acceptable, but that again has different characteristics.
ENOSPC is the best choice for both in my opinon.
I understand what you said, but in fact, the free space allocator can degenerate the profile if it doesn't find enough free space. This patch just follows the rule which exists in the current code.
I'm not sure what you mean with "free space allocation". In my understanding, new "free" space is made available by allocating a new chunk, and that's what you're suggesting to change. What am I missing here?
Calculation of free space like for df output is known to be at least unintuitive (I'd say wrong). We'd better fix that.
Space reservation may be wrongly assuming it can use the whole disk. If that's the case, we must fix it.
Quoted 1 line
Maybe adding a new mount option is another good option.
I don't think so. If you want RAID-1, you get RAID-1, i.e. every stripe on two disks. If you're okay with DUP, use DUP. But even with a mount option, degeneration would still go silent and you'd never know which part of your data would survive a single disk failure.
Think of degenration of your metadata profile to DUP: Suddenly, your most recent extent tree wouldn't survive a single disk failure.
In my opinion, we're responsible not to offer any dangerous (mount) options that can make users lose data eventually. Even if we can point them to the documentation (which they didn't read) that explains the risks of that option.
-Jan
Quoted 14 lines · Miao Xie
If there is no free space, the free space allocator will try to get space from the block group with the degenerated profile. For example, if there is no free space in the RAID1 block groups, the allocator will try to allocate space from the DUP block groups. And besides that, the space reservation has the similar
behaviour: if there is no enough space in the space cache to reserve, it will reserve the space according to the disk space, and it just take mirror storage into account, no RAID0, RAID1, or RAID10.So we'd better make the behaviour of chunk allocation correspond with space reservation and free space allocation, if there is no enough disk space to allocate
RAID(RAID0, RAID1, RAID10)chunks, we degenerate the profile and try to allocate chunks again. Otherwise, enospc will happen though we reserve the space successfully andBUG_ON()will be triggered.
Quoted 4 lines
Degenerating rule:
RAID10 -> RAID1 -> DUP
RAID0 -> SINGLE
Instead of changing the profile, wouldn't it be easier to just allow RAID10 go down to 2 disks and RAID0 to 1? That would make things easier in many places.
What I'm strongly opposed to is changing a RAID1 to DUP, as this loses the protection against a single disk failure.
-Arne
Quoted 66 lines
Signed-off-by Miao Xie <[email protected]>--- fs/btrfs/extent-tree.c | 43 +++++++++++++++++++++++++++++++++++++++++-- 1 files changed, 41 insertions(+), 2 deletions(-)fs/btrfs/extent-tree.c +41
-2@@ -3065,6 +3065,30 @@ u64 btrfs_reduce_alloc_profile(struct btrfs_root *root, u64 flags)return flags; } +/* +* Degenerate the alloc profile: +* RAID10 -> RAID1 -> DUP +* RAID0 -> SINGLE +* +* This is used when there is no enough disk space to do chunk allocation. +* After degenerating the profile, we will try to allocate new chunks again. +*/ +static u64 btrfs_degenerate_alloc_profile (u64 flags) +{ +if (flags &BTRFS_BLOCK_GROUP_RAID10 ) { + flags &= ~BTRFS_BLOCK_GROUP_RAID10 ; + flags |=BTRFS_BLOCK_GROUP_RAID1 ; + }else if (flags &BTRFS_BLOCK_GROUP_RAID1 ) { + flags &= ~BTRFS_BLOCK_GROUP_RAID1 ; + flags |=BTRFS_BLOCK_GROUP_DUP ; + }else if (flags &BTRFS_BLOCK_GROUP_RAID0 ) { + flags &= ~BTRFS_BLOCK_GROUP_RAID0 ; + }else + flags =ULLONG_MAX ; + +return flags; +} +static u64 get_alloc_profile (struct btrfs_root *root,u64 flags) {if (flags &BTRFS_BLOCK_GROUP_DATA )@@ -3356,8 +3380,23 @@ again: } ret =btrfs_alloc_chunk (trans, extent_root, flags); -if (ret <0 && ret != -ENOSPC ) - goto out; +if (ret <0 ) { +if (ret != -ENOSPC ) + goto out; + +/* +* Degenerate the alloc profile: +* RAID10 -> RAID1 -> DUP +* RAID0 -> SINGLE +* then we will try to allocate new chunks again. By this way, +* we can utilize the whole disk spacem and make the behaviour +* of the chunk allocation correspond with the space reservation +* and the free space allocation. +*/ + flags =btrfs_degenerate_alloc_profile (flags); +if (flags !=ULLONG_MAX ) + goto again; + }spin_lock (&space_info->lock);if (ret)
Quoted 9 lines · Miao Xie
So we'd better make the behaviour of chunk allocation correspond with space reservation and free space allocation, if there is no enough disk space to allocate
RAID(RAID0, RAID1, RAID10)chunks, we degenerate the profile and try to allocate chunks again. Otherwise, enospc will happen though we reserve the space successfully andBUG_ON()will be triggered.Degenerating rule:
RAID10 -> RAID1 -> DUP
RAID0 -> SINGLE
I think that silently changing RAID level under unpredictable conditions could be a very unpleasant suprise to the administrator. Even worse when it requires manual intervention to undo the change and without any possibility to prevent it to happen again.
In case ENOSPC happens early due to overbooking (like untarring lots of files as reported several times), will probably make it happen sooner than when "there is absolutely no free space available". We will lose automatic raid1-repair and scrub repair capabilities, that's not good.
david