[PATCH 0/3] btrfs: apply strict alignment checks to extent maps

[PATCH 0/3] btrfs: apply strict alignment checks to extent maps

[Qu Wenruo]

Although we already have strict checks on file extent items from
tree-checker, we never do proper alignment checks for extent maps.

The reason is mostly due to the failure of self tests and how hard it is
to touch them, especially for the inode self tests.

I have to say the inode self test is really something, the extent maps
of the self test makes no sense, and would be rejected by tree-checker if
they show up in the real world.
Thankfully only the first few ones are invalid, the remaining ones are
totally fine.

The comments are not any better, after the first line, there are no more
aligned number at all, and the numbers are not offset by 1, but 3 or
even more.
Considering we're using decimals for most of our dump-tree and comments,
no one is really going to note the wrong numbers until one throw them
into python or whatever calculator one prefers.

The series will mostly rework the inode self tests file extent item
layout so that they represent the real world better, and update the
comments and make the poor person who needs to update that selftest
suffer less in the future.

Then address the minor problem in the extent-map selftest where
fs_info->sectorsize is never properly populated for the 4K based self
test.

With all those done, we can finally put a proper alignment check into 
validate_extent_map() which is called for every new, merged or replaced
extent map.

Qu Wenruo (3):
  btrfs: tests: remove invalid file extent map tests
  btrfs: tests: prepare extent map tests for strict alignment checks
  btrfs: add strict extent map alignment checks

 fs/btrfs/extent_map.c             |  12 ++++
 fs/btrfs/tests/extent-map-tests.c |  16 +++--
 fs/btrfs/tests/inode-tests.c      | 111 +++++++++++++++++-------------
 3 files changed, 87 insertions(+), 52 deletions(-)

-- 
2.52.0

[PATCH 1/3] btrfs: tests: remove invalid file extent map tests

[Qu Wenruo]

In the inode self tests, there are several problems:

- Several invalid cases that will be rejected by tree-checkers
  E.g. hole range [4K, 4K + 4) is completely invalid.

  Only inlined extent maps can have an unaligned ram_bytes, and even for
  that case, the generaeted extent map will use sectorsize as em->len.

- Manually inserted hole after an inlined extent map
  The kernel never does this by itself, the current btrfs_get_extent()
  will only return a single inlined extent map that covers the first
  block.

- Completely incorrect numbers in the comment
  E.g. 12291 no matter if you add or dec 1, is not aligned to 4K.
  The properly number for 12K is 12288, I don't know why there is even a
  diff of 3, and this completely doesn't match the extent map we
  inserted later.

- Super hard to modify sequence in setup_file_extents()
  If some unfortunate person, just like me, needs to modify
  setup_file_extents(), good luck not screwing up the file offset.

Fix them by:

- Remove invalid unaligned extent maps
  This mostly means remove the [4K, 4K + 4) hole case.
  The remaining ones are already properly aligned.

  This slightly changes the on-disk data extent allocation, with that
  removed, the regular extents at [4K, 8K) and [8K , 12K) can be merged.

  So also add a 4K gap between those two data extents to prevent em
  merge.

- Remove the manual insert of the implied hole after an inlined extent
  Just like what the kernel is doning for inlined extents in the real
  world.

- Update the commit using proper numbers with Kilo suffix
  Since there is no unaligned range except the first inlined one, we can
  always use numbers with 'K' suffix, which is way more easier to read,
  and will always be aligned to 1024 at least.

- Add extra ASSERT()s and comments in setup_file_extents()
  The ASSERT()s are used to indicate the current file offset.
  The extra comments are for the basic info of the file extent.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/tests/inode-tests.c | 111 ++++++++++++++++++++---------------
 1 file changed, 64 insertions(+), 47 deletions(-)

diff --git a/fs/btrfs/tests/inode-tests.c b/fs/btrfs/tests/inode-tests.c
index a4c2b7748b95..58b75bdfed9e 100644
--- a/fs/btrfs/tests/inode-tests.c
+++ b/fs/btrfs/tests/inode-tests.c
@@ -81,17 +81,20 @@ static void insert_inode_item_key(struct btrfs_root *root)
  * diagram of how the extents will look though this may not be possible we still
  * want to make sure everything acts normally (the last number is not inclusive)
  *
- * [0  - 6][     6 - 4096     ][ 4096 - 4100][4100 - 8195][8195  -  12291]
- * [inline][hole but no extent][    hole    ][   regular ][regular1 split]
+ * The numbers are using 4K fs block size as an example, the real test will scale
+ * all the extent maps (except the inlined one) according to the block size.
  *
- * [12291 - 16387][16387 - 24579][24579 - 28675][ 28675 - 32771][32771 - 36867 ]
- * [    hole    ][regular1 split][   prealloc ][   prealloc1  ][prealloc1 written]
+ * [ 0  - 6 ][ 6 - 4K       ][ 4K - 8K ][ 8K -  12K      ]
+ * [ inline ][ implied hole ][ regular ][ regular1 split ]
  *
- * [36867 - 45059][45059 - 53251][53251 - 57347][57347 - 61443][61443- 69635]
- * [  prealloc1  ][ compressed  ][ compressed1 ][    regular  ][ compressed1]
+ * [ 12K - 16K ][ 16K - 24K      ][ 24K - 28K ][ 28K - 32K ][ 32K - 36K         ]
+ * [ hole      ][ regular1 split ][ prealloc  ][ prealloc1 ][ prealloc1 written ]
  *
