public inbox for linux-btrfs@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] btrfs: zoned: don't skip block group profile checks on conv zones
@ 2024-02-09 10:46 Johannes Thumshirn
  2024-02-09 14:29 ` Naohiro Aota
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Johannes Thumshirn @ 2024-02-09 10:46 UTC (permalink / raw)
  To: linux-btrfs
  Cc: David Sterba, Qu Wenruo, Naohiro Aota, Johannes Thumshirn,
	韩于惟

On a zoned filesystem with conventional zones, we're skipping the block
group profile checks for the conventional zones.

This allows converting a zoned filesystem's data block groups to RAID when
all of the zones backing the chunk are on conventional zones.  But this
will lead to problems, once we're trying to allocate chunks backed by
sequential zones.

So also check for conventional zones when loading a block group's profile
on them.

Reported-by: 韩于惟 <hrx@bupt.moe>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
 fs/btrfs/zoned.c | 30 +++++++++++++++++++++++++++---
 1 file changed, 27 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
index d9716456bce0..5beb6b936e61 100644
--- a/fs/btrfs/zoned.c
+++ b/fs/btrfs/zoned.c
@@ -1369,8 +1369,10 @@ static int btrfs_load_block_group_single(struct btrfs_block_group *bg,
 		return -EIO;
 	}
 
-	bg->alloc_offset = info->alloc_offset;
-	bg->zone_capacity = info->capacity;
+	if (info->alloc_offset != WP_CONVENTIONAL) {
+		bg->alloc_offset = info->alloc_offset;
+		bg->zone_capacity = info->capacity;
+	}
 	if (test_bit(0, active))
 		set_bit(BLOCK_GROUP_FLAG_ZONE_IS_ACTIVE, &bg->runtime_flags);
 	return 0;
@@ -1406,6 +1408,16 @@ static int btrfs_load_block_group_dup(struct btrfs_block_group *bg,
 		return -EIO;
 	}
 
+	if (zone_info[0].alloc_offset == WP_CONVENTIONAL) {
+		zone_info[0].alloc_offset = bg->alloc_offset;
+		zone_info[0].capacity = bg->length;
+	}
+
+	if (zone_info[1].alloc_offset == WP_CONVENTIONAL) {
+		zone_info[1].alloc_offset = bg->alloc_offset;
+		zone_info[1].capacity = bg->length;
+	}
+
 	if (test_bit(0, active) != test_bit(1, active)) {
 		if (!btrfs_zone_activate(bg))
 			return -EIO;
@@ -1458,6 +1470,9 @@ static int btrfs_load_block_group_raid1(struct btrfs_block_group *bg,
 						 zone_info[1].capacity);
 	}
 
+	if (zone_info[0].alloc_offset == WP_CONVENTIONAL)
+		zone_info[0].alloc_offset = bg->alloc_offset;
+
 	if (zone_info[0].alloc_offset != WP_MISSING_DEV)
 		bg->alloc_offset = zone_info[0].alloc_offset;
 	else
@@ -1479,6 +1494,11 @@ static int btrfs_load_block_group_raid0(struct btrfs_block_group *bg,
 		return -EINVAL;
 	}
 
+	for (int i = 0; i < map->num_stripes; i++) {
+		if (zone_info[i].alloc_offset == WP_CONVENTIONAL)
+			zone_info[i].alloc_offset = bg->alloc_offset;
+	}
+
 	for (int i = 0; i < map->num_stripes; i++) {
 		if (zone_info[i].alloc_offset == WP_MISSING_DEV ||
 		    zone_info[i].alloc_offset == WP_CONVENTIONAL)
@@ -1511,6 +1531,11 @@ static int btrfs_load_block_group_raid10(struct btrfs_block_group *bg,
 		return -EINVAL;
 	}
 
+	for (int i = 0; i < map->num_stripes; i++) {
+		if (zone_info[i].alloc_offset == WP_CONVENTIONAL)
+			zone_info[i].alloc_offset = bg->alloc_offset;
+	}
+
 	for (int i = 0; i < map->num_stripes; i++) {
 		if (zone_info[i].alloc_offset == WP_MISSING_DEV ||
 		    zone_info[i].alloc_offset == WP_CONVENTIONAL)
@@ -1605,7 +1630,6 @@ int btrfs_load_block_group_zone_info(struct btrfs_block_group *cache, bool new)
 		} else if (map->num_stripes == num_conventional) {
 			cache->alloc_offset = last_alloc;
 			set_bit(BLOCK_GROUP_FLAG_ZONE_IS_ACTIVE, &cache->runtime_flags);
-			goto out;
 		}
 	}
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] btrfs: zoned: don't skip block group profile checks on conv zones
  2024-02-09 10:46 Johannes Thumshirn
@ 2024-02-09 14:29 ` Naohiro Aota
  2024-02-12  9:48   ` Johannes Thumshirn
  2024-02-12  1:59 ` Damien Le Moal
  2024-02-12 11:26 ` David Sterba
  2 siblings, 1 reply; 10+ messages in thread
From: Naohiro Aota @ 2024-02-09 14:29 UTC (permalink / raw)
  To: Johannes Thumshirn
  Cc: linux-btrfs@vger.kernel.org, David Sterba, Qu Wenruo,
	韩于惟

On Fri, Feb 09, 2024 at 02:46:26AM -0800, Johannes Thumshirn wrote:
> On a zoned filesystem with conventional zones, we're skipping the block
> group profile checks for the conventional zones.
> 
> This allows converting a zoned filesystem's data block groups to RAID when
> all of the zones backing the chunk are on conventional zones.  But this
> will lead to problems, once we're trying to allocate chunks backed by
> sequential zones.
> 
> So also check for conventional zones when loading a block group's profile
> on them.
> 
> Reported-by: 韩于惟 <hrx@bupt.moe>
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> ---
>  fs/btrfs/zoned.c | 30 +++++++++++++++++++++++++++---
>  1 file changed, 27 insertions(+), 3 deletions(-)

This seems complex than necessary. It is duplicating
calculate_alloc_pointer() code into the per-profile loading functions. How
about adding a check equivalent below after the out label?

	if ((map->type & BTRFS_BLOCK_GROUP_DATA) && !fs_info->stripe_root) {
		btrfs_err(fs_info, "zoned: data %s needs raid-stripe-tree",
			  btrfs_bg_type_to_raid_name(map->type));
		return -EINVAL;
	}

> diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
> index d9716456bce0..5beb6b936e61 100644
> --- a/fs/btrfs/zoned.c
> +++ b/fs/btrfs/zoned.c
> @@ -1369,8 +1369,10 @@ static int btrfs_load_block_group_single(struct btrfs_block_group *bg,
>  		return -EIO;
>  	}
>  
> -	bg->alloc_offset = info->alloc_offset;
> -	bg->zone_capacity = info->capacity;
> +	if (info->alloc_offset != WP_CONVENTIONAL) {
> +		bg->alloc_offset = info->alloc_offset;
> +		bg->zone_capacity = info->capacity;
> +	}
>  	if (test_bit(0, active))
>  		set_bit(BLOCK_GROUP_FLAG_ZONE_IS_ACTIVE, &bg->runtime_flags);
>  	return 0;
> @@ -1406,6 +1408,16 @@ static int btrfs_load_block_group_dup(struct btrfs_block_group *bg,
>  		return -EIO;
>  	}
>  
> +	if (zone_info[0].alloc_offset == WP_CONVENTIONAL) {
> +		zone_info[0].alloc_offset = bg->alloc_offset;
> +		zone_info[0].capacity = bg->length;
> +	}
> +
> +	if (zone_info[1].alloc_offset == WP_CONVENTIONAL) {
> +		zone_info[1].alloc_offset = bg->alloc_offset;
> +		zone_info[1].capacity = bg->length;
> +	}
> +
>  	if (test_bit(0, active) != test_bit(1, active)) {
>  		if (!btrfs_zone_activate(bg))
>  			return -EIO;
> @@ -1458,6 +1470,9 @@ static int btrfs_load_block_group_raid1(struct btrfs_block_group *bg,
>  						 zone_info[1].capacity);
>  	}
>  
> +	if (zone_info[0].alloc_offset == WP_CONVENTIONAL)
> +		zone_info[0].alloc_offset = bg->alloc_offset;
> +
>  	if (zone_info[0].alloc_offset != WP_MISSING_DEV)
>  		bg->alloc_offset = zone_info[0].alloc_offset;
>  	else
> @@ -1479,6 +1494,11 @@ static int btrfs_load_block_group_raid0(struct btrfs_block_group *bg,
>  		return -EINVAL;
>  	}
>  
> +	for (int i = 0; i < map->num_stripes; i++) {
> +		if (zone_info[i].alloc_offset == WP_CONVENTIONAL)
> +			zone_info[i].alloc_offset = bg->alloc_offset;
> +	}
> +
>  	for (int i = 0; i < map->num_stripes; i++) {
>  		if (zone_info[i].alloc_offset == WP_MISSING_DEV ||
>  		    zone_info[i].alloc_offset == WP_CONVENTIONAL)
> @@ -1511,6 +1531,11 @@ static int btrfs_load_block_group_raid10(struct btrfs_block_group *bg,
>  		return -EINVAL;
>  	}
>  
> +	for (int i = 0; i < map->num_stripes; i++) {
> +		if (zone_info[i].alloc_offset == WP_CONVENTIONAL)
> +			zone_info[i].alloc_offset = bg->alloc_offset;
> +	}
> +
>  	for (int i = 0; i < map->num_stripes; i++) {
>  		if (zone_info[i].alloc_offset == WP_MISSING_DEV ||
>  		    zone_info[i].alloc_offset == WP_CONVENTIONAL)
> @@ -1605,7 +1630,6 @@ int btrfs_load_block_group_zone_info(struct btrfs_block_group *cache, bool new)
>  		} else if (map->num_stripes == num_conventional) {
>  			cache->alloc_offset = last_alloc;
>  			set_bit(BLOCK_GROUP_FLAG_ZONE_IS_ACTIVE, &cache->runtime_flags);
> -			goto out;
>  		}
>  	}
>  
> -- 
> 2.43.0
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] btrfs: zoned: don't skip block group profile checks on conv zones
  2024-02-09 10:46 Johannes Thumshirn
  2024-02-09 14:29 ` Naohiro Aota
@ 2024-02-12  1:59 ` Damien Le Moal
  2024-02-12  4:00   ` HAN Yuwei
  2024-02-12 11:26 ` David Sterba
  2 siblings, 1 reply; 10+ messages in thread
From: Damien Le Moal @ 2024-02-12  1:59 UTC (permalink / raw)
  To: Johannes Thumshirn, linux-btrfs
  Cc: David Sterba, Qu Wenruo, Naohiro Aota, 韩于惟

On 2/9/24 19:46, Johannes Thumshirn wrote:
> On a zoned filesystem with conventional zones, we're skipping the block
> group profile checks for the conventional zones.
> 
> This allows converting a zoned filesystem's data block groups to RAID when
> all of the zones backing the chunk are on conventional zones.  But this
> will lead to problems, once we're trying to allocate chunks backed by
> sequential zones.
> 
> So also check for conventional zones when loading a block group's profile
> on them.
> 
> Reported-by: 韩于惟 <hrx@bupt.moe>

Let's keep using the roman alphabet for names please...

Yuwei,

Not all kernel developers can read Chinese, so please sign your emails with your
name written in roman alphabet.

> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> ---
>  fs/btrfs/zoned.c | 30 +++++++++++++++++++++++++++---
>  1 file changed, 27 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
> index d9716456bce0..5beb6b936e61 100644
> --- a/fs/btrfs/zoned.c
> +++ b/fs/btrfs/zoned.c
> @@ -1369,8 +1369,10 @@ static int btrfs_load_block_group_single(struct btrfs_block_group *bg,
>  		return -EIO;
>  	}
>  
> -	bg->alloc_offset = info->alloc_offset;
> -	bg->zone_capacity = info->capacity;
> +	if (info->alloc_offset != WP_CONVENTIONAL) {
> +		bg->alloc_offset = info->alloc_offset;
> +		bg->zone_capacity = info->capacity;
> +	}
>  	if (test_bit(0, active))
>  		set_bit(BLOCK_GROUP_FLAG_ZONE_IS_ACTIVE, &bg->runtime_flags);
>  	return 0;
> @@ -1406,6 +1408,16 @@ static int btrfs_load_block_group_dup(struct btrfs_block_group *bg,
>  		return -EIO;
>  	}
>  
> +	if (zone_info[0].alloc_offset == WP_CONVENTIONAL) {
> +		zone_info[0].alloc_offset = bg->alloc_offset;
> +		zone_info[0].capacity = bg->length;
> +	}
> +
> +	if (zone_info[1].alloc_offset == WP_CONVENTIONAL) {
> +		zone_info[1].alloc_offset = bg->alloc_offset;
> +		zone_info[1].capacity = bg->length;
> +	}
> +
>  	if (test_bit(0, active) != test_bit(1, active)) {
>  		if (!btrfs_zone_activate(bg))
>  			return -EIO;
> @@ -1458,6 +1470,9 @@ static int btrfs_load_block_group_raid1(struct btrfs_block_group *bg,
>  						 zone_info[1].capacity);
>  	}
>  
> +	if (zone_info[0].alloc_offset == WP_CONVENTIONAL)
> +		zone_info[0].alloc_offset = bg->alloc_offset;
> +
>  	if (zone_info[0].alloc_offset != WP_MISSING_DEV)
>  		bg->alloc_offset = zone_info[0].alloc_offset;
>  	else
> @@ -1479,6 +1494,11 @@ static int btrfs_load_block_group_raid0(struct btrfs_block_group *bg,
>  		return -EINVAL;
>  	}
>  
> +	for (int i = 0; i < map->num_stripes; i++) {
> +		if (zone_info[i].alloc_offset == WP_CONVENTIONAL)
> +			zone_info[i].alloc_offset = bg->alloc_offset;
> +	}
> +
>  	for (int i = 0; i < map->num_stripes; i++) {
>  		if (zone_info[i].alloc_offset == WP_MISSING_DEV ||
>  		    zone_info[i].alloc_offset == WP_CONVENTIONAL)
> @@ -1511,6 +1531,11 @@ static int btrfs_load_block_group_raid10(struct btrfs_block_group *bg,
>  		return -EINVAL;
>  	}
>  
> +	for (int i = 0; i < map->num_stripes; i++) {
> +		if (zone_info[i].alloc_offset == WP_CONVENTIONAL)
> +			zone_info[i].alloc_offset = bg->alloc_offset;
> +	}
> +
>  	for (int i = 0; i < map->num_stripes; i++) {
>  		if (zone_info[i].alloc_offset == WP_MISSING_DEV ||
>  		    zone_info[i].alloc_offset == WP_CONVENTIONAL)
> @@ -1605,7 +1630,6 @@ int btrfs_load_block_group_zone_info(struct btrfs_block_group *cache, bool new)
>  		} else if (map->num_stripes == num_conventional) {
>  			cache->alloc_offset = last_alloc;
>  			set_bit(BLOCK_GROUP_FLAG_ZONE_IS_ACTIVE, &cache->runtime_flags);
> -			goto out;
>  		}
>  	}
>  

-- 
Damien Le Moal
Western Digital Research


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] btrfs: zoned: don't skip block group profile checks on conv zones
  2024-02-12  1:59 ` Damien Le Moal
@ 2024-02-12  4:00   ` HAN Yuwei
  0 siblings, 0 replies; 10+ messages in thread
From: HAN Yuwei @ 2024-02-12  4:00 UTC (permalink / raw)
  To: Damien Le Moal, Johannes Thumshirn, linux-btrfs
  Cc: David Sterba, Qu Wenruo, Naohiro Aota


[-- Attachment #1.1.1: Type: text/plain, Size: 3812 bytes --]


在 2024/2/12 9:59, Damien Le Moal 写道:
> On 2/9/24 19:46, Johannes Thumshirn wrote:
>> On a zoned filesystem with conventional zones, we're skipping the block
>> group profile checks for the conventional zones.
>>
>> This allows converting a zoned filesystem's data block groups to RAID when
>> all of the zones backing the chunk are on conventional zones.  But this
>> will lead to problems, once we're trying to allocate chunks backed by
>> sequential zones.
>>
>> So also check for conventional zones when loading a block group's profile
>> on them.
>>
>> Reported-by: 韩于惟 <hrx@bupt.moe>
> Let's keep using the roman alphabet for names please...

Updated. Can change to "HAN Yuwei".

>> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
>> ---
>>   fs/btrfs/zoned.c | 30 +++++++++++++++++++++++++++---
>>   1 file changed, 27 insertions(+), 3 deletions(-)
>>
>> diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
>> index d9716456bce0..5beb6b936e61 100644
>> --- a/fs/btrfs/zoned.c
>> +++ b/fs/btrfs/zoned.c
>> @@ -1369,8 +1369,10 @@ static int btrfs_load_block_group_single(struct btrfs_block_group *bg,
>>   		return -EIO;
>>   	}
>>   
>> -	bg->alloc_offset = info->alloc_offset;
>> -	bg->zone_capacity = info->capacity;
>> +	if (info->alloc_offset != WP_CONVENTIONAL) {
>> +		bg->alloc_offset = info->alloc_offset;
>> +		bg->zone_capacity = info->capacity;
>> +	}
>>   	if (test_bit(0, active))
>>   		set_bit(BLOCK_GROUP_FLAG_ZONE_IS_ACTIVE, &bg->runtime_flags);
>>   	return 0;
>> @@ -1406,6 +1408,16 @@ static int btrfs_load_block_group_dup(struct btrfs_block_group *bg,
>>   		return -EIO;
>>   	}
>>   
>> +	if (zone_info[0].alloc_offset == WP_CONVENTIONAL) {
>> +		zone_info[0].alloc_offset = bg->alloc_offset;
>> +		zone_info[0].capacity = bg->length;
>> +	}
>> +
>> +	if (zone_info[1].alloc_offset == WP_CONVENTIONAL) {
>> +		zone_info[1].alloc_offset = bg->alloc_offset;
>> +		zone_info[1].capacity = bg->length;
>> +	}
>> +
>>   	if (test_bit(0, active) != test_bit(1, active)) {
>>   		if (!btrfs_zone_activate(bg))
>>   			return -EIO;
>> @@ -1458,6 +1470,9 @@ static int btrfs_load_block_group_raid1(struct btrfs_block_group *bg,
>>   						 zone_info[1].capacity);
>>   	}
>>   
>> +	if (zone_info[0].alloc_offset == WP_CONVENTIONAL)
>> +		zone_info[0].alloc_offset = bg->alloc_offset;
>> +
>>   	if (zone_info[0].alloc_offset != WP_MISSING_DEV)
>>   		bg->alloc_offset = zone_info[0].alloc_offset;
>>   	else
>> @@ -1479,6 +1494,11 @@ static int btrfs_load_block_group_raid0(struct btrfs_block_group *bg,
>>   		return -EINVAL;
>>   	}
>>   
>> +	for (int i = 0; i < map->num_stripes; i++) {
>> +		if (zone_info[i].alloc_offset == WP_CONVENTIONAL)
>> +			zone_info[i].alloc_offset = bg->alloc_offset;
>> +	}
>> +
>>   	for (int i = 0; i < map->num_stripes; i++) {
>>   		if (zone_info[i].alloc_offset == WP_MISSING_DEV ||
>>   		    zone_info[i].alloc_offset == WP_CONVENTIONAL)
>> @@ -1511,6 +1531,11 @@ static int btrfs_load_block_group_raid10(struct btrfs_block_group *bg,
>>   		return -EINVAL;
>>   	}
>>   
>> +	for (int i = 0; i < map->num_stripes; i++) {
>> +		if (zone_info[i].alloc_offset == WP_CONVENTIONAL)
>> +			zone_info[i].alloc_offset = bg->alloc_offset;
>> +	}
>> +
>>   	for (int i = 0; i < map->num_stripes; i++) {
>>   		if (zone_info[i].alloc_offset == WP_MISSING_DEV ||
>>   		    zone_info[i].alloc_offset == WP_CONVENTIONAL)
>> @@ -1605,7 +1630,6 @@ int btrfs_load_block_group_zone_info(struct btrfs_block_group *cache, bool new)
>>   		} else if (map->num_stripes == num_conventional) {
>>   			cache->alloc_offset = last_alloc;
>>   			set_bit(BLOCK_GROUP_FLAG_ZONE_IS_ACTIVE, &cache->runtime_flags);
>> -			goto out;
>>   		}
>>   	}
>>   

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 1589 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] btrfs: zoned: don't skip block group profile checks on conv zones
  2024-02-09 14:29 ` Naohiro Aota
@ 2024-02-12  9:48   ` Johannes Thumshirn
  0 siblings, 0 replies; 10+ messages in thread
From: Johannes Thumshirn @ 2024-02-12  9:48 UTC (permalink / raw)
  To: Naohiro Aota
  Cc: linux-btrfs@vger.kernel.org, David Sterba, Qu Wenruo,
	韩于惟

On 09.02.24 15:29, Naohiro Aota wrote:
> On Fri, Feb 09, 2024 at 02:46:26AM -0800, Johannes Thumshirn wrote:
>> On a zoned filesystem with conventional zones, we're skipping the block
>> group profile checks for the conventional zones.
>>
>> This allows converting a zoned filesystem's data block groups to RAID when
>> all of the zones backing the chunk are on conventional zones.  But this
>> will lead to problems, once we're trying to allocate chunks backed by
>> sequential zones.
>>
>> So also check for conventional zones when loading a block group's profile
>> on them.
>>
>> Reported-by: 韩于惟 <hrx@bupt.moe>
>> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
>> ---
>>   fs/btrfs/zoned.c | 30 +++++++++++++++++++++++++++---
>>   1 file changed, 27 insertions(+), 3 deletions(-)
> 
> This seems complex than necessary. It is duplicating
> calculate_alloc_pointer() code into the per-profile loading functions. How
> about adding a check equivalent below after the out label?
> 
> 	if ((map->type & BTRFS_BLOCK_GROUP_DATA) && !fs_info->stripe_root) {
> 		btrfs_err(fs_info, "zoned: data %s needs raid-stripe-tree",
> 			  btrfs_bg_type_to_raid_name(map->type));
> 		return -EINVAL;
> 	}

OK will do


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] btrfs: zoned: don't skip block group profile checks on conv zones
  2024-02-09 10:46 Johannes Thumshirn
  2024-02-09 14:29 ` Naohiro Aota
  2024-02-12  1:59 ` Damien Le Moal
@ 2024-02-12 11:26 ` David Sterba
  2 siblings, 0 replies; 10+ messages in thread
From: David Sterba @ 2024-02-12 11:26 UTC (permalink / raw)
  To: Johannes Thumshirn
  Cc: linux-btrfs, David Sterba, Qu Wenruo, Naohiro Aota,
	韩于惟

On Fri, Feb 09, 2024 at 02:46:26AM -0800, Johannes Thumshirn wrote:
> On a zoned filesystem with conventional zones, we're skipping the block
> group profile checks for the conventional zones.
> 
> This allows converting a zoned filesystem's data block groups to RAID when
> all of the zones backing the chunk are on conventional zones.  But this
> will lead to problems, once we're trying to allocate chunks backed by
> sequential zones.

I don't remember if or to what extent the seq+zoned devices are
supported. There are many places with btrfs_is_zoned() that does not
distinguish if the sequential area is present. Some of the changes could
work on sequential but this could lead to strange problems, like in this
case.

We could support as much as is practical but now I don't see which use
cases are that.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH] btrfs: zoned: don't skip block group profile checks on conv zones
@ 2024-02-13  8:16 Johannes Thumshirn
  2024-02-13 21:58 ` Boris Burkov
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Johannes Thumshirn @ 2024-02-13  8:16 UTC (permalink / raw)
  To: linux-btrfs
  Cc: David Sterba, Qu Wenruo, Naohiro Aota, Johannes Thumshirn,
	HAN Yuwei

On a zoned filesystem with conventional zones, we're skipping the block
group profile checks for the conventional zones.

This allows converting a zoned filesystem's data block groups to RAID when
all of the zones backing the chunk are on conventional zones.  But this
will lead to problems, once we're trying to allocate chunks backed by
sequential zones.

So also check for conventional zones when loading a block group's profile
on them.

Reported-by: HAN Yuwei <hrx@bupt.moe>
Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
---
---
 fs/btrfs/zoned.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
index d9716456bce0..e43c689b1253 100644
--- a/fs/btrfs/zoned.c
+++ b/fs/btrfs/zoned.c
@@ -1637,6 +1637,15 @@ int btrfs_load_block_group_zone_info(struct btrfs_block_group *cache, bool new)
 	}
 
 out:
+	/* Reject non SINGLE data profiles without RST */
+	if ((map->type & BTRFS_BLOCK_GROUP_DATA) &&
+	    (map->type & BTRFS_BLOCK_GROUP_PROFILE_MASK) &&
+	    !fs_info->stripe_root) {
+		btrfs_err(fs_info, "zoned: data %s needs raid-stripe-tree",
+			  btrfs_bg_type_to_raid_name(map->type));
+		return -EINVAL;
+	}
+
 	if (cache->alloc_offset > cache->zone_capacity) {
 		btrfs_err(fs_info,
 "zoned: invalid write pointer %llu (larger than zone capacity %llu) in block group %llu",
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] btrfs: zoned: don't skip block group profile checks on conv zones
  2024-02-13  8:16 [PATCH] btrfs: zoned: don't skip block group profile checks on conv zones Johannes Thumshirn
@ 2024-02-13 21:58 ` Boris Burkov
  2024-02-14 15:59 ` Naohiro Aota
  2024-02-14 18:11 ` David Sterba
  2 siblings, 0 replies; 10+ messages in thread
From: Boris Burkov @ 2024-02-13 21:58 UTC (permalink / raw)
  To: Johannes Thumshirn
  Cc: linux-btrfs, David Sterba, Qu Wenruo, Naohiro Aota, HAN Yuwei

On Tue, Feb 13, 2024 at 12:16:15AM -0800, Johannes Thumshirn wrote:
> On a zoned filesystem with conventional zones, we're skipping the block
> group profile checks for the conventional zones.
> 
> This allows converting a zoned filesystem's data block groups to RAID when
> all of the zones backing the chunk are on conventional zones.  But this
> will lead to problems, once we're trying to allocate chunks backed by
> sequential zones.
> 
> So also check for conventional zones when loading a block group's profile
> on them.
> 
> Reported-by: HAN Yuwei <hrx@bupt.moe>
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
Reviewed-by: Boris Burkov <boris@bur.io>
> ---
> ---
>  fs/btrfs/zoned.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
> index d9716456bce0..e43c689b1253 100644
> --- a/fs/btrfs/zoned.c
> +++ b/fs/btrfs/zoned.c
> @@ -1637,6 +1637,15 @@ int btrfs_load_block_group_zone_info(struct btrfs_block_group *cache, bool new)
>  	}
>  
>  out:
> +	/* Reject non SINGLE data profiles without RST */
> +	if ((map->type & BTRFS_BLOCK_GROUP_DATA) &&
> +	    (map->type & BTRFS_BLOCK_GROUP_PROFILE_MASK) &&
> +	    !fs_info->stripe_root) {
> +		btrfs_err(fs_info, "zoned: data %s needs raid-stripe-tree",
> +			  btrfs_bg_type_to_raid_name(map->type));
> +		return -EINVAL;
> +	}
> +
>  	if (cache->alloc_offset > cache->zone_capacity) {
>  		btrfs_err(fs_info,
>  "zoned: invalid write pointer %llu (larger than zone capacity %llu) in block group %llu",
> -- 
> 2.43.0
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] btrfs: zoned: don't skip block group profile checks on conv zones
  2024-02-13  8:16 [PATCH] btrfs: zoned: don't skip block group profile checks on conv zones Johannes Thumshirn
  2024-02-13 21:58 ` Boris Burkov
@ 2024-02-14 15:59 ` Naohiro Aota
  2024-02-14 18:11 ` David Sterba
  2 siblings, 0 replies; 10+ messages in thread
From: Naohiro Aota @ 2024-02-14 15:59 UTC (permalink / raw)
  To: Johannes Thumshirn
  Cc: linux-btrfs@vger.kernel.org, David Sterba, Qu Wenruo, HAN Yuwei

On Tue, Feb 13, 2024 at 12:16:15AM -0800, Johannes Thumshirn wrote:
> On a zoned filesystem with conventional zones, we're skipping the block
> group profile checks for the conventional zones.
> 
> This allows converting a zoned filesystem's data block groups to RAID when
> all of the zones backing the chunk are on conventional zones.  But this
> will lead to problems, once we're trying to allocate chunks backed by
> sequential zones.
> 
> So also check for conventional zones when loading a block group's profile
> on them.
> 
> Reported-by: HAN Yuwei <hrx@bupt.moe>
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>
> ---
> ---
>  fs/btrfs/zoned.c | 9 +++++++++
>  1 file changed, 9 insertions(+)

Looks good,

Reviewed-by: Naohiro Aota <naohiro.aota@wdc.com>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] btrfs: zoned: don't skip block group profile checks on conv zones
  2024-02-13  8:16 [PATCH] btrfs: zoned: don't skip block group profile checks on conv zones Johannes Thumshirn
  2024-02-13 21:58 ` Boris Burkov
  2024-02-14 15:59 ` Naohiro Aota
@ 2024-02-14 18:11 ` David Sterba
  2 siblings, 0 replies; 10+ messages in thread
From: David Sterba @ 2024-02-14 18:11 UTC (permalink / raw)
  To: Johannes Thumshirn
  Cc: linux-btrfs, David Sterba, Qu Wenruo, Naohiro Aota, HAN Yuwei

On Tue, Feb 13, 2024 at 12:16:15AM -0800, Johannes Thumshirn wrote:
> On a zoned filesystem with conventional zones, we're skipping the block
> group profile checks for the conventional zones.
> 
> This allows converting a zoned filesystem's data block groups to RAID when
> all of the zones backing the chunk are on conventional zones.  But this
> will lead to problems, once we're trying to allocate chunks backed by
> sequential zones.
> 
> So also check for conventional zones when loading a block group's profile
> on them.
> 
> Reported-by: HAN Yuwei <hrx@bupt.moe>
> Signed-off-by: Johannes Thumshirn <johannes.thumshirn@wdc.com>

Reviewed-by: David Sterba <dsterba@suse.com>

Please add link to the report (as Link: tag), I think it is interesting
how the root cause was discovered.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2024-02-14 18:12 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-13  8:16 [PATCH] btrfs: zoned: don't skip block group profile checks on conv zones Johannes Thumshirn
2024-02-13 21:58 ` Boris Burkov
2024-02-14 15:59 ` Naohiro Aota
2024-02-14 18:11 ` David Sterba
  -- strict thread matches above, loose matches on Subject: below --
2024-02-09 10:46 Johannes Thumshirn
2024-02-09 14:29 ` Naohiro Aota
2024-02-12  9:48   ` Johannes Thumshirn
2024-02-12  1:59 ` Damien Le Moal
2024-02-12  4:00   ` HAN Yuwei
2024-02-12 11:26 ` David Sterba

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox