[PATCH 0/4] btrfs-progs: check: add the ability to detect and report mixed inline and regular data extents

[PATCH 0/4] btrfs-progs: check: add the ability to detect and report mixed inline and regular data extents

[Qu Wenruo]

Btrfs check original mode can't detect mixed inline and regular data
extents at all, while lowmem mode reports the problem as hole gap, not
to the point.

This patchset will add the ability to detect such problem, with a test
image dumped from subpage branch (with the inline extent disable patch
reverted).

The patchset is here to detect such problem exposed during subpage
development, while also acts as the final safenet to catch such mixed
types bug.

Qu Wenruo (4):
  btrfs-progs: check/original: add the "0x" prefix for hex error number
  btrfs-progs: check/original: detect and report mixed inline and
    regular data extents
  btrfs-progs: check/lowmem: detect and report mixed inline and regular
    extents properly
  btrfs-progs: fsck-tests: add test image for mixed inline and regular
    data extents

 check/main.c                                  |   7 +++-
 check/mode-lowmem.c                           |  31 +++++++++++++++---
 check/mode-original.h                         |   2 ++
 .../047-mixed-extent-types/default.img.xz     | Bin 0 -> 2112 bytes
 .../fsck-tests/047-mixed-extent-types/test.sh |  19 +++++++++++
 5 files changed, 54 insertions(+), 5 deletions(-)
 create mode 100644 tests/fsck-tests/047-mixed-extent-types/default.img.xz
 create mode 100755 tests/fsck-tests/047-mixed-extent-types/test.sh

-- 
2.31.1

[PATCH 1/4] btrfs-progs: check/original: add the "0x" prefix for hex error number

[Qu Wenruo]

In print_inode_error(), it print the error number in hex, but without
"0x" prefix.

Just add the prefix.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 check/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/check/main.c b/check/main.c
index 0c4eb3ca5ded..1e65f8da4c6c 100644
--- a/check/main.c
+++ b/check/main.c
@@ -578,7 +578,7 @@ static void print_inode_error(struct btrfs_root *root, struct inode_record *rec)
 		root_objectid = root->root_key.offset;
 		fprintf(stderr, "reloc");
 	}
-	fprintf(stderr, "root %llu inode %llu errors %x",
+	fprintf(stderr, "root %llu inode %llu errors 0x%x",
 		(unsigned long long) root_objectid,
 		(unsigned long long) rec->ino, rec->errors);
 
-- 
2.31.1

[PATCH 2/4] btrfs-progs: check/original: detect and report mixed inline and regular data extents

[Qu Wenruo]

When a btrfs filesystem has mixed inline and regular data extents, btrfs
check original mode won't detect it as an error.

Considering how much effort we have done just to avoid such cases, we
really want btrfs check to detect such problem.

This error detection is even more important for the incoming btrfs
subpage support, as subpage data rw support can cause such problem much
easier.

So this patch will just add such ability to original mode.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 check/main.c          | 5 +++++
 check/mode-original.h | 2 ++
 2 files changed, 7 insertions(+)

diff --git a/check/main.c b/check/main.c
index 1e65f8da4c6c..7837c3647f6f 100644
--- a/check/main.c
+++ b/check/main.c
@@ -621,6 +621,8 @@ static void print_inode_error(struct btrfs_root *root, struct inode_record *rec)
 			rec->imode & ~07777);
 	if (errors & I_ERR_INVALID_GEN)
 		fprintf(stderr, ", invalid inode generation or transid");
+	if (errors & I_ERR_MIXED_EXTENTS)
+		fprintf(stderr, ", mixed regular and inline extents");
 	fprintf(stderr, "\n");
 
 	/* Print the holes if needed */