- * [69635-73731][   73731 - 86019   ][86019-90115]
- * [  regular  ][ hole but no extent][  regular  ]
+ * [ 36K - 44K ][ 44K - 52K  ][ 52K - 56K   ][ 56K - 60K ][ 60K - 68 K  ]
+ * [ prealloc1 ][ compressed ][ compressed1 ][ regular   ][ compressed1 ]
+ *
+ * [ 68K - 72K ][ 72K - 84K          ][ 84K - 88K ]
+ * [  regular  ][ hole but no extent ][ regular   ]
  */
 static void setup_file_extents(struct btrfs_root *root, u32 sectorsize)
 {
@@ -100,40 +103,49 @@ static void setup_file_extents(struct btrfs_root *root, u32 sectorsize)
 	u64 offset = 0;
 
 	/*
+	 * Start 0, length 6, inlined.
+	 *
 	 * Tree-checker has strict limits on inline extents that they can only
 	 * exist at file offset 0, thus we can only have one inline file extent
 	 * at most.
 	 */
+	ASSERT(offset == 0);
 	insert_extent(root, offset, 6, 6, 0, 0, 0, BTRFS_FILE_EXTENT_INLINE, 0,
 		      slot);
 	slot++;
 	offset = sectorsize;
 
-	/* Now another hole */
-	insert_extent(root, offset, 4, 4, 0, 0, 0, BTRFS_FILE_EXTENT_REG, 0,
-		      slot);
+	/* Start 1 * blocksize, length 1 * blocksize, regular */
+	ASSERT(offset == 1 * sectorsize);
+	insert_extent(root, offset, sectorsize, sectorsize, 0,
+		      disk_bytenr, sectorsize, BTRFS_FILE_EXTENT_REG, 0, slot);
 	slot++;
-	offset += 4;
 
-	/* Now for a regular extent */
-	insert_extent(root, offset, sectorsize - 1, sectorsize - 1, 0,
-		      disk_bytenr, sectorsize - 1, BTRFS_FILE_EXTENT_REG, 0, slot);
-	slot++;
-	disk_bytenr += sectorsize;
-	offset += sectorsize - 1;
+	/* We don't want the regular em got merged with the next one. */
+	disk_bytenr += 2 * sectorsize;
+	offset += sectorsize;
 
 	/*
+	 * Start 2 * blocksize, length 1 * blocksize, regular.
+	 *
 	 * Now for 3 extents that were split from a hole punch so we test
 	 * offsets properly.
 	 */
+	ASSERT(offset == 2 * sectorsize);
 	insert_extent(root, offset, sectorsize, 4 * sectorsize, 0, disk_bytenr,
 		      4 * sectorsize, BTRFS_FILE_EXTENT_REG, 0, slot);
 	slot++;
 	offset += sectorsize;
+
+	/* Start 3 * blocksize, length 1 * blocksize, regular, explicit hole. */
+	ASSERT(offset == 3 * sectorsize);
 	insert_extent(root, offset, sectorsize, sectorsize, 0, 0, 0,
 		      BTRFS_FILE_EXTENT_REG, 0, slot);
 	slot++;
 	offset += sectorsize;
+
+	/* Start 4 * blocksize, length 2 * blocksize, regular. */
+	ASSERT(offset == 4 * sectorsize);
 	insert_extent(root, offset, 2 * sectorsize, 4 * sectorsize,
 		      2 * sectorsize, disk_bytenr, 4 * sectorsize,
 		      BTRFS_FILE_EXTENT_REG, 0, slot);
@@ -141,7 +153,8 @@ static void setup_file_extents(struct btrfs_root *root, u32 sectorsize)
 	offset += 2 * sectorsize;
 	disk_bytenr += 4 * sectorsize;
 
-	/* Now for a unwritten prealloc extent */
+	/* Start 6 * blocksize, length 1 * blocksize, preallocated. */
+	ASSERT(offset == 6 * sectorsize);
 	insert_extent(root, offset, sectorsize, sectorsize, 0, disk_bytenr,
 		sectorsize, BTRFS_FILE_EXTENT_PREALLOC, 0, slot);
 	slot++;
@@ -154,19 +167,28 @@ static void setup_file_extents(struct btrfs_root *root, u32 sectorsize)
 	disk_bytenr += 2 * sectorsize;
 
 	/*
+	 * Start 7 * blocksize, length 1 * blocksize, prealloc.
+	 *
 	 * Now for a partially written prealloc extent, basically the same as
 	 * the hole punch example above.  Ram_bytes never changes when you mark
 	 * extents written btw.
 	 */
+	ASSERT(offset == 7 * sectorsize);
 	insert_extent(root, offset, sectorsize, 4 * sectorsize, 0, disk_bytenr,
 		      4 * sectorsize, BTRFS_FILE_EXTENT_PREALLOC, 0, slot);
 	slot++;
 	offset += sectorsize;
+
+	/* Start 8 * blocksize, length 1 * blocksize, regular. */
+	ASSERT(offset == 8 * sectorsize);
 	insert_extent(root, offset, sectorsize, 4 * sectorsize, sectorsize,
 		      disk_bytenr, 4 * sectorsize, BTRFS_FILE_EXTENT_REG, 0,
 		      slot);
 	slot++;
 	offset += sectorsize;
+
+	/* Start 9 * blocksize, length 2 * blocksize, prealloc. */
+	ASSERT(offset == 9 * sectorsize);
 	insert_extent(root, offset, 2 * sectorsize, 4 * sectorsize,
 		      2 * sectorsize, disk_bytenr, 4 * sectorsize,
 		      BTRFS_FILE_EXTENT_PREALLOC, 0, slot);
@@ -174,7 +196,8 @@ static void setup_file_extents(struct btrfs_root *root, u32 sectorsize)
 	offset += 2 * sectorsize;
 	disk_bytenr += 4 * sectorsize;
 
-	/* Now a normal compressed extent */
+	/* Start 11 * blocksize, length 2 * blocksize, regular . */
+	ASSERT(offset == 11 * sectorsize);
 	insert_extent(root, offset, 2 * sectorsize, 2 * sectorsize, 0,
 		      disk_bytenr, sectorsize, BTRFS_FILE_EXTENT_REG,
 		      BTRFS_COMPRESS_ZLIB, slot);
@@ -183,17 +206,24 @@ static void setup_file_extents(struct btrfs_root *root, u32 sectorsize)
 	/* No merges */
 	disk_bytenr += 2 * sectorsize;
 
-	/* Now a split compressed extent */
+	/* Start 13 * blocksize, length 1 * blocksize, regular. */
+	ASSERT(offset == 13 * sectorsize);
 	insert_extent(root, offset, sectorsize, 4 * sectorsize, 0, disk_bytenr,
 		      sectorsize, BTRFS_FILE_EXTENT_REG,
 		      BTRFS_COMPRESS_ZLIB, slot);
 	slot++;
 	offset += sectorsize;
+
+	/* Start 14 * blocksize, length 1 * blocksize, regular. */
+	ASSERT(offset == 14 * sectorsize);
 	insert_extent(root, offset, sectorsize, sectorsize, 0,
 		      disk_bytenr + sectorsize, sectorsize,
 		      BTRFS_FILE_EXTENT_REG, 0, slot);
 	slot++;
 	offset += sectorsize;
+
+	/* Start 15 * blocksize, length 2 * blocksize, regular. */
+	ASSERT(offset == 15 * sectorsize);
 	insert_extent(root, offset, 2 * sectorsize, 4 * sectorsize,
 		      2 * sectorsize, disk_bytenr, sectorsize,
 		      BTRFS_FILE_EXTENT_REG, BTRFS_COMPRESS_ZLIB, slot);
@@ -201,12 +231,21 @@ static void setup_file_extents(struct btrfs_root *root, u32 sectorsize)
 	offset += 2 * sectorsize;
 	disk_bytenr += 2 * sectorsize;
 
-	/* Now extents that have a hole but no hole extent */
+	/* Start 17 * blocksize, length 1 * blocksize, regular. */
+	ASSERT(offset == 17 * sectorsize);
 	insert_extent(root, offset, sectorsize, sectorsize, 0, disk_bytenr,
 		      sectorsize, BTRFS_FILE_EXTENT_REG, 0, slot);
 	slot++;
 	offset += 4 * sectorsize;
 	disk_bytenr += sectorsize;
+
+	/*
+	 * Start 18 * blocksize, length 3 * blocksize, implied hole (aka no
+	 * file extent item).
+	 *
+	 * Start 21 * blocksize, length 1 * blocksize, regular.
+	 */
+	ASSERT(offset == 21 * sectorsize);
 	insert_extent(root, offset, sectorsize, sectorsize, 0, disk_bytenr,
 		      sectorsize, BTRFS_FILE_EXTENT_REG, 0, slot);
 }
