linux-btrfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] btrfs-progs: add new precaution check for incoming subpage support
@ 2020-11-09  5:39 Qu Wenruo
  2020-11-09  5:39 ` [PATCH 1/2] btrfs-progs: check: detect and warn about tree blocks cross 64K page boundary Qu Wenruo
  2020-11-09  5:39 ` [PATCH 2/2] btrfs-progs: tests: check the result log for critical warnings Qu Wenruo
  0 siblings, 2 replies; 8+ messages in thread
From: Qu Wenruo @ 2020-11-09  5:39 UTC (permalink / raw)
  To: linux-btrfs

For the incoming subpage support, we need to ensure no tree block could
cross 64K page boundary.

Currently both kernel and btrfs-progs won't create such tree blocks, but
still add such check as non-critical warning to catch any ancient btrfs
in the wild.

Also enhance the existing test cases to catch the warning message just
in case we miss something.

Qu Wenruo (2):
  btrfs-progs: check: detect and warn about tree blocks cross 64K page
    boundary
  btrfs-progs: tests: check the result log for critical warnings

 check/main.c           |  2 ++
 check/mode-common.h    | 18 ++++++++++++++++++
 check/mode-lowmem.c    |  2 ++
 tests/common           | 12 ++++++++++++
 tests/convert-tests.sh |  1 +
 tests/fsck-tests.sh    |  1 +
 tests/misc-tests.sh    |  1 +
 tests/mkfs-tests.sh    |  1 +
 8 files changed, 38 insertions(+)

-- 
2.29.2


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

* [PATCH 1/2] btrfs-progs: check: detect and warn about tree blocks cross 64K page boundary
  2020-11-09  5:39 [PATCH 0/2] btrfs-progs: add new precaution check for incoming subpage support Qu Wenruo
@ 2020-11-09  5:39 ` Qu Wenruo
  2021-03-02  8:48   ` Wang Yugui
  2020-11-09  5:39 ` [PATCH 2/2] btrfs-progs: tests: check the result log for critical warnings Qu Wenruo
  1 sibling, 1 reply; 8+ messages in thread
From: Qu Wenruo @ 2020-11-09  5:39 UTC (permalink / raw)
  To: linux-btrfs

For the incoming subpage support, there is a new requirement for tree
blocks.
Tree blocks should not cross 64K page boudnary.

For current btrfs-progs and kernel, there shouldn't be any causes to
create such tree blocks.

But still, we want to detect such tree blocks in the wild before subpage
support fully lands in upstream.

This patch will add such check for both lowmem and original mode.
Currently it's just a warning, since there aren't many users using 64K
page size yet.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 check/main.c        |  2 ++
 check/mode-common.h | 18 ++++++++++++++++++
 check/mode-lowmem.c |  2 ++
 3 files changed, 22 insertions(+)

diff --git a/check/main.c b/check/main.c
index e7996b7c8c0e..0ce9c2f334b4 100644
--- a/check/main.c
+++ b/check/main.c
@@ -5375,6 +5375,8 @@ static int process_extent_item(struct btrfs_root *root,
 		      num_bytes, gfs_info->sectorsize);
 		return -EIO;
 	}
+	if (metadata)
+		btrfs_check_subpage_eb_alignment(key.objectid, num_bytes);
 
 	memset(&tmpl, 0, sizeof(tmpl));
 	tmpl.start = key.objectid;
diff --git a/check/mode-common.h b/check/mode-common.h
index 4efc07a4f44d..bcda0f53e2c4 100644
--- a/check/mode-common.h
+++ b/check/mode-common.h
@@ -171,4 +171,22 @@ static inline u32 btrfs_type_to_imode(u8 type)
 
 	return imode_by_btrfs_type[(type)];
 }
+
+/*
+ * Check tree block alignement for future subpage support on 64K page system.
+ *
+ * Subpage support on 64K page size require one eb to be completely contained
+ * by a page. Not allowing a tree block to cross 64K page boudanry.
+ *
+ * Since subpage support is still under development, this check only provides
+ * warning.
+ */
+static inline void btrfs_check_subpage_eb_alignment(u64 start, u32 len)
+{
+	if (start / BTRFS_MAX_METADATA_BLOCKSIZE !=
+	    (start + len) / BTRFS_MAX_METADATA_BLOCKSIZE)
+		warning(
+"tree block [%llu, %llu) crosses 64K page boudnary, may cause problem for 64K page system",
+			start, start + len);
+}
 #endif
diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index 2b689b2abf63..6dbfe829bb7c 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -4206,6 +4206,8 @@ static int check_extent_item(struct btrfs_path *path)
 		      key.objectid, key.objectid + nodesize);
 		err |= CROSSING_STRIPE_BOUNDARY;
 	}
+	if (metadata)
+		btrfs_check_subpage_eb_alignment(key.objectid, nodesize);
 
 	ptr = (unsigned long)(ei + 1);
 
-- 
2.29.2


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

* [PATCH 2/2] btrfs-progs: tests: check the result log for critical warnings
  2020-11-09  5:39 [PATCH 0/2] btrfs-progs: add new precaution check for incoming subpage support Qu Wenruo
  2020-11-09  5:39 ` [PATCH 1/2] btrfs-progs: check: detect and warn about tree blocks cross 64K page boundary Qu Wenruo