@@ -1583,6 +1585,7 @@ static int process_file_extent(struct btrfs_root *root,
 	if (extent_type == BTRFS_FILE_EXTENT_INLINE) {
 		struct btrfs_item *item = btrfs_item_nr(slot);
 
+		rec->found_inline_extent = 1;
 		num_bytes = btrfs_file_extent_ram_bytes(eb, fi);
 		if (num_bytes == 0)
 			rec->errors |= I_ERR_BAD_FILE_EXTENT;
@@ -1602,6 +1605,8 @@ static int process_file_extent(struct btrfs_root *root,
 		num_bytes = (num_bytes + mask) & ~mask;
 	} else if (extent_type == BTRFS_FILE_EXTENT_REG ||
 		   extent_type == BTRFS_FILE_EXTENT_PREALLOC) {
+		if (rec->found_inline_extent)
+			rec->errors |= I_ERR_MIXED_EXTENTS;
 		num_bytes = btrfs_file_extent_num_bytes(eb, fi);
 		disk_bytenr = btrfs_file_extent_disk_bytenr(eb, fi);
 		extent_offset = btrfs_file_extent_offset(eb, fi);
diff --git a/check/mode-original.h b/check/mode-original.h
index b075a95c9757..f7efc06ec7c7 100644
--- a/check/mode-original.h
+++ b/check/mode-original.h
@@ -186,6 +186,7 @@ struct unaligned_extent_rec_t {
 #define I_ERR_MISMATCH_DIR_HASH		(1 << 18)
 #define I_ERR_INVALID_IMODE		(1 << 19)
 #define I_ERR_INVALID_GEN		(1 << 20)
+#define I_ERR_MIXED_EXTENTS		(1 << 21)
 
 struct inode_record {
 	struct list_head backrefs;
@@ -197,6 +198,7 @@ struct inode_record {
 	unsigned int found_csum_item:1;
 	unsigned int some_csum_missing:1;
 	unsigned int nodatasum:1;
+	unsigned int found_inline_extent:1;
 	int errors;
 
 	struct list_head unaligned_extent_recs;
-- 
2.31.1

[PATCH 3/4] btrfs-progs: check/lowmem: detect and report mixed inline and regular extents properly

[Qu Wenruo]

[BUG]
When lowmem mode check is executed on a filesystem with mixed inline and
regular extent, it reports the error as file extent gap:

  [4/7] checking fs roots
  ERROR: root 5 EXTENT_DATA[257 4096] gap exists, expected: EXTENT_DATA[257 707]
  ERROR: errors found in fs roots
  found 135168 bytes used, error(s) found

Not detecting the mixed extent types.

[CAUSE]
The offending fs has the following data extents:

        item 9 key (257 INODE_REF 256) itemoff 15703 itemsize 14
                index 2 namelen 4 name: file
        item 10 key (257 EXTENT_DATA 0) itemoff 14975 itemsize 728
                generation 7 type 0 (inline)
                inline extent data size 707 ram_bytes 707 compression 0 (none)
        item 11 key (257 EXTENT_DATA 4096) itemoff 14922 itemsize 53
                generation 7 type 2 (prealloc)
                prealloc data disk byte 102457344 nr 4096
                prealloc data offset 0 nr 4096

The mixed extent type error is obvious, but for inline extent,
check_file_extent_inline() will increase the extent_end by inline size.

However for such mixed case, the regular extent will only start
sectorsize, leaving lowmem check only to report a hole gap.

[FIX]
This patch will fix it by:
- Introduce @found_inline parameter to detect mixed extent types
- Increase @end by sectorsize for check_file_extent_inline()

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 check/mode-lowmem.c | 31 +++++++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)

diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index 2531fde858b2..ed71833eb21e 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -2003,7 +2003,12 @@ static int check_file_extent_inline(struct btrfs_root *root,
 			}
 		}
 	}
-	*end += extent_num_bytes;
+	/*
+	 * For inline extent, we expect no more extents. But if there are
+	 * regular extents after inlined extent, they should start at
+	 * sectorsize, not the inline size.
+	 */
+	*end += root->fs_info->sectorsize;
 	*size += extent_num_bytes;
 
 	return err;
@@ -2022,7 +2027,7 @@ static int check_file_extent_inline(struct btrfs_root *root,
  */
 static int check_file_extent(struct btrfs_root *root, struct btrfs_path *path,
 			     unsigned int nodatasum, u64 isize, u64 *size,
-			     u64 *end)
+			     u64 *end, bool *found_inline)
 {
 	struct btrfs_file_extent_item *fi;
 	struct btrfs_key fkey;
@@ -2058,9 +2063,25 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_path *path,
 	}
 
 	/* Check inline extent */
-	if (extent_type == BTRFS_FILE_EXTENT_INLINE)
+	if (extent_type == BTRFS_FILE_EXTENT_INLINE) {
+		*found_inline = true;
 		return check_file_extent_inline(root, path, size, end);
+	}
 
+	/*
+	 * Here we hit a regular/prealloc file extent, if this inode already
+	 * has an inline extent, we have mixed inline and regular extents.
+	 */
+	if (*found_inline) {
+		err |= FILE_EXTENT_ERROR;
+		error(
+		"root %llu inode %llu has mixed inline and regular extents",
+		      root->objectid, fkey.objectid);
+		/*
+		 * We still want to continue checking, as mixed inline and
+		 * regular is just a minor problem.
+		 */
+	}
 	/* Check REG_EXTENT/PREALLOC_EXTENT */
 	gen = btrfs_file_extent_generation(node, fi);
 	disk_bytenr = btrfs_file_extent_disk_bytenr(node, fi);
@@ -2606,6 +2627,7 @@ static int check_inode_item(struct btrfs_root *root, struct btrfs_path *path)
 	int slot;
 	int ret;
 	int err = 0;
+	bool found_inline = false;
 	char namebuf[BTRFS_NAME_LEN] = {0};
 	u32 name_len = 0;
 
@@ -2726,7 +2748,8 @@ static int check_inode_item(struct btrfs_root *root, struct btrfs_path *path)
 					key.offset);
 			}
 			ret = check_file_extent(root, path, nodatasum, isize,
-						&extent_size, &extent_end);
+						&extent_size, &extent_end,
+						&found_inline);
 			err |= ret;
 			break;
 		case BTRFS_XATTR_ITEM_KEY:
-- 
2.31.1

[PATCH 4/4] btrfs-progs: fsck-tests: add test image for mixed inline and regular data extents

[Qu Wenruo]

To make sure btrfs check can detect such problem.

The image is dumped by using subpage data rw branch, with inline extent
disable patch reverted, running with 4K sectorsize on 64K page size ARM board,
with the following command on an empty btrfs filesystem:

  fsx -q -l 262144 -o 65536 -S 191110531 -N 9057 -R -W $mnt/file

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 .../047-mixed-extent-types/default.img.xz     | Bin 0 -> 2112 bytes
 .../fsck-tests/047-mixed-extent-types/test.sh |  19 ++++++++++++++++++
 2 files changed, 19 insertions(+)
 create mode 100644 tests/fsck-tests/047-mixed-extent-types/default.img.xz
 create mode 100755 tests/fsck-tests/047-mixed-extent-types/test.sh

diff --git a/tests/fsck-tests/047-mixed-extent-types/default.img.xz b/tests/fsck-tests/047-mixed-extent-types/default.img.xz
new file mode 100644
index 0000000000000000000000000000000000000000..cf240501f08f38dbaaf9116330fdae438f14a6d7
GIT binary patch
literal 2112
zcmV-G2*3CJH+ooF000E$*0e?f03iVu0001VFXf})5B~@LT>wRyj;C3^v%$$4d1t#p
zhA1?_LT>WU6(o{g^wy`#%^URoE0#nM%oKGSkItgLPyY-!1}Su+9>j|f-76RLVnK$o
zzYOv_3rdd_fZKMM{YA%Du4uLOnx-~%Oui=VxQ*`~=s`}KFZ`e_;=z*FCl3k5%VkrY
zw1vd^{`LF%Axxn_@4sN;ThyD#+Rkop)Moxa(9EZBg0s&NJNaU}f;Q}7(?0tmpY!??
zo8p4C+}LykeUZMt*e5F@@dPkKJ;8g&e}uJuj#@S0v98{!TOZF)FQR3a;Q-*Ec{oEf
zg{fY;sW1U9V?cH@DFUUuOE=5+=-Z_7U2ubDF#ab*sQWK^YEu)I4PQv46+~#s)56lY
z1)p#P{$S63tcP+Dz%Iuj@MKEmhJ;CtUK8W*CndS{H7#-}#yu8lcBBKhf+E86y@n)A
zvnVX!)6XztuPEpZd4%D^#2E~Tq`2#r?z7rhV07<yAE=40yTfU0Ey?1Y@NWr3sU_N+
zEH>l$>mX=Qf{Aq3_Jk2G;_<Asgw&EnBKsGtyj}h0Xr)lqi?tiVfHVt4%+T_l74G@o
zUb`S048xP=V6vQ%@_5Mv#I7O5O=yZCf=jU3OHBfVwydfpAYqJMyl2*8N?@gLzXpIG
z;d_LvV!-dkK8;VQfZ>5v8{hjpRP6M(<^&$p#N~uJ-7o?A&g`pJ&1lR2jYL;(#|G1#
zkejXx^o#~T=E+^n{iC>)`bI@2!{F&y!nSKVq{>L?|ASA2UDLWiVEyYL4eQ}x2_v{4
z^8(W)B(hJaJI#F_AfigWvBR9q>_p*5zAm$xo4>CoFDl*a97qO9XOz$eE(W2}Mb)0I
z#cf|1PEDJw7yAk01~V&lv$ZS-y5lcFDHOUK*3PLi0Z&{l5A6Ic^efOKqt@N-&si0X
zLyc#2;2*Q9rhjzmLu16XEekLrYomAgzig<zh-et8awaxmCY+Xpn>@ZUD3*A{B!GIR
zBJV`N9crp;pc!=hSrYO@4Gruzl|KtmFL`EOOWNXDd1NUEdQm-yqi!USL8YMtR{SoC
z&CEBfzYR#Ob&$2RJVQbcEN6j(B$r+^sfhE-uE4|e`ddQ#E|Xj?OX-5caOLfKS1JHj
zwEk>zdx@d|ZRJCz!Y2@BTHRQoY2N<-JMOq}apB%oy@22Fn*^LRXWK7EwqcC1RwpmA
zRh9NBkanX7iqoZ)agWEh#*SR%Bg7TEeAoZckW9+uZQ3VwJbcTNqvqctu`D4o(OyXJ
z);Z`XWYUWw)n|+ijL8$SLr}?N&0n<K@$^!UtZ;CdHc~cO_qYL}?oiMns8r!EV>Qoy
zyr3+8=e20w%P#s<*n)q7>e3QVZ<pYeuXS=|wX8G9Ya1%5Xp>J>oVjx5_Jp4jY@de~
zSJftrDb4$VXbwx~b9Ti2&ACxSMiWg^Zzx(v!z-AG@CTdouyA6Gre4h5vfU53Z*%4L
z**4%ogY(-ZAE6qPxDAidL27LH)o7`OS?^^$<bgcKQ#zi%(JU6lda@x~WF@ee>i-{_
z5=VPJ#m*m%B`6;NLM}mMzOm&rwq@t&O8g8X`9FHH>xA1A<b*@})lI^r@LL$!P_^{H
z(+i&X40>LQdDqU03<@8P)T9`a=b8Kaa(C3Gt?4T#+Cs!FIMf(Da?ai`2R8qor`q79
zZ1aE|Al%^xyc7+LH^7U>x~+NT<y1Rkg+h^TE<I5t!^pC$(T_Fk+P<l`g_w@+tfM}$
z{DKsuf1mA1PGN3ad#mH(E_iwZ@I&?(0(q3g(nfN6ZvYt1KF4ZmtOgimGxR-t{tpJz
zF32TcW){+w(sy#5mLs7fQ=0(QL&XJvFSVbt9fz#6CTH0z<8?dLar_DMRFX{#VbvdI
zN(?M3GeH*YOBl7p%(z)a`o~b<UjyAI=^FzfL+ct7RD}#mCx@jIkoAH%RdP+t-tEd{
zaQ47Fl7Kq&o+%I%^b={P?bDJs-A=kJ#Ep6|6(kfBqrn4I(4|?$*ZW(UO!5wfr%oc}
z#a~`D5^ZTFa(X_UW5j<L9fy@h@c1q6^L4t*X3rU6$~U4gYwpe-cD>e=9V!29KfN{F
z0w!#%Hw(MMiV<#6Ti5kTco#sGq?TVAuwKs~qZpsrqtW&^+-cy#1KeHgg>JMc<^6{T
z74`DUohLA*qq>@JmULBNn%BPbHG11qMPZo`Ner*ICa8J=){d4>V|+ND#f>86w|q0n
z)qCd3$`BeDfYjPB<hH$y@^`ub>nNfVLlJ^gyud;r|4k3F>67JN#edHPU>NG=A$VaJ
z96TrYWX9iMy~0iM3Hwa{6sUsF<eOma%zXBQWY#K_M)nD1|4yrDd$5cYU+Ux7@;}(G
zwM&v})uo85rika%lO>}gA7cny=`B2XGv?ys{?xXG2})&r|3uY_y5i_l2e&3lCje59
z>SJ<jXe`KTFcSr3$v*nNugH$jQ%?t-1P;piQfO+{Oxl^yzw?c^u7>5xoh~5M2{<ac
zetE!Ya%-ms5-%|mQb?qCcuU9|_%FI!zIby|H|J9%3PSw%Zl2qKX>&?wz)+LIR9U*<
zp?0*Z@ga3rmVg!F8h0;VwculDi_UQkO)ouWRO!dPltFgVGtKkYLJ`IrRPF+GAd(Ut
z*yHITSaW%>c%?gVGv&(=HCp>rG4$9=3^nV!Aj3rC!GLAcoC{rQBrcCWf>`*}S>kL!
q%)y2L0001*U%@Gyl;P$80h$niAOHZNhFr?A#Ao{g000001X)`88z)2n

literal 0
HcmV?d00001

diff --git a/tests/fsck-tests/047-mixed-extent-types/test.sh b/tests/fsck-tests/047-mixed-extent-types/test.sh
new file mode 100755
index 000000000000..9c7f5690c552
--- /dev/null
+++ b/tests/fsck-tests/047-mixed-extent-types/test.sh
@@ -0,0 +1,19 @@
+#!/bin/bash
+#
+# Verify that check can detect inline and regular extents inside the same inode
+#
+# This problem takes us quite much effort in the past, and subpage data write
+# can cause such problem more easily.
+#
+# Make sure btrfs check can at least detect such error
+
+source "$TEST_TOP/common"
+
+check_prereq btrfs
+
+check_image() {
+	run_mustfail "mixed extent types not detected" \
+		"$TOP/btrfs" check "$1"
+}
+
+check_all_images
-- 
2.31.1

Re: [PATCH 0/4] btrfs-progs: check: add the ability to detect and report mixed inline and regular data extents

[Su Yue]

On Tue 04 May 2021 at 14:25, Qu Wenruo <wqu@suse.com> wrote:

> Btrfs check original mode can't detect mixed inline and regular 
> data
> extents at all, while lowmem mode reports the problem as hole 
> gap, not
> to the point.
>
> This patchset will add the ability to detect such problem, with 
> a test
> image dumped from subpage branch (with the inline extent disable 
> patch
> reverted).
>
> The patchset is here to detect such problem exposed during 
> subpage
> development, while also acts as the final safenet to catch such 
> mixed
> types bug.
>
> Qu Wenruo (4):
>   btrfs-progs: check/original: add the "0x" prefix for hex error 
>   number
>   btrfs-progs: check/original: detect and report mixed inline 
>   and
>     regular data extents
>   btrfs-progs: check/lowmem: detect and report mixed inline and 
>   regular
>     extents properly
>   btrfs-progs: fsck-tests: add test image for mixed inline and 
>   regular
>     data extents
>
For the series,
Reviewed-by: Su Yue <l@damenly.su>

--
Su
>  check/main.c                                  |   7 +++-
>  check/mode-lowmem.c                           |  31 
>  +++++++++++++++---
>  check/mode-original.h                         |   2 ++
>  .../047-mixed-extent-types/default.img.xz     | Bin 0 -> 2112 
>  bytes
>  .../fsck-tests/047-mixed-extent-types/test.sh |  19 +++++++++++
>  5 files changed, 54 insertions(+), 5 deletions(-)
>  create mode 100644 
>  tests/fsck-tests/047-mixed-extent-types/default.img.xz
>  create mode 100755 
>  tests/fsck-tests/047-mixed-extent-types/test.sh

Re: [PATCH 0/4] btrfs-progs: check: add the ability to detect and report mixed inline and regular data extents

[Qu Wenruo]

On 2021/5/4 下午2:25, Qu Wenruo wrote:
> Btrfs check original mode can't detect mixed inline and regular data
> extents at all, while lowmem mode reports the problem as hole gap, not
> to the point.

Please discard the patchset.

We don't yet have an agreement on whether inline extent should be 
exclusive, and since kernel can handle it without any problem, it isn't 
something that urgent yet.

Furthermore the fix would be way more complex than I thought, as even on 
x86 we could reproduce such inline + regular extents, it's not that 
worthy yet.

Thanks,
Qu

> 
> This patchset will add the ability to detect such problem, with a test
> image dumped from subpage branch (with the inline extent disable patch
> reverted).
> 
> The patchset is here to detect such problem exposed during subpage
> development, while also acts as the final safenet to catch such mixed
> types bug.
> 
> Qu Wenruo (4):
>    btrfs-progs: check/original: add the "0x" prefix for hex error number
>    btrfs-progs: check/original: detect and report mixed inline and
>      regular data extents
>    btrfs-progs: check/lowmem: detect and report mixed inline and regular
>      extents properly
>    btrfs-progs: fsck-tests: add test image for mixed inline and regular
>      data extents
> 
>   check/main.c                                  |   7 +++-
>   check/mode-lowmem.c                           |  31 +++++++++++++++---
>   check/mode-original.h                         |   2 ++
>   .../047-mixed-extent-types/default.img.xz     | Bin 0 -> 2112 bytes
>   .../fsck-tests/047-mixed-extent-types/test.sh |  19 +++++++++++
>   5 files changed, 54 insertions(+), 5 deletions(-)
>   create mode 100644 tests/fsck-tests/047-mixed-extent-types/default.img.xz
>   create mode 100755 tests/fsck-tests/047-mixed-extent-types/test.sh
>