@@ -316,28 +355,6 @@ static noinline int test_btrfs_get_extent(u32 sectorsize, u32 nodesize)
 	offset = em->start + em->len;
 	btrfs_free_extent_map(em);
 
-	em = btrfs_get_extent(BTRFS_I(inode), NULL, offset, sectorsize);
-	if (IS_ERR(em)) {
-		test_err("got an error when we shouldn't have");
-		goto out;
-	}
-	if (em->disk_bytenr != EXTENT_MAP_HOLE) {
-		test_err("expected a hole, got %llu", em->disk_bytenr);
-		goto out;
-	}
-	if (em->start != offset || em->len != 4) {
-		test_err(
-	"unexpected extent wanted start %llu len 4, got start %llu len %llu",
-			offset, em->start, em->len);
-		goto out;
-	}
-	if (em->flags != 0) {
-		test_err("unexpected flags set, want 0 have %u", em->flags);
-		goto out;
-	}
-	offset = em->start + em->len;
-	btrfs_free_extent_map(em);
-
 	/* Regular extent */
 	em = btrfs_get_extent(BTRFS_I(inode), NULL, offset, sectorsize);
 	if (IS_ERR(em)) {
@@ -348,10 +365,10 @@ static noinline int test_btrfs_get_extent(u32 sectorsize, u32 nodesize)
 		test_err("expected a real extent, got %llu", em->disk_bytenr);
 		goto out;
 	}
-	if (em->start != offset || em->len != sectorsize - 1) {
+	if (em->start != offset || em->len != sectorsize) {
 		test_err(
-	"unexpected extent wanted start %llu len 4095, got start %llu len %llu",
-			offset, em->start, em->len);
+	"unexpected extent wanted start %llu len %u, got start %llu len %llu",
+			offset, sectorsize, em->start, em->len);
 		goto out;
 	}
 	if (em->flags != 0) {
-- 
2.52.0

[PATCH 2/3] btrfs: tests: prepare extent map tests for strict alignment checks

[Qu Wenruo]

Currently the extent map self tests have the following points that will
cause false alerts for the incoming strict extent map alignment checks:

- Inlined extents have their sized smaller than block size
  Which is not following what the kernel is doing for inlined extents,
  as btrfs_extent_item_to_extent_map() always uses the fs block size as
  the length, not the ram_bytes.

  Fix it by using SZ_4K as extent map's length.

- btrfs_fs_info::sectorsize is not properly initialized
  As we always use PAGE_SIZE, which can be values larger than 4K.
  And all the immediate numbers used in the test case is based on 4K
  fs block size.

  Fix it by using fixed SZ_4K fs block size when allocating the dummy
  btrfs_fs_info.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/tests/extent-map-tests.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/fs/btrfs/tests/extent-map-tests.c b/fs/btrfs/tests/extent-map-tests.c
index aabf825e8d7b..811f36d41101 100644
--- a/fs/btrfs/tests/extent-map-tests.c
+++ b/fs/btrfs/tests/extent-map-tests.c
@@ -173,9 +173,12 @@ static int test_case_2(struct btrfs_fs_info *fs_info, struct btrfs_inode *inode)
 		return -ENOMEM;
 	}
 
-	/* Add [0, 1K) */
+	/*
+	 * Add [0, 1K) which is inlined. And the extent map length must
+	 * be one block.
+	 */
 	em->start = 0;
-	em->len = SZ_1K;
+	em->len = SZ_4K;
 	em->disk_bytenr = EXTENT_MAP_INLINE;
 	em->disk_num_bytes = 0;
 	em->ram_bytes = SZ_1K;
@@ -219,7 +222,7 @@ static int test_case_2(struct btrfs_fs_info *fs_info, struct btrfs_inode *inode)
 
 	/* Add [0, 1K) */
 	em->start = 0;
-	em->len = SZ_1K;
+	em->len = SZ_4K;
 	em->disk_bytenr = EXTENT_MAP_INLINE;
 	em->disk_num_bytes = 0;
 	em->ram_bytes = SZ_1K;
@@ -235,7 +238,7 @@ static int test_case_2(struct btrfs_fs_info *fs_info, struct btrfs_inode *inode)
 		ret = -ENOENT;
 		goto out;
 	}
-	if (em->start != 0 || btrfs_extent_map_end(em) != SZ_1K ||
+	if (em->start != 0 || btrfs_extent_map_end(em) != SZ_4K ||
 	    em->disk_bytenr != EXTENT_MAP_INLINE) {
 		test_err(
 "case2 [0 1K]: ret %d return a wrong em (start %llu len %llu disk_bytenr %llu",
@@ -1131,8 +1134,11 @@ int btrfs_test_extent_map(void)
 	/*
 	 * Note: the fs_info is not set up completely, we only need
 	 * fs_info::fsid for the tracepoint.
+	 *
+	 * And all the immediate numbers are based on 4K blocksize,
+	 * thus we have to use 4K as sectorsize no matter the page size.
 	 */
-	fs_info = btrfs_alloc_dummy_fs_info(PAGE_SIZE, PAGE_SIZE);
+	fs_info = btrfs_alloc_dummy_fs_info(SZ_4K, SZ_4K);
 	if (!fs_info) {
 		test_std_err(TEST_ALLOC_FS_INFO);
 		return -ENOMEM;
-- 
2.52.0

[PATCH 3/3] btrfs: add strict extent map alignment checks

[Qu Wenruo]

Currently we do not check the alignment of extent_map structure.

One of the reason is, the existing self tests for inode and extent-map
will not pass.

However that test failure comes from two reasons:

- We have invalid cases in the self tests
  Those cases will be rejected by tree-checkers, and no one is really
  bothering removing the those cases, due to how hard to modify the test
  cases.

- Some test cases is always 4K block sized based but the fs_info is not
  properly initialized to reflect that

Thankfully those legacy problems are properly addressed by previous
patches, now we can finally put the alignment checks into
validate_extent_map().

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 fs/btrfs/extent_map.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/fs/btrfs/extent_map.c b/fs/btrfs/extent_map.c
index 7e38c23a0c1c..07a7bd426c84 100644
--- a/fs/btrfs/extent_map.c
+++ b/fs/btrfs/extent_map.c
@@ -319,8 +319,15 @@ static void dump_extent_map(struct btrfs_fs_info *fs_info, const char *prefix,
 /* Internal sanity checks for btrfs debug builds. */
 static void validate_extent_map(struct btrfs_fs_info *fs_info, struct extent_map *em)
 {
+	const u32 blocksize = fs_info->sectorsize;
+
 	if (!IS_ENABLED(CONFIG_BTRFS_DEBUG))
 		return;
+
+	if (!IS_ALIGNED(em->start, blocksize) ||
+	    !IS_ALIGNED(em->len, blocksize))
+		dump_extent_map(fs_info, "unaligned shared members", em);
+
 	if (em->disk_bytenr < EXTENT_MAP_LAST_BYTE) {
 		if (em->disk_num_bytes == 0)
 			dump_extent_map(fs_info, "zero disk_num_bytes", em);
@@ -334,6 +341,11 @@ static void validate_extent_map(struct btrfs_fs_info *fs_info, struct extent_map
 			dump_extent_map(fs_info,
 		"ram_bytes mismatch with disk_num_bytes for non-compressed em",
 					em);
+		if (!IS_ALIGNED(em->disk_bytenr, blocksize) ||
+		    !IS_ALIGNED(em->disk_num_bytes, blocksize) ||
+		    !IS_ALIGNED(em->offset, blocksize) ||
+		    !IS_ALIGNED(em->ram_bytes, blocksize))
+			dump_extent_map(fs_info, "unaligned members", em);
 	} else if (em->offset) {
 		dump_extent_map(fs_info, "non-zero offset for hole/inline", em);
 	}
-- 
2.52.0

Re: [PATCH 1/3] btrfs: tests: remove invalid file extent map tests

[Filipe Manana]

On Sun, Jan 18, 2026 at 5:30 AM Qu Wenruo <wqu@suse.com> wrote:
>
> In the inode self tests, there are several problems:
>
> - Several invalid cases that will be rejected by tree-checkers

by tree-checkers -> by the tree-checker

Also missing punctuation at the end of the sentence, and this is a
pattern in every first sentence below too.

>   E.g. hole range [4K, 4K + 4) is completely invalid.
>
>   Only inlined extent maps can have an unaligned ram_bytes, and even for
>   that case, the generaeted extent map will use sectorsize as em->len.

generaeted -> generated

>
> - Manually inserted hole after an inlined extent map
>   The kernel never does this by itself, the current btrfs_get_extent()
>   will only return a single inlined extent map that covers the first
>   block.
>
> - Completely incorrect numbers in the comment
>   E.g. 12291 no matter if you add or dec 1, is not aligned to 4K.
>   The properly number for 12K is 12288, I don't know why there is even a
>   diff of 3, and this completely doesn't match the extent map we
>   inserted later.
>
> - Super hard to modify sequence in setup_file_extents()
>   If some unfortunate person, just like me, needs to modify
>   setup_file_extents(), good luck not screwing up the file offset.
>
> Fix them by:
>
> - Remove invalid unaligned extent maps
>   This mostly means remove the [4K, 4K + 4) hole case.
>   The remaining ones are already properly aligned.
>
>   This slightly changes the on-disk data extent allocation, with that
>   removed, the regular extents at [4K, 8K) and [8K , 12K) can be merged.
>
>   So also add a 4K gap between those two data extents to prevent em
>   merge.
>
> - Remove the manual insert of the implied hole after an inlined extent
>   Just like what the kernel is doning for inlined extents in the real

doning -> doing

>   world.
>
> - Update the commit using proper numbers with Kilo suffix
>   Since there is no unaligned range except the first inlined one, we can
>   always use numbers with 'K' suffix, which is way more easier to read,
>   and will always be aligned to 1024 at least.
>
> - Add extra ASSERT()s and comments in setup_file_extents()
>   The ASSERT()s are used to indicate the current file offset.
>   The extra comments are for the basic info of the file extent.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  fs/btrfs/tests/inode-tests.c | 111 ++++++++++++++++++++---------------
>  1 file changed, 64 insertions(+), 47 deletions(-)
>
> diff --git a/fs/btrfs/tests/inode-tests.c b/fs/btrfs/tests/inode-tests.c
> index a4c2b7748b95..58b75bdfed9e 100644
> --- a/fs/btrfs/tests/inode-tests.c
> +++ b/fs/btrfs/tests/inode-tests.c
> @@ -81,17 +81,20 @@ static void insert_inode_item_key(struct btrfs_root *root)
>   * diagram of how the extents will look though this may not be possible we still
>   * want to make sure everything acts normally (the last number is not inclusive)
>   *
> - * [0  - 6][     6 - 4096     ][ 4096 - 4100][4100 - 8195][8195  -  12291]
> - * [inline][hole but no extent][    hole    ][   regular ][regular1 split]
> + * The numbers are using 4K fs block size as an example, the real test will scale
> + * all the extent maps (except the inlined one) according to the block size.
>   *
> - * [12291 - 16387][16387 - 24579][24579 - 28675][ 28675 - 32771][32771 - 36867 ]
> - * [    hole    ][regular1 split][   prealloc ][   prealloc1  ][prealloc1 written]
> + * [ 0  - 6 ][ 6 - 4K       ][ 4K - 8K ][ 8K -  12K      ]
> + * [ inline ][ implied hole ][ regular ][ regular1 split ]
>   *
> - * [36867 - 45059][45059 - 53251][53251 - 57347][57347 - 61443][61443- 69635]
> - * [  prealloc1  ][ compressed  ][ compressed1 ][    regular  ][ compressed1]
> + * [ 12K - 16K ][ 16K - 24K      ][ 24K - 28K ][ 28K - 32K ][ 32K - 36K         ]
> + * [ hole      ][ regular1 split ][ prealloc  ][ prealloc1 ][ prealloc1 written ]
>   *
> - * [69635-73731][   73731 - 86019   ][86019-90115]
> - * [  regular  ][ hole but no extent][  regular  ]
> + * [ 36K - 44K ][ 44K - 52K  ][ 52K - 56K   ][ 56K - 60K ][ 60K - 68 K  ]
> + * [ prealloc1 ][ compressed ][ compressed1 ][ regular   ][ compressed1 ]
> + *
> + * [ 68K - 72K ][ 72K - 84K          ][ 84K - 88K ]
> + * [  regular  ][ hole but no extent ][ regular   ]
>   */
>  static void setup_file_extents(struct btrfs_root *root, u32 sectorsize)
>  {
> @@ -100,40 +103,49 @@ static void setup_file_extents(struct btrfs_root *root, u32 sectorsize)
>         u64 offset = 0;
>
>         /*
> +        * Start 0, length 6, inlined.
> +        *
>          * Tree-checker has strict limits on inline extents that they can only
>          * exist at file offset 0, thus we can only have one inline file extent
>          * at most.
>          */
> +       ASSERT(offset == 0);

Err, what's the point? We have just assigned 0 to offset right above.

>         insert_extent(root, offset, 6, 6, 0, 0, 0, BTRFS_FILE_EXTENT_INLINE, 0,
>                       slot);
>         slot++;
>         offset = sectorsize;
>
> -       /* Now another hole */
> -       insert_extent(root, offset, 4, 4, 0, 0, 0, BTRFS_FILE_EXTENT_REG, 0,
> -                     slot);
> +       /* Start 1 * blocksize, length 1 * blocksize, regular */

End sentence with punctuation (done in most other comments below).

> +       ASSERT(offset == 1 * sectorsize);

Same here, we just assigned sectorsize to offset right above.

> +       insert_extent(root, offset, sectorsize, sectorsize, 0,
> +                     disk_bytenr, sectorsize, BTRFS_FILE_EXTENT_REG, 0, slot);
>         slot++;
> -       offset += 4;
>
> -       /* Now for a regular extent */
> -       insert_extent(root, offset, sectorsize - 1, sectorsize - 1, 0,
> -                     disk_bytenr, sectorsize - 1, BTRFS_FILE_EXTENT_REG, 0, slot);
> -       slot++;
> -       disk_bytenr += sectorsize;
> -       offset += sectorsize - 1;
> +       /* We don't want the regular em got merged with the next one. */

Remove "got".

> +       disk_bytenr += 2 * sectorsize;
> +       offset += sectorsize;
>
>         /*
> +        * Start 2 * blocksize, length 1 * blocksize, regular.
> +        *
>          * Now for 3 extents that were split from a hole punch so we test
>          * offsets properly.
>          */
> +       ASSERT(offset == 2 * sectorsize);

Same here, we have just incremented offset by sectorsize.

>         insert_extent(root, offset, sectorsize, 4 * sectorsize, 0, disk_bytenr,
>                       4 * sectorsize, BTRFS_FILE_EXTENT_REG, 0, slot);
>         slot++;
>         offset += sectorsize;
> +
> +       /* Start 3 * blocksize, length 1 * blocksize, regular, explicit hole. */
> +       ASSERT(offset == 3 * sectorsize);

Same.

>         insert_extent(root, offset, sectorsize, sectorsize, 0, 0, 0,
>                       BTRFS_FILE_EXTENT_REG, 0, slot);
>         slot++;
>         offset += sectorsize;
> +
> +       /* Start 4 * blocksize, length 2 * blocksize, regular. */
> +       ASSERT(offset == 4 * sectorsize);

Same.

>         insert_extent(root, offset, 2 * sectorsize, 4 * sectorsize,
>                       2 * sectorsize, disk_bytenr, 4 * sectorsize,
>                       BTRFS_FILE_EXTENT_REG, 0, slot);
> @@ -141,7 +153,8 @@ static void setup_file_extents(struct btrfs_root *root, u32 sectorsize)
>         offset += 2 * sectorsize;
>         disk_bytenr += 4 * sectorsize;
>
> -       /* Now for a unwritten prealloc extent */
> +       /* Start 6 * blocksize, length 1 * blocksize, preallocated. */
> +       ASSERT(offset == 6 * sectorsize);

Same here, and every ASSERT below.
We have the comments to guide us at which offset we are too, so one
less reason to have the ASSERT, not to mention the tests will fail if
they get screwed up.

>         insert_extent(root, offset, sectorsize, sectorsize, 0, disk_bytenr,
>                 sectorsize, BTRFS_FILE_EXTENT_PREALLOC, 0, slot);
>         slot++;
> @@ -154,19 +167,28 @@ static void setup_file_extents(struct btrfs_root *root, u32 sectorsize)
>         disk_bytenr += 2 * sectorsize;
>
>         /*
> +        * Start 7 * blocksize, length 1 * blocksize, prealloc.
> +        *
>          * Now for a partially written prealloc extent, basically the same as
>          * the hole punch example above.  Ram_bytes never changes when you mark
>          * extents written btw.
>          */
> +       ASSERT(offset == 7 * sectorsize);
>         insert_extent(root, offset, sectorsize, 4 * sectorsize, 0, disk_bytenr,
>                       4 * sectorsize, BTRFS_FILE_EXTENT_PREALLOC, 0, slot);
>         slot++;
>         offset += sectorsize;
> +
> +       /* Start 8 * blocksize, length 1 * blocksize, regular. */
> +       ASSERT(offset == 8 * sectorsize);
>         insert_extent(root, offset, sectorsize, 4 * sectorsize, sectorsize,
>                       disk_bytenr, 4 * sectorsize, BTRFS_FILE_EXTENT_REG, 0,
>                       slot);
>         slot++;
>         offset += sectorsize;
> +
> +       /* Start 9 * blocksize, length 2 * blocksize, prealloc. */
> +       ASSERT(offset == 9 * sectorsize);
>         insert_extent(root, offset, 2 * sectorsize, 4 * sectorsize,
>                       2 * sectorsize, disk_bytenr, 4 * sectorsize,
>                       BTRFS_FILE_EXTENT_PREALLOC, 0, slot);
> @@ -174,7 +196,8 @@ static void setup_file_extents(struct btrfs_root *root, u32 sectorsize)
>         offset += 2 * sectorsize;
>         disk_bytenr += 4 * sectorsize;
>
> -       /* Now a normal compressed extent */
> +       /* Start 11 * blocksize, length 2 * blocksize, regular . */

Space before the punctuation.

Otherwise it looks fine, thanks.

> +       ASSERT(offset == 11 * sectorsize);
>         insert_extent(root, offset, 2 * sectorsize, 2 * sectorsize, 0,
>                       disk_bytenr, sectorsize, BTRFS_FILE_EXTENT_REG,
>                       BTRFS_COMPRESS_ZLIB, slot);
> @@ -183,17 +206,24 @@ static void setup_file_extents(struct btrfs_root *root, u32 sectorsize)
>         /* No merges */
>         disk_bytenr += 2 * sectorsize;
>
> -       /* Now a split compressed extent */
> +       /* Start 13 * blocksize, length 1 * blocksize, regular. */
> +       ASSERT(offset == 13 * sectorsize);
>         insert_extent(root, offset, sectorsize, 4 * sectorsize, 0, disk_bytenr,
>                       sectorsize, BTRFS_FILE_EXTENT_REG,
>                       BTRFS_COMPRESS_ZLIB, slot);
>         slot++;
>         offset += sectorsize;
> +
> +       /* Start 14 * blocksize, length 1 * blocksize, regular. */
> +       ASSERT(offset == 14 * sectorsize);
>         insert_extent(root, offset, sectorsize, sectorsize, 0,
>                       disk_bytenr + sectorsize, sectorsize,
>                       BTRFS_FILE_EXTENT_REG, 0, slot);
>         slot++;
>         offset += sectorsize;
> +
> +       /* Start 15 * blocksize, length 2 * blocksize, regular. */
> +       ASSERT(offset == 15 * sectorsize);
>         insert_extent(root, offset, 2 * sectorsize, 4 * sectorsize,
>                       2 * sectorsize, disk_bytenr, sectorsize,
>                       BTRFS_FILE_EXTENT_REG, BTRFS_COMPRESS_ZLIB, slot);
> @@ -201,12 +231,21 @@ static void setup_file_extents(struct btrfs_root *root, u32 sectorsize)
>         offset += 2 * sectorsize;
>         disk_bytenr += 2 * sectorsize;
>
> -       /* Now extents that have a hole but no hole extent */
> +       /* Start 17 * blocksize, length 1 * blocksize, regular. */
> +       ASSERT(offset == 17 * sectorsize);
>         insert_extent(root, offset, sectorsize, sectorsize, 0, disk_bytenr,
>                       sectorsize, BTRFS_FILE_EXTENT_REG, 0, slot);
>         slot++;
>         offset += 4 * sectorsize;
>         disk_bytenr += sectorsize;
> +
> +       /*
> +        * Start 18 * blocksize, length 3 * blocksize, implied hole (aka no
> +        * file extent item).
> +        *
> +        * Start 21 * blocksize, length 1 * blocksize, regular.
> +        */
> +       ASSERT(offset == 21 * sectorsize);
>         insert_extent(root, offset, sectorsize, sectorsize, 0, disk_bytenr,
>                       sectorsize, BTRFS_FILE_EXTENT_REG, 0, slot);
>  }
> @@ -316,28 +355,6 @@ static noinline int test_btrfs_get_extent(u32 sectorsize, u32 nodesize)
>         offset = em->start + em->len;
>         btrfs_free_extent_map(em);
>
> -       em = btrfs_get_extent(BTRFS_I(inode), NULL, offset, sectorsize);
> -       if (IS_ERR(em)) {
> -               test_err("got an error when we shouldn't have");
> -               goto out;
> -       }
> -       if (em->disk_bytenr != EXTENT_MAP_HOLE) {
> -               test_err("expected a hole, got %llu", em->disk_bytenr);
> -               goto out;
> -       }
> -       if (em->start != offset || em->len != 4) {
> -               test_err(
> -       "unexpected extent wanted start %llu len 4, got start %llu len %llu",
> -                       offset, em->start, em->len);
> -               goto out;
> -       }
> -       if (em->flags != 0) {
> -               test_err("unexpected flags set, want 0 have %u", em->flags);
> -               goto out;
> -       }
> -       offset = em->start + em->len;
> -       btrfs_free_extent_map(em);
> -
>         /* Regular extent */
>         em = btrfs_get_extent(BTRFS_I(inode), NULL, offset, sectorsize);
>         if (IS_ERR(em)) {
> @@ -348,10 +365,10 @@ static noinline int test_btrfs_get_extent(u32 sectorsize, u32 nodesize)
>                 test_err("expected a real extent, got %llu", em->disk_bytenr);
>                 goto out;
>         }
> -       if (em->start != offset || em->len != sectorsize - 1) {
> +       if (em->start != offset || em->len != sectorsize) {
>                 test_err(
> -       "unexpected extent wanted start %llu len 4095, got start %llu len %llu",
> -                       offset, em->start, em->len);
> +       "unexpected extent wanted start %llu len %u, got start %llu len %llu",
> +                       offset, sectorsize, em->start, em->len);
>                 goto out;
>         }
>         if (em->flags != 0) {
> --
> 2.52.0
>
>

Re: [PATCH 2/3] btrfs: tests: prepare extent map tests for strict alignment checks

[Filipe Manana]

On Sun, Jan 18, 2026 at 5:30 AM Qu Wenruo <wqu@suse.com> wrote:
>
> Currently the extent map self tests have the following points that will
> cause false alerts for the incoming strict extent map alignment checks:
>
> - Inlined extents have their sized smaller than block size

sized -> size

As usual, there is a lack of punctuation in the first sentence everywhere.

>   Which is not following what the kernel is doing for inlined extents,
>   as btrfs_extent_item_to_extent_map() always uses the fs block size as
>   the length, not the ram_bytes.
>
>   Fix it by using SZ_4K as extent map's length.
>
> - btrfs_fs_info::sectorsize is not properly initialized
>   As we always use PAGE_SIZE, which can be values larger than 4K.
>   And all the immediate numbers used in the test case is based on 4K

is based -> are based

Otherwise it looks good, thanks.

Reviewed-by: Filipe Manana <fdmanana@suse.com>

>   fs block size.
>
>   Fix it by using fixed SZ_4K fs block size when allocating the dummy
>   btrfs_fs_info.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  fs/btrfs/tests/extent-map-tests.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/fs/btrfs/tests/extent-map-tests.c b/fs/btrfs/tests/extent-map-tests.c
> index aabf825e8d7b..811f36d41101 100644
> --- a/fs/btrfs/tests/extent-map-tests.c
> +++ b/fs/btrfs/tests/extent-map-tests.c
> @@ -173,9 +173,12 @@ static int test_case_2(struct btrfs_fs_info *fs_info, struct btrfs_inode *inode)
>                 return -ENOMEM;
>         }
>
> -       /* Add [0, 1K) */
> +       /*
> +        * Add [0, 1K) which is inlined. And the extent map length must
> +        * be one block.
> +        */
>         em->start = 0;
> -       em->len = SZ_1K;
> +       em->len = SZ_4K;
>         em->disk_bytenr = EXTENT_MAP_INLINE;
>         em->disk_num_bytes = 0;
>         em->ram_bytes = SZ_1K;
> @@ -219,7 +222,7 @@ static int test_case_2(struct btrfs_fs_info *fs_info, struct btrfs_inode *inode)
>
>         /* Add [0, 1K) */
>         em->start = 0;
> -       em->len = SZ_1K;
> +       em->len = SZ_4K;
>         em->disk_bytenr = EXTENT_MAP_INLINE;
>         em->disk_num_bytes = 0;
>         em->ram_bytes = SZ_1K;
> @@ -235,7 +238,7 @@ static int test_case_2(struct btrfs_fs_info *fs_info, struct btrfs_inode *inode)
>                 ret = -ENOENT;
>                 goto out;
>         }
> -       if (em->start != 0 || btrfs_extent_map_end(em) != SZ_1K ||
> +       if (em->start != 0 || btrfs_extent_map_end(em) != SZ_4K ||
>             em->disk_bytenr != EXTENT_MAP_INLINE) {
>                 test_err(
>  "case2 [0 1K]: ret %d return a wrong em (start %llu len %llu disk_bytenr %llu",
> @@ -1131,8 +1134,11 @@ int btrfs_test_extent_map(void)
>         /*
>          * Note: the fs_info is not set up completely, we only need
>          * fs_info::fsid for the tracepoint.
> +        *
> +        * And all the immediate numbers are based on 4K blocksize,
> +        * thus we have to use 4K as sectorsize no matter the page size.
>          */
> -       fs_info = btrfs_alloc_dummy_fs_info(PAGE_SIZE, PAGE_SIZE);
> +       fs_info = btrfs_alloc_dummy_fs_info(SZ_4K, SZ_4K);
>         if (!fs_info) {
>                 test_std_err(TEST_ALLOC_FS_INFO);
>                 return -ENOMEM;
> --
> 2.52.0
>
>

Re: [PATCH 3/3] btrfs: add strict extent map alignment checks

[Filipe Manana]

On Sun, Jan 18, 2026 at 5:31 AM Qu Wenruo <wqu@suse.com> wrote:
>
> Currently we do not check the alignment of extent_map structure.
>
> One of the reason is, the existing self tests for inode and extent-map
> will not pass.

The reasons are the inode and extent-map tests use unaligned values
for start offsets and lengths.

>
> However that test failure comes from two reasons:
>
> - We have invalid cases in the self tests
>   Those cases will be rejected by tree-checkers, and no one is really

by tree-checkers -> by the tree-checker

>   bothering removing the those cases, due to how hard to modify the test

the those cases -> those cases

>   cases.
>
> - Some test cases is always 4K block sized based but the fs_info is not

is always -> are always

>   properly initialized to reflect that
>
> Thankfully those legacy problems are properly addressed by previous
> patches, now we can finally put the alignment checks into
> validate_extent_map().
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  fs/btrfs/extent_map.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/fs/btrfs/extent_map.c b/fs/btrfs/extent_map.c
> index 7e38c23a0c1c..07a7bd426c84 100644
> --- a/fs/btrfs/extent_map.c
> +++ b/fs/btrfs/extent_map.c
> @@ -319,8 +319,15 @@ static void dump_extent_map(struct btrfs_fs_info *fs_info, const char *prefix,
>  /* Internal sanity checks for btrfs debug builds. */
>  static void validate_extent_map(struct btrfs_fs_info *fs_info, struct extent_map *em)
>  {
> +       const u32 blocksize = fs_info->sectorsize;
> +
>         if (!IS_ENABLED(CONFIG_BTRFS_DEBUG))
>                 return;
> +
> +       if (!IS_ALIGNED(em->start, blocksize) ||
> +           !IS_ALIGNED(em->len, blocksize))
> +               dump_extent_map(fs_info, "unaligned shared members", em);

What are shared members? I would say:  "unaligned start offset or
length members".

Thanks.


> +
>         if (em->disk_bytenr < EXTENT_MAP_LAST_BYTE) {
>                 if (em->disk_num_bytes == 0)
>                         dump_extent_map(fs_info, "zero disk_num_bytes", em);
> @@ -334,6 +341,11 @@ static void validate_extent_map(struct btrfs_fs_info *fs_info, struct extent_map
>                         dump_extent_map(fs_info,
>                 "ram_bytes mismatch with disk_num_bytes for non-compressed em",
>                                         em);
> +               if (!IS_ALIGNED(em->disk_bytenr, blocksize) ||
> +                   !IS_ALIGNED(em->disk_num_bytes, blocksize) ||
> +                   !IS_ALIGNED(em->offset, blocksize) ||
> +                   !IS_ALIGNED(em->ram_bytes, blocksize))
> +                       dump_extent_map(fs_info, "unaligned members", em);
>         } else if (em->offset) {
>                 dump_extent_map(fs_info, "non-zero offset for hole/inline", em);
>         }
> --
> 2.52.0
>
>

Re: [PATCH 1/3] btrfs: tests: remove invalid file extent map tests

[Qu Wenruo]

在 2026/1/22 03:39, Filipe Manana 写道:
> On Sun, Jan 18, 2026 at 5:30 AM Qu Wenruo <wqu@suse.com> wrote:
>>
>> In the inode self tests, there are several problems:
>>
>> - Several invalid cases that will be rejected by tree-checkers
> 
> by tree-checkers -> by the tree-checker
> 
> Also missing punctuation at the end of the sentence, and this is a
> pattern in every first sentence below too.

It's an item of a list, not a full sentence.

Remove the "that" part, the item is just "Several invalid cases", why 
you want to put a punctuation for something that is not a full sentence?

For a lot cases, we just want to express some short values/items without 
a full sentence, and there is no subject-verb-object at all.
Thus I don't think we want to put a punctuation for everything.

[...]
>>    */
>>   static void setup_file_extents(struct btrfs_root *root, u32 sectorsize)
>>   {
>> @@ -100,40 +103,49 @@ static void setup_file_extents(struct btrfs_root *root, u32 sectorsize)
>>          u64 offset = 0;
>>
>>          /*
>> +        * Start 0, length 6, inlined.
>> +        *
>>           * Tree-checker has strict limits on inline extents that they can only
>>           * exist at file offset 0, thus we can only have one inline file extent
>>           * at most.
>>           */
>> +       ASSERT(offset == 0);
> 
> Err, what's the point? We have just assigned 0 to offset right above.

Already explained in the commit message:

- Super hard to modify sequence in setup_file_extents()
   If some unfortunate person, just like me, needs to modify
   setup_file_extents(), good luck not screwing up the file offset.

If one has to add/remove some file extent items just like this patch, 
@offset will be changed and good luck counting it manually.

The ASSERT() is an explicit way to tell what's the current offset.

> 
>>          insert_extent(root, offset, 6, 6, 0, 0, 0, BTRFS_FILE_EXTENT_INLINE, 0,
>>                        slot);
>>          slot++;
>>          offset = sectorsize;
>>
>> -       /* Now another hole */
>> -       insert_extent(root, offset, 4, 4, 0, 0, 0, BTRFS_FILE_EXTENT_REG, 0,
>> -                     slot);
>> +       /* Start 1 * blocksize, length 1 * blocksize, regular */
> 
> End sentence with punctuation (done in most other comments below).
> 
>> +       ASSERT(offset == 1 * sectorsize);
> 
> Same here, we just assigned sectorsize to offset right above.

The same as explained.

It's easy to grab the offset for the first one or two items, then please 
tell me the offset in a quick glance for the last few items.

Thanks,
Qu

Re: [PATCH 1/3] btrfs: tests: remove invalid file extent map tests

[Filipe Manana]

On Wed, Jan 21, 2026 at 9:05 PM Qu Wenruo <quwenruo.btrfs@gmx.com> wrote:
>
>
>
> 在 2026/1/22 03:39, Filipe Manana 写道:
> > On Sun, Jan 18, 2026 at 5:30 AM Qu Wenruo <wqu@suse.com> wrote:
> >>
> >> In the inode self tests, there are several problems:
> >>
> >> - Several invalid cases that will be rejected by tree-checkers
> >
> > by tree-checkers -> by the tree-checker
> >
> > Also missing punctuation at the end of the sentence, and this is a
> > pattern in every first sentence below too.
>
> It's an item of a list, not a full sentence.
>
> Remove the "that" part, the item is just "Several invalid cases", why
> you want to put a punctuation for something that is not a full sentence?

The way it's formatted suggests it's a full sentence.
Leave a blank line between that and the rest to make it clear it's not
a sentence.
If everything is stuck together without spacing, it's confusing.

>
> For a lot cases, we just want to express some short values/items without
> a full sentence, and there is no subject-verb-object at all.
> Thus I don't think we want to put a punctuation for everything.
>
> [...]
> >>    */
> >>   static void setup_file_extents(struct btrfs_root *root, u32 sectorsize)
> >>   {
> >> @@ -100,40 +103,49 @@ static void setup_file_extents(struct btrfs_root *root, u32 sectorsize)
> >>          u64 offset = 0;
> >>
> >>          /*
> >> +        * Start 0, length 6, inlined.
> >> +        *
> >>           * Tree-checker has strict limits on inline extents that they can only
> >>           * exist at file offset 0, thus we can only have one inline file extent
> >>           * at most.
> >>           */
> >> +       ASSERT(offset == 0);
> >
> > Err, what's the point? We have just assigned 0 to offset right above.
>
> Already explained in the commit message:
>
> - Super hard to modify sequence in setup_file_extents()
>    If some unfortunate person, just like me, needs to modify
>    setup_file_extents(), good luck not screwing up the file offset.
>
> If one has to add/remove some file extent items just like this patch,
> @offset will be changed and good luck counting it manually.
>
> The ASSERT() is an explicit way to tell what's the current offset.

As I said before, there's no need for the asserts because the comments
you added, right above each assert, already keep telling us the
current offset, so it's not like we get easily lost, far from that.

If we need to change the tests and mess up, then I'd rather have the
test fail gracefully rather than an assert, as the latter requires a
reboot which is annoying.


>
> >
> >>          insert_extent(root, offset, 6, 6, 0, 0, 0, BTRFS_FILE_EXTENT_INLINE, 0,
> >>                        slot);
> >>          slot++;
> >>          offset = sectorsize;
> >>
> >> -       /* Now another hole */
> >> -       insert_extent(root, offset, 4, 4, 0, 0, 0, BTRFS_FILE_EXTENT_REG, 0,
> >> -                     slot);
> >> +       /* Start 1 * blocksize, length 1 * blocksize, regular */
> >
> > End sentence with punctuation (done in most other comments below).
> >
> >> +       ASSERT(offset == 1 * sectorsize);
> >
> > Same here, we just assigned sectorsize to offset right above.
>
> The same as explained.
>
> It's easy to grab the offset for the first one or two items, then please
> tell me the offset in a quick glance for the last few items.
>
> Thanks,
> Qu
>