@ 2020-11-09  5:39 ` Qu Wenruo
  2021-02-19 14:12   ` David Sterba
  1 sibling, 1 reply; 8+ messages in thread
From: Qu Wenruo @ 2020-11-09  5:39 UTC (permalink / raw)
  To: linux-btrfs

Introduce a new function, check_test_results(), for
misc/fsck/convert/mkfs test cases.

This function is currently to catch warning message for subpage support,
but can be later expanded for other usages.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 tests/common           | 12 ++++++++++++
 tests/convert-tests.sh |  1 +
 tests/fsck-tests.sh    |  1 +
 tests/misc-tests.sh    |  1 +
 tests/mkfs-tests.sh    |  1 +
 5 files changed, 16 insertions(+)

diff --git a/tests/common b/tests/common
index b3f5cc139525..f1314d8f5a20 100644
--- a/tests/common
+++ b/tests/common
@@ -817,4 +817,16 @@ init_env()
 		echo "  convert: $TEST_ARGS_CONVERT" >> "$RESULTS"
 	fi
 }
+
+# Catch critical warning messages
+check_test_results()
+{
+	local results="$1"
+	local testname="$2"
+
+	# Check subpage related warning
+	if grep -q "crrosses 64K page boundary" "$results"; then
+		_fail "found subpage related warning for case $testname"
+	fi
+}
 init_env
diff --git a/tests/convert-tests.sh b/tests/convert-tests.sh
index 24b3ec0df144..9653033fe09a 100755
--- a/tests/convert-tests.sh
+++ b/tests/convert-tests.sh
@@ -70,6 +70,7 @@ run_one_test() {
 			fi
 			_fail "test failed for case $testname"
 		fi
+		check_test_results "$RESULTS" "$testname"
 	else
 		_fail "custom test script not found"
 	fi
diff --git a/tests/fsck-tests.sh b/tests/fsck-tests.sh
index 15e3d8d5995c..ed18136f3ab9 100755
--- a/tests/fsck-tests.sh
+++ b/tests/fsck-tests.sh
@@ -64,6 +64,7 @@ run_one_test() {
 			fi
 			_fail "test failed for case $(basename $testname)"
 		fi
+		check_test_results "$RESULTS" "$testname"
 	else
 		# Type 1
 		check_all_images
diff --git a/tests/misc-tests.sh b/tests/misc-tests.sh
index 3b49ab012e78..7da13513641a 100755
--- a/tests/misc-tests.sh
+++ b/tests/misc-tests.sh
@@ -66,6 +66,7 @@ do
 			fi
 			_fail "test failed for case $(basename $i)"
 		fi
+		check_test_results "$RESULTS" "$(basename $i)"
 	fi
 	cd "$TEST_TOP"
 done
diff --git a/tests/mkfs-tests.sh b/tests/mkfs-tests.sh
index 150f094f2303..d6e8bb6702c9 100755
--- a/tests/mkfs-tests.sh
+++ b/tests/mkfs-tests.sh
@@ -61,6 +61,7 @@ do
 			fi
 			_fail "test failed for case $(basename $i)"
 		fi
+		check_test_results "$RESULTS" "$(basename $i)"
 	fi
 	cd "$TEST_TOP"
 done
-- 
2.29.2


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

* Re: [PATCH 2/2] btrfs-progs: tests: check the result log for critical warnings
  2020-11-09  5:39 ` [PATCH 2/2] btrfs-progs: tests: check the result log for critical warnings Qu Wenruo
@ 2021-02-19 14:12   ` David Sterba
  0 siblings, 0 replies; 8+ messages in thread
From: David Sterba @ 2021-02-19 14:12 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: linux-btrfs

On Mon, Nov 09, 2020 at 01:39:52PM +0800, Qu Wenruo wrote:
> Introduce a new function, check_test_results(), for
> misc/fsck/convert/mkfs test cases.
> 
> This function is currently to catch warning message for subpage support,
> but can be later expanded for other usages.

That sounds very useful, thanks.

Added to devel.

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

* Re: [PATCH 1/2] btrfs-progs: check: detect and warn about tree blocks cross 64K page boundary
  2020-11-09  5:39 ` [PATCH 1/2] btrfs-progs: check: detect and warn about tree blocks cross 64K page boundary Qu Wenruo
@ 2021-03-02  8:48   ` Wang Yugui
  2021-03-02 10:14     ` Qu Wenruo
  0 siblings, 1 reply; 8+ messages in thread
From: Wang Yugui @ 2021-03-02  8:48 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: linux-btrfs

Hi, Qu Wenruo

This warning message happen even in new created filesystem on amd64
system.

Should we slicent it before mkfs.btrfs is ready for  64K page system?

The paration is aligned in 1GiB

btrfs-progs: v5.10.x branch

# mkfs.btrfs /dev/sdb1 -f

# btrfs check /dev/sdb1
Opening filesystem to check...
Checking filesystem on /dev/sdb1
UUID: b298271d-6d1d-4792-a579-fb93653aa811
[1/7] checking root items
[2/7] checking extents
WARNING: tree block [5292032, 5308416) crosses 64K page boudnary, may cause problem for 64K page system
WARNING: tree block [5357568, 5373952) crosses 64K page boudnary, may cause problem for 64K page system
[3/7] checking free space tree
[4/7] checking fs roots
[5/7] checking only csums items (without verifying data)
[6/7] checking root refs
[7/7] checking quota groups skipped (not enabled on this FS)
found 147456 bytes used, no error found
total csum bytes: 0
total tree bytes: 147456
total fs tree bytes: 32768
total extent tree bytes: 16384
btree space waste bytes: 140356
file data blocks allocated: 0
 referenced 0

# parted /dev/sdb unit KiB print
Model: TOSHIBA PX05SMQ040 (scsi)
Disk /dev/sdb: 390711384kiB
Sector size (logical/physical): 4096B/4096B
Partition Table: gpt
Disk Flags:

Number  Start         End           Size         File system  Name     Flags
 1      1048576kiB    63963136kiB   62914560kiB  btrfs        primary


Best Regards
Wang Yugui (wangyugui@e16-tech.com)
2021/03/02

> For the incoming subpage support, there is a new requirement for tree
> blocks.
> Tree blocks should not cross 64K page boudnary.
> 
> For current btrfs-progs and kernel, there shouldn't be any causes to
> create such tree blocks.
> 
> But still, we want to detect such tree blocks in the wild before subpage
> support fully lands in upstream.
> 
> This patch will add such check for both lowmem and original mode.
> Currently it's just a warning, since there aren't many users using 64K
> page size yet.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  check/main.c        |  2 ++
>  check/mode-common.h | 18 ++++++++++++++++++
>  check/mode-lowmem.c |  2 ++
>  3 files changed, 22 insertions(+)
> 
> diff --git a/check/main.c b/check/main.c
> index e7996b7c8c0e..0ce9c2f334b4 100644
> --- a/check/main.c
> +++ b/check/main.c
> @@ -5375,6 +5375,8 @@ static int process_extent_item(struct btrfs_root *root,
>  		      num_bytes, gfs_info->sectorsize);
>  		return -EIO;
>  	}
> +	if (metadata)
> +		btrfs_check_subpage_eb_alignment(key.objectid, num_bytes);
>  
>  	memset(&tmpl, 0, sizeof(tmpl));
>  	tmpl.start = key.objectid;
> diff --git a/check/mode-common.h b/check/mode-common.h
> index 4efc07a4f44d..bcda0f53e2c4 100644
> --- a/check/mode-common.h
> +++ b/check/mode-common.h
> @@ -171,4 +171,22 @@ static inline u32 btrfs_type_to_imode(u8 type)
>  
>  	return imode_by_btrfs_type[(type)];
>  }
> +
> +/*
> + * Check tree block alignement for future subpage support on 64K page system.
> + *
> + * Subpage support on 64K page size require one eb to be completely contained
> + * by a page. Not allowing a tree block to cross 64K page boudanry.
> + *
> + * Since subpage support is still under development, this check only provides
> + * warning.
> + */
> +static inline void btrfs_check_subpage_eb_alignment(u64 start, u32 len)
> +{
> +	if (start / BTRFS_MAX_METADATA_BLOCKSIZE !=
> +	    (start + len) / BTRFS_MAX_METADATA_BLOCKSIZE)
> +		warning(
> +"tree block [%llu, %llu) crosses 64K page boudnary, may cause problem for 64K page system",
> +			start, start + len);
> +}
>  #endif
> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
> index 2b689b2abf63..6dbfe829bb7c 100644
> --- a/check/mode-lowmem.c
> +++ b/check/mode-lowmem.c
> @@ -4206,6 +4206,8 @@ static int check_extent_item(struct btrfs_path *path)
>  		      key.objectid, key.objectid + nodesize);
>  		err |= CROSSING_STRIPE_BOUNDARY;
>  	}
> +	if (metadata)
> +		btrfs_check_subpage_eb_alignment(key.objectid, nodesize);
>  
>  	ptr = (unsigned long)(ei + 1);
>  
> -- 
> 2.29.2



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

* Re: [PATCH 1/2] btrfs-progs: check: detect and warn about tree blocks cross 64K page boundary
  2021-03-02  8:48   ` Wang Yugui
@ 2021-03-02 10:14     ` Qu Wenruo
  2021-03-02 10:36       ` Wang Yugui
  0 siblings, 1 reply; 8+ messages in thread
From: Qu Wenruo @ 2021-03-02 10:14 UTC (permalink / raw)
  To: Wang Yugui; +Cc: linux-btrfs



On 2021/3/2 下午4:48, Wang Yugui wrote:
> Hi, Qu Wenruo
> 
> This warning message happen even in new created filesystem on amd64
> system.
> 
> Should we slicent it before mkfs.btrfs is ready for  64K page system?

Nope.

If your fs reports such problem, it means your metadata chunk is not 
properly aligned.

The behavior of chunk allocator alignment has been there for a long long 
time, thus most metadata chunks should already been properly aligned to 64K.

Either btrfs kernel module or mkfs.btrfs has something wrong.


> 
> The paration is aligned in 1GiB
> 
> btrfs-progs: v5.10.x branch
> 
> # mkfs.btrfs /dev/sdb1 -f
> 

And running v5.10.1 I can't reproduce it.

> # btrfs check /dev/sdb1
> Opening filesystem to check...
> Checking filesystem on /dev/sdb1
> UUID: b298271d-6d1d-4792-a579-fb93653aa811
> [1/7] checking root items
> [2/7] checking extents
> WARNING: tree block [5292032, 5308416) crosses 64K page boudnary, may cause problem for 64K page system
> WARNING: tree block [5357568, 5373952) crosses 64K page boudnary, may cause problem for 64K page system

I doubt if you're really using v5.10.x mkfs.btrfs.

As for default btrfs, the metadata chunk is after DATA and SYS chunks, 
this means metadata chunks should only exist after bytenr 16M, but here 
your metadata is only at around 5M.

I strongly doubt your mkfs parameters.

Please attach the full mkfs output.

Thanks,
Qu

> [3/7] checking free space tree
> [4/7] checking fs roots
> [5/7] checking only csums items (without verifying data)
> [6/7] checking root refs
> [7/7] checking quota groups skipped (not enabled on this FS)
> found 147456 bytes used, no error found
> total csum bytes: 0
> total tree bytes: 147456
> total fs tree bytes: 32768
> total extent tree bytes: 16384
> btree space waste bytes: 140356
> file data blocks allocated: 0
>   referenced 0
> 
> # parted /dev/sdb unit KiB print
> Model: TOSHIBA PX05SMQ040 (scsi)
> Disk /dev/sdb: 390711384kiB
> Sector size (logical/physical): 4096B/4096B
> Partition Table: gpt
> Disk Flags:
> 
> Number  Start         End           Size         File system  Name     Flags
>   1      1048576kiB    63963136kiB   62914560kiB  btrfs        primary
> 
> 
> Best Regards
> Wang Yugui (wangyugui@e16-tech.com)
> 2021/03/02
> 
>> For the incoming subpage support, there is a new requirement for tree
>> blocks.
>> Tree blocks should not cross 64K page boudnary.
>>
>> For current btrfs-progs and kernel, there shouldn't be any causes to
>> create such tree blocks.
>>
>> But still, we want to detect such tree blocks in the wild before subpage
>> support fully lands in upstream.
>>
>> This patch will add such check for both lowmem and original mode.
>> Currently it's just a warning, since there aren't many users using 64K
>> page size yet.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>   check/main.c        |  2 ++
>>   check/mode-common.h | 18 ++++++++++++++++++
>>   check/mode-lowmem.c |  2 ++
>>   3 files changed, 22 insertions(+)
>>
>> diff --git a/check/main.c b/check/main.c
>> index e7996b7c8c0e..0ce9c2f334b4 100644
>> --- a/check/main.c
>> +++ b/check/main.c
>> @@ -5375,6 +5375,8 @@ static int process_extent_item(struct btrfs_root *root,
>>   		      num_bytes, gfs_info->sectorsize);
>>   		return -EIO;
>>   	}
>> +	if (metadata)
>> +		btrfs_check_subpage_eb_alignment(key.objectid, num_bytes);
>>   
>>   	memset(&tmpl, 0, sizeof(tmpl));
>>   	tmpl.start = key.objectid;
>> diff --git a/check/mode-common.h b/check/mode-common.h
>> index 4efc07a4f44d..bcda0f53e2c4 100644
>> --- a/check/mode-common.h
>> +++ b/check/mode-common.h
>> @@ -171,4 +171,22 @@ static inline u32 btrfs_type_to_imode(u8 type)
>>   
>>   	return imode_by_btrfs_type[(type)];
>>   }
>> +
>> +/*
>> + * Check tree block alignement for future subpage support on 64K page system.
>> + *
>> + * Subpage support on 64K page size require one eb to be completely contained
>> + * by a page. Not allowing a tree block to cross 64K page boudanry.
>> + *
>> + * Since subpage support is still under development, this check only provides
>> + * warning.
>> + */
>> +static inline void btrfs_check_subpage_eb_alignment(u64 start, u32 len)
>> +{
>> +	if (start / BTRFS_MAX_METADATA_BLOCKSIZE !=
>> +	    (start + len) / BTRFS_MAX_METADATA_BLOCKSIZE)
>> +		warning(
>> +"tree block [%llu, %llu) crosses 64K page boudnary, may cause problem for 64K page system",
>> +			start, start + len);
>> +}
>>   #endif
>> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
>> index 2b689b2abf63..6dbfe829bb7c 100644
>> --- a/check/mode-lowmem.c
>> +++ b/check/mode-lowmem.c
>> @@ -4206,6 +4206,8 @@ static int check_extent_item(struct btrfs_path *path)
>>   		      key.objectid, key.objectid + nodesize);
>>   		err |= CROSSING_STRIPE_BOUNDARY;
>>   	}
>> +	if (metadata)
>> +		btrfs_check_subpage_eb_alignment(key.objectid, nodesize);
>>   
>>   	ptr = (unsigned long)(ei + 1);
>>   
>> -- 
>> 2.29.2
> 
> 


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

* Re: [PATCH 1/2] btrfs-progs: check: detect and warn about tree blocks cross 64K page boundary
  2021-03-02 10:14     ` Qu Wenruo
@ 2021-03-02 10:36       ` Wang Yugui
  2021-03-06  0:33         ` Qu Wenruo
  0 siblings, 1 reply; 8+ messages in thread
From: Wang Yugui @ 2021-03-02 10:36 UTC (permalink / raw)
  To: Qu Wenruo; +Cc: linux-btrfs

Hi,

This is a full mkfs.btrtfs log

[root@T7610 ~]# mkfs.btrfs -O no-holes -R free-space-tree /dev/sdb1 -f
btrfs-progs v5.10.1
See http://btrfs.wiki.kernel.org for more information.

Detected a SSD, turning off metadata duplication.  Mkfs with -m dup if you want to force metadata duplication.
Label:              (null)
UUID:               8c745f77-3cdb-45f0-9b67-c69a9bd491a1
Node size:          16384
Sector size:        4096
Filesystem size:    60.00GiB
Block group profiles:
  Data:             single            8.00MiB
  Metadata:         single            8.00MiB
  System:           single            4.00MiB
SSD detected:       yes
Incompat features:  extref, skinny-metadata, no-holes
Runtime features:   free-space-tree
Checksum:           crc32c
Number of devices:  1
Devices:
   ID        SIZE  PATH
    1    60.00GiB  /dev/sdb1

[root@T7610 ~]# btrfs check /dev/sdb1
Opening filesystem to check...
Checking filesystem on /dev/sdb1
UUID: 8c745f77-3cdb-45f0-9b67-c69a9bd491a1
[1/7] checking root items
[2/7] checking extents
WARNING: tree block [5292032, 5308416) crosses 64K page boudnary, may cause problem for 64K page system
WARNING: tree block [5357568, 5373952) crosses 64K page boudnary, may cause problem for 64K page system
[3/7] checking free space tree
[4/7] checking fs roots
[5/7] checking only csums items (without verifying data)
[6/7] checking root refs
[7/7] checking quota groups skipped (not enabled on this FS)
found 147456 bytes used, no error found
total csum bytes: 0
total tree bytes: 147456
total fs tree bytes: 32768
total extent tree bytes: 16384
btree space waste bytes: 140356
file data blocks allocated: 0
 referenced 0
[root@T7610 ~]# mount /dev/sdb1 /mnt/scratch/
[root@T7610 ~]# btrfs filesystem usage /mnt/scratch
Overall:
    Device size:                  60.00GiB
    Device allocated:             20.00MiB
    Device unallocated:           59.98GiB
    Device missing:                  0.00B
    Used:                        144.00KiB
    Free (estimated):             59.99GiB      (min: 59.99GiB)
    Free (statfs, df):            59.99GiB
    Data ratio:                       1.00
    Metadata ratio:                   1.00
    Global reserve:                3.25MiB      (used: 0.00B)
    Multiple profiles:                  no

Data,single: Size:8.00MiB, Used:0.00B (0.00%)
   /dev/sdb1       8.00MiB

Metadata,single: Size:8.00MiB, Used:128.00KiB (1.56%)
   /dev/sdb1       8.00MiB

System,single: Size:4.00MiB, Used:16.00KiB (0.39%)
   /dev/sdb1       4.00MiB

Unallocated:
   /dev/sdb1      59.98GiB


[root@T7610 ~]# parted /dev/sdb unit s print
Model: TOSHIBA PX05SMQ040 (scsi)
Disk /dev/sdb: 97677846s
Sector size (logical/physical): 4096B/4096B
Partition Table: gpt
Disk Flags:

Number  Start      End        Size       File system  Name     Flags
 1      262144s    15990783s  15728640s  btrfs        primary
 2      15990784s  31719423s  15728640s               primary
 3      31719424s  47448063s  15728640s               primary
 4      47448064s  63176703s  15728640s               primary
 5      63176704s  78905343s  15728640s               primary

Best Regards
Wang Yugui (wangyugui@e16-tech.com)
2021/03/02

> 
> 
> On 2021/3/2 下午4:48, Wang Yugui wrote:
> > Hi, Qu Wenruo
> >
> > This warning message happen even in new created filesystem on amd64
> > system.
> >
> > Should we slicent it before mkfs.btrfs is ready for  64K page system?
> 
> Nope.
> 
> If your fs reports such problem, it means your metadata chunk is not properly aligned.
> 
> The behavior of chunk allocator alignment has been there for a long long time, thus most metadata chunks should already been properly aligned to 64K.
> 
> Either btrfs kernel module or mkfs.btrfs has something wrong.
> 
> 
> >
> > The paration is aligned in 1GiB
> >
> > btrfs-progs: v5.10.x branch
> >
> > # mkfs.btrfs /dev/sdb1 -f
> > 
> And running v5.10.1 I can't reproduce it.
> 
> > # btrfs check /dev/sdb1
> > Opening filesystem to check...
> > Checking filesystem on /dev/sdb1
> > UUID: b298271d-6d1d-4792-a579-fb93653aa811
> > [1/7] checking root items
> > [2/7] checking extents
> > WARNING: tree block [5292032, 5308416) crosses 64K page boudnary, may cause problem for 64K page system
> > WARNING: tree block [5357568, 5373952) crosses 64K page boudnary, may cause problem for 64K page system
> 
> I doubt if you're really using v5.10.x mkfs.btrfs.
> 
> As for default btrfs, the metadata chunk is after DATA and SYS chunks, this means metadata chunks should only exist after bytenr 16M, but here your metadata is only at around 5M.
> 
> I strongly doubt your mkfs parameters.
> 
> Please attach the full mkfs output.
> 
> Thanks,
> Qu
> 
> > [3/7] checking free space tree
> > [4/7] checking fs roots
> > [5/7] checking only csums items (without verifying data)
> > [6/7] checking root refs
> > [7/7] checking quota groups skipped (not enabled on this FS)
> > found 147456 bytes used, no error found
> > total csum bytes: 0
> > total tree bytes: 147456
> > total fs tree bytes: 32768
> > total extent tree bytes: 16384
> > btree space waste bytes: 140356
> > file data blocks allocated: 0
> >   referenced 0
> >
> > # parted /dev/sdb unit KiB print
> > Model: TOSHIBA PX05SMQ040 (scsi)
> > Disk /dev/sdb: 390711384kiB
> > Sector size (logical/physical): 4096B/4096B
> > Partition Table: gpt
> > Disk Flags:
> >
> > Number  Start         End           Size         File system  Name     Flags
> >   1      1048576kiB    63963136kiB   62914560kiB  btrfs        primary
> >
> >
> > Best Regards
> > Wang Yugui (wangyugui@e16-tech.com)
> > 2021/03/02
> >
> >> For the incoming subpage support, there is a new requirement for tree
> >> blocks.
> >> Tree blocks should not cross 64K page boudnary.
> >>
> >> For current btrfs-progs and kernel, there shouldn't be any causes to
> >> create such tree blocks.
> >>
> >> But still, we want to detect such tree blocks in the wild before subpage
> >> support fully lands in upstream.
> >>
> >> This patch will add such check for both lowmem and original mode.
> >> Currently it's just a warning, since there aren't many users using 64K
> >> page size yet.
> >>
> >> Signed-off-by: Qu Wenruo <wqu@suse.com>
> >> ---
> >>   check/main.c        |  2 ++
> >>   check/mode-common.h | 18 ++++++++++++++++++
> >>   check/mode-lowmem.c |  2 ++
> >>   3 files changed, 22 insertions(+)
> >>
> >> diff --git a/check/main.c b/check/main.c
> >> index e7996b7c8c0e..0ce9c2f334b4 100644
> >> --- a/check/main.c
> >> +++ b/check/main.c
> >> @@ -5375,6 +5375,8 @@ static int process_extent_item(struct btrfs_root *root,
> >>   		      num_bytes, gfs_info->sectorsize);
> >>   		return -EIO;
> >>   	}
> >> +	if (metadata)
> >> +		btrfs_check_subpage_eb_alignment(key.objectid, num_bytes);
> >>  >>   	memset(&tmpl, 0, sizeof(tmpl));
> >>   	tmpl.start = key.objectid;
> >> diff --git a/check/mode-common.h b/check/mode-common.h
> >> index 4efc07a4f44d..bcda0f53e2c4 100644
> >> --- a/check/mode-common.h
> >> +++ b/check/mode-common.h
> >> @@ -171,4 +171,22 @@ static inline u32 btrfs_type_to_imode(u8 type)
> >>  >>   	return imode_by_btrfs_type[(type)];
> >>   }
> >> +
> >> +/*
> >> + * Check tree block alignement for future subpage support on 64K page system.
> >> + *
> >> + * Subpage support on 64K page size require one eb to be completely contained
> >> + * by a page. Not allowing a tree block to cross 64K page boudanry.
> >> + *
> >> + * Since subpage support is still under development, this check only provides
> >> + * warning.
> >> + */
> >> +static inline void btrfs_check_subpage_eb_alignment(u64 start, u32 len)
> >> +{
> >> +	if (start / BTRFS_MAX_METADATA_BLOCKSIZE !=
> >> +	    (start + len) / BTRFS_MAX_METADATA_BLOCKSIZE)
> >> +		warning(
> >> +"tree block [%llu, %llu) crosses 64K page boudnary, may cause problem for 64K page system",
> >> +			start, start + len);
> >> +}
> >>   #endif
> >> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
> >> index 2b689b2abf63..6dbfe829bb7c 100644
> >> --- a/check/mode-lowmem.c
> >> +++ b/check/mode-lowmem.c
> >> @@ -4206,6 +4206,8 @@ static int check_extent_item(struct btrfs_path *path)
> >>   		      key.objectid, key.objectid + nodesize);
> >>   		err |= CROSSING_STRIPE_BOUNDARY;
> >>   	}
> >> +	if (metadata)
> >> +		btrfs_check_subpage_eb_alignment(key.objectid, nodesize);
> >>  >>   	ptr = (unsigned long)(ei + 1);
> >>  >> --
> >> 2.29.2
> >
> > 



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

* Re: [PATCH 1/2] btrfs-progs: check: detect and warn about tree blocks cross 64K page boundary
  2021-03-02 10:36       ` Wang Yugui
@ 2021-03-06  0:33         ` Qu Wenruo
  0 siblings, 0 replies; 8+ messages in thread
From: Qu Wenruo @ 2021-03-06  0:33 UTC (permalink / raw)
  To: Wang Yugui, Qu Wenruo; +Cc: linux-btrfs



On 2021/3/2 下午6:36, Wang Yugui wrote:
> Hi,
>
> This is a full mkfs.btrtfs log
>
> [root@T7610 ~]# mkfs.btrfs -O no-holes -R free-space-tree /dev/sdb1 -f
> btrfs-progs v5.10.1
> See http://btrfs.wiki.kernel.org for more information.
>
> Detected a SSD, turning off metadata duplication.  Mkfs with -m dup if you want to force metadata duplication.
> Label:              (null)
> UUID:               8c745f77-3cdb-45f0-9b67-c69a9bd491a1
> Node size:          16384
> Sector size:        4096
> Filesystem size:    60.00GiB
> Block group profiles:
>    Data:             single            8.00MiB
>    Metadata:         single            8.00MiB
>    System:           single            4.00MiB
> SSD detected:       yes
> Incompat features:  extref, skinny-metadata, no-holes
> Runtime features:   free-space-tree
> Checksum:           crc32c
> Number of devices:  1
> Devices:
>     ID        SIZE  PATH
>      1    60.00GiB  /dev/sdb1
>
> [root@T7610 ~]# btrfs check /dev/sdb1
> Opening filesystem to check...
> Checking filesystem on /dev/sdb1
> UUID: 8c745f77-3cdb-45f0-9b67-c69a9bd491a1
> [1/7] checking root items
> [2/7] checking extents
> WARNING: tree block [5292032, 5308416) crosses 64K page boudnary, may cause problem for 64K page system
> WARNING: tree block [5357568, 5373952) crosses 64K page boudnary, may cause problem for 64K page system

This seems to be a false alert.

5292032 % 65536 = 49152, which is the last slot for 16K, and it doesn't
cross 64K page boundary.

The same goes for bytenr 5357568.

I'll fix it soon.

Thanks,
Qu
> [3/7] checking free space tree
> [4/7] checking fs roots
> [5/7] checking only csums items (without verifying data)
> [6/7] checking root refs
> [7/7] checking quota groups skipped (not enabled on this FS)
> found 147456 bytes used, no error found
> total csum bytes: 0
> total tree bytes: 147456
> total fs tree bytes: 32768
> total extent tree bytes: 16384
> btree space waste bytes: 140356
> file data blocks allocated: 0
>   referenced 0
> [root@T7610 ~]# mount /dev/sdb1 /mnt/scratch/
> [root@T7610 ~]# btrfs filesystem usage /mnt/scratch
> Overall:
>      Device size:                  60.00GiB
>      Device allocated:             20.00MiB
>      Device unallocated:           59.98GiB
>      Device missing:                  0.00B
>      Used:                        144.00KiB
>      Free (estimated):             59.99GiB      (min: 59.99GiB)
>      Free (statfs, df):            59.99GiB
>      Data ratio:                       1.00
>      Metadata ratio:                   1.00
>      Global reserve:                3.25MiB      (used: 0.00B)
>      Multiple profiles:                  no
>
> Data,single: Size:8.00MiB, Used:0.00B (0.00%)
>     /dev/sdb1       8.00MiB
>
> Metadata,single: Size:8.00MiB, Used:128.00KiB (1.56%)
>     /dev/sdb1       8.00MiB
>
> System,single: Size:4.00MiB, Used:16.00KiB (0.39%)
>     /dev/sdb1       4.00MiB
>
> Unallocated:
>     /dev/sdb1      59.98GiB
>
>
> [root@T7610 ~]# parted /dev/sdb unit s print
> Model: TOSHIBA PX05SMQ040 (scsi)
> Disk /dev/sdb: 97677846s
> Sector size (logical/physical): 4096B/4096B
> Partition Table: gpt
> Disk Flags:
>
> Number  Start      End        Size       File system  Name     Flags
>   1      262144s    15990783s  15728640s  btrfs        primary
>   2      15990784s  31719423s  15728640s               primary
>   3      31719424s  47448063s  15728640s               primary
>   4      47448064s  63176703s  15728640s               primary
>   5      63176704s  78905343s  15728640s               primary
>
> Best Regards
> Wang Yugui (wangyugui@e16-tech.com)
> 2021/03/02
>
>>
>>
>> On 2021/3/2 下午4:48, Wang Yugui wrote:
>>> Hi, Qu Wenruo
>>>
>>> This warning message happen even in new created filesystem on amd64
>>> system.
>>>
>>> Should we slicent it before mkfs.btrfs is ready for  64K page system?
>>
>> Nope.
>>
>> If your fs reports such problem, it means your metadata chunk is not properly aligned.
>>
>> The behavior of chunk allocator alignment has been there for a long long time, thus most metadata chunks should already been properly aligned to 64K.
>>
>> Either btrfs kernel module or mkfs.btrfs has something wrong.
>>
>>
>>>
>>> The paration is aligned in 1GiB
>>>
>>> btrfs-progs: v5.10.x branch
>>>
>>> # mkfs.btrfs /dev/sdb1 -f
>>>
>> And running v5.10.1 I can't reproduce it.
>>
>>> # btrfs check /dev/sdb1
>>> Opening filesystem to check...
>>> Checking filesystem on /dev/sdb1
>>> UUID: b298271d-6d1d-4792-a579-fb93653aa811
>>> [1/7] checking root items
>>> [2/7] checking extents
>>> WARNING: tree block [5292032, 5308416) crosses 64K page boudnary, may cause problem for 64K page system
>>> WARNING: tree block [5357568, 5373952) crosses 64K page boudnary, may cause problem for 64K page system
>>
>> I doubt if you're really using v5.10.x mkfs.btrfs.
>>
>> As for default btrfs, the metadata chunk is after DATA and SYS chunks, this means metadata chunks should only exist after bytenr 16M, but here your metadata is only at around 5M.
>>
>> I strongly doubt your mkfs parameters.
>>
>> Please attach the full mkfs output.
>>
>> Thanks,
>> Qu
>>
>>> [3/7] checking free space tree
>>> [4/7] checking fs roots
>>> [5/7] checking only csums items (without verifying data)
>>> [6/7] checking root refs
>>> [7/7] checking quota groups skipped (not enabled on this FS)
>>> found 147456 bytes used, no error found
>>> total csum bytes: 0
>>> total tree bytes: 147456
>>> total fs tree bytes: 32768
>>> total extent tree bytes: 16384
>>> btree space waste bytes: 140356
>>> file data blocks allocated: 0
>>>    referenced 0
>>>
>>> # parted /dev/sdb unit KiB print
>>> Model: TOSHIBA PX05SMQ040 (scsi)
>>> Disk /dev/sdb: 390711384kiB
>>> Sector size (logical/physical): 4096B/4096B
>>> Partition Table: gpt
>>> Disk Flags:
>>>
>>> Number  Start         End           Size         File system  Name     Flags
>>>    1      1048576kiB    63963136kiB   62914560kiB  btrfs        primary
>>>
>>>
>>> Best Regards
>>> Wang Yugui (wangyugui@e16-tech.com)
>>> 2021/03/02
>>>
>>>> For the incoming subpage support, there is a new requirement for tree
>>>> blocks.
>>>> Tree blocks should not cross 64K page boudnary.
>>>>
>>>> For current btrfs-progs and kernel, there shouldn't be any causes to
>>>> create such tree blocks.
>>>>
>>>> But still, we want to detect such tree blocks in the wild before subpage
>>>> support fully lands in upstream.
>>>>
>>>> This patch will add such check for both lowmem and original mode.
>>>> Currently it's just a warning, since there aren't many users using 64K
>>>> page size yet.
>>>>
>>>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>>>> ---
>>>>    check/main.c        |  2 ++
>>>>    check/mode-common.h | 18 ++++++++++++++++++
>>>>    check/mode-lowmem.c |  2 ++
>>>>    3 files changed, 22 insertions(+)
>>>>
>>>> diff --git a/check/main.c b/check/main.c
>>>> index e7996b7c8c0e..0ce9c2f334b4 100644
>>>> --- a/check/main.c
>>>> +++ b/check/main.c
>>>> @@ -5375,6 +5375,8 @@ static int process_extent_item(struct btrfs_root *root,
>>>>    		      num_bytes, gfs_info->sectorsize);
>>>>    		return -EIO;
>>>>    	}
>>>> +	if (metadata)
>>>> +		btrfs_check_subpage_eb_alignment(key.objectid, num_bytes);
>>>>   >>   	memset(&tmpl, 0, sizeof(tmpl));
>>>>    	tmpl.start = key.objectid;
>>>> diff --git a/check/mode-common.h b/check/mode-common.h
>>>> index 4efc07a4f44d..bcda0f53e2c4 100644
>>>> --- a/check/mode-common.h
>>>> +++ b/check/mode-common.h
>>>> @@ -171,4 +171,22 @@ static inline u32 btrfs_type_to_imode(u8 type)
>>>>   >>   	return imode_by_btrfs_type[(type)];
>>>>    }
>>>> +
>>>> +/*
>>>> + * Check tree block alignement for future subpage support on 64K page system.
>>>> + *
>>>> + * Subpage support on 64K page size require one eb to be completely contained
>>>> + * by a page. Not allowing a tree block to cross 64K page boudanry.
>>>> + *
>>>> + * Since subpage support is still under development, this check only provides
>>>> + * warning.
>>>> + */
>>>> +static inline void btrfs_check_subpage_eb_alignment(u64 start, u32 len)
>>>> +{
>>>> +	if (start / BTRFS_MAX_METADATA_BLOCKSIZE !=
>>>> +	    (start + len) / BTRFS_MAX_METADATA_BLOCKSIZE)
>>>> +		warning(
>>>> +"tree block [%llu, %llu) crosses 64K page boudnary, may cause problem for 64K page system",
>>>> +			start, start + len);
>>>> +}
>>>>    #endif
>>>> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
>>>> index 2b689b2abf63..6dbfe829bb7c 100644
>>>> --- a/check/mode-lowmem.c
>>>> +++ b/check/mode-lowmem.c
>>>> @@ -4206,6 +4206,8 @@ static int check_extent_item(struct btrfs_path *path)
>>>>    		      key.objectid, key.objectid + nodesize);
>>>>    		err |= CROSSING_STRIPE_BOUNDARY;
>>>>    	}
>>>> +	if (metadata)
>>>> +		btrfs_check_subpage_eb_alignment(key.objectid, nodesize);
>>>>   >>   	ptr = (unsigned long)(ei + 1);
>>>>   >> --
>>>> 2.29.2
>>>
>>>
>
>

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

end of thread, other threads:[~2021-03-06  0:34 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-09  5:39 [PATCH 0/2] btrfs-progs: add new precaution check for incoming subpage support Qu Wenruo
2020-11-09  5:39 ` [PATCH 1/2] btrfs-progs: check: detect and warn about tree blocks cross 64K page boundary Qu Wenruo
2021-03-02  8:48   ` Wang Yugui
2021-03-02 10:14     ` Qu Wenruo
2021-03-02 10:36       ` Wang Yugui
2021-03-06  0:33         ` Qu Wenruo
2020-11-09  5:39 ` [PATCH 2/2] btrfs-progs: tests: check the result log for critical warnings Qu Wenruo
2021-02-19 14:12   ` David Sterba

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).