[PATCH 0/6] btrfs-progs: Fixes inline ram_bytes related bugs

[PATCH 0/6] btrfs-progs: Fixes inline ram_bytes related bugs

[Qu Wenruo]

The patchset can be fetched from github (*):
https://github.com/adam900710/btrfs-progs/tree/inline_ram_bytes

It's based on David's devel branch, whose HEAD is:
commit 0d1c5812e28e286648781c7b35b542311cc01aa4 (david/devel)
Author: Matthias Benkard <matthias.benkard@egym.de>
Date:   Wed Apr 25 16:34:54 2018 +0200

    btrfs-progs: mkfs: traverse_directory: Reset error code on continue

Reported-by Steve Leung <sjleung@shaw.ca>, his old btrfs (at least
offending inodes are from 2014) has inline uncompressed extent, while
its ram_bytes mismatch with item size.

Latest kernel tree check catches this bug, while we failed to detect by
dump-tree.

It turns out that btrfs-progs is doing something evil to avoid reading
ram_bytes from inline uncompressed extent.


So this patchset will address all such ram_bytes related problems.

The 1st patch is a not-so-relative fix for restore, which is using
ram_bytes for decompress. Although thanks to the compression header, we
won't read out-of-boundary, but fixing it is never a bad thing.

The 2nd patch will get rid of the evil btrfs_file_extent_inline_len()
which hides raw ram_bytes from us, and fooling us for a long long time.

The 3rd~5th patches introduce check/repair function for both original
and lowmem mode (although lowmem mode can detect it even before this patch).

The last one is the test case for it as usual.

*: Or should I just migrate to gitlab after M$ acquired github?

Qu Wenruo (6):
  btrfs-progs: restore: Fix wrong compressed item size for decompress()
  btrfs-progs: Get rid of the confusing btrfs_file_extent_inline_len()
  btrfs-progs: check/original: Detect and repair wrong inline ram_bytes
  btrfs-progs: check/lowmem: Prepare check_file_extent() to handle
    repair
  btrfs-progs: check/lowmem: Repair wrong inlien ram_bytes for
    uncompressed extent
  btrfs-progs: fsck-tests: Add test case for corrupted inline ram_bytes

 check/main.c                                  |  46 ++++++-
 check/mode-lowmem.c                           | 120 ++++++++++++++----
 check/mode-original.h                         |   1 +
 cmds-restore.c                                |   5 +-
 ctree.h                                       |  22 ----
 file.c                                        |   3 +-
 print-tree.c                                  |   4 +-
 .../offset_by_one.img                         | Bin 0 -> 3072 bytes
 .../035-inline-bad-ram-bytes/test.sh          |  11 ++
 9 files changed, 157 insertions(+), 55 deletions(-)
 create mode 100644 tests/fsck-tests/035-inline-bad-ram-bytes/offset_by_one.img
 create mode 100755 tests/fsck-tests/035-inline-bad-ram-bytes/test.sh

-- 
2.17.1

[PATCH 1/6] btrfs-progs: restore: Fix wrong compressed item size for decompress()

[Qu Wenruo]

When using decompress() in copy_one_inline(), we're passing the
decompressed extent size (ram_bytes) into decompress().
However we only has @inline_item_len read out, the pending data will be
uninitialized data.

Thankfully, all compression methods supported have some extra data in
its header, thus it won't cause real problem.

Whatever fixing it is never a bad idea.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 cmds-restore.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/cmds-restore.c b/cmds-restore.c
index 342f5cc7eabb..29a329a397c2 100644
--- a/cmds-restore.c
+++ b/cmds-restore.c
@@ -324,7 +324,8 @@ static int copy_one_inline(struct btrfs_root *root, int fd,
 		return -ENOMEM;
 	}
 
-	ret = decompress(root, buf, outbuf, len, &ram_size, compress);
+	ret = decompress(root, buf, outbuf, inline_item_len, &ram_size,
+			 compress);
 	if (ret) {
 		free(outbuf);
 		return ret;
-- 
2.17.1

[PATCH 2/6] btrfs-progs: Get rid of the confusing btrfs_file_extent_inline_len()

[Qu Wenruo]

[BUG]
If one uncompressed inline extent has incorrect ram_bytes, neither btrfs
check nor dump-tree could detect such corruption.

[CAUSE]
Every caller tries to read inline extent ram_bytes is using
btrfs_file_extent_inline_len(), other than directly calling
btrfs_file_extent_ram_bytes().

For compressed extent, it's just calling btrfs_file_extent_ram_bytes().
However for uncompressed extent, it falls back to
btrfs_file_extent_inline_item_len(), makes us unable to detect anything
wrong in ram_bytes.

[FIX]
Just get rid of such confusing btrfs_file_extent_inline_len() function.

Reported-by: Steve Leung <sjleung@shaw.ca>
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 check/main.c        |  2 +-
 check/mode-lowmem.c |  2 +-
 cmds-restore.c      |  2 +-
 ctree.h             | 22 ----------------------
 file.c              |  3 +--
 print-tree.c        |  4 ++--
 6 files changed, 6 insertions(+), 29 deletions(-)

diff --git a/check/main.c b/check/main.c
index 235db373a86c..52ef181add61 100644
--- a/check/main.c
+++ b/check/main.c
@@ -1472,7 +1472,7 @@ static int process_file_extent(struct btrfs_root *root,
 		u8 compression = btrfs_file_extent_compression(eb, fi);
 		struct btrfs_item *item = btrfs_item_nr(slot);
 
-		num_bytes = btrfs_file_extent_inline_len(eb, slot, fi);
+		num_bytes = btrfs_file_extent_ram_bytes(eb, fi);
 		if (num_bytes == 0)
 			rec->errors |= I_ERR_BAD_FILE_EXTENT;
 		if (compression) {
diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index c42dfcc50352..b3198fffa250 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -1776,7 +1776,7 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_key *fkey,
 		u32 item_inline_len;
 
 		item_inline_len = btrfs_file_extent_inline_item_len(node, e);
-		extent_num_bytes = btrfs_file_extent_inline_len(node, slot, fi);
+		extent_num_bytes = btrfs_file_extent_ram_bytes(node, fi);
 		compressed = btrfs_file_extent_compression(node, fi);
 		if (extent_num_bytes == 0) {
 			error(
diff --git a/cmds-restore.c b/cmds-restore.c
index 29a329a397c2..9542221fd265 100644
--- a/cmds-restore.c
+++ b/cmds-restore.c
@@ -302,7 +302,7 @@ static int copy_one_inline(struct btrfs_root *root, int fd,
 	fi = btrfs_item_ptr(leaf, path->slots[0],
 			    struct btrfs_file_extent_item);
 	ptr = btrfs_file_extent_inline_start(fi);
-	len = btrfs_file_extent_inline_len(leaf, path->slots[0], fi);
+	len = btrfs_file_extent_ram_bytes(leaf, fi);
 	inline_item_len = btrfs_file_extent_inline_item_len(leaf, btrfs_item_nr(path->slots[0]));
 	read_extent_buffer(leaf, buf, ptr, inline_item_len);
 
diff --git a/ctree.h b/ctree.h
index de4b1b7e6416..04a77550c715 100644
--- a/ctree.h
+++ b/ctree.h
@@ -2443,28 +2443,6 @@ static inline u32 btrfs_search_header_len(struct btrfs_ioctl_search_header *sh)
 	return get_unaligned_32(&sh->len);
 }
 
-/* this returns the number of file bytes represented by the inline item.
- * If an item is compressed, this is the uncompressed size
- */
-static inline u32 btrfs_file_extent_inline_len(struct extent_buffer *eb,
-					       int slot,
-					       struct btrfs_file_extent_item *fi)
-{
-	/*
-	 * return the space used on disk if this item isn't
-	 * compressed or encoded
-	 */
-	if (btrfs_file_extent_compression(eb, fi) == 0 &&
-	    btrfs_file_extent_encryption(eb, fi) == 0 &&
-	    btrfs_file_extent_other_encoding(eb, fi) == 0) {
-		return btrfs_file_extent_inline_item_len(eb,
-							 btrfs_item_nr(slot));
-	}
-
-	/* otherwise use the ram bytes field */
-	return btrfs_file_extent_ram_bytes(eb, fi);
-}
-
 #define btrfs_fs_incompat(fs_info, opt) \
 	__btrfs_fs_incompat((fs_info), BTRFS_FEATURE_INCOMPAT_##opt)
 
diff --git a/file.c b/file.c
index f5e645c47bda..056be1045814 100644
--- a/file.c
+++ b/file.c
@@ -255,8 +255,7 @@ int btrfs_read_file(struct btrfs_root *root, u64 ino, u64 start, int len,
 		/* Inline extent, one inode should only one inline extent */
 		if (btrfs_file_extent_type(leaf, fi) ==
 		    BTRFS_FILE_EXTENT_INLINE) {
-			extent_len = btrfs_file_extent_inline_len(leaf, slot,
-								  fi);
+			extent_len = btrfs_file_extent_ram_bytes(leaf, fi);
 			if (extent_start + extent_len <= start)
 				goto next;
 			read_extent_buffer(leaf, dest,
diff --git a/print-tree.c b/print-tree.c
index dfa7bb6b676f..a09ecfbb28f0 100644
--- a/print-tree.c
+++ b/print-tree.c
@@ -357,9 +357,9 @@ static void print_file_extent_item(struct extent_buffer *eb,
 			extent_type, file_extent_type_to_str(extent_type));
 
 	if (extent_type == BTRFS_FILE_EXTENT_INLINE) {
-		printf("\t\tinline extent data size %u ram_bytes %u compression %hhu (%s)\n",
+		printf("\t\tinline extent data size %u ram_bytes %llu compression %hhu (%s)\n",
 				btrfs_file_extent_inline_item_len(eb, item),
-				btrfs_file_extent_inline_len(eb, slot, fi),
+				btrfs_file_extent_ram_bytes(eb, fi),
 				btrfs_file_extent_compression(eb, fi),
 				compress_str);
 		return;
-- 
2.17.1

[PATCH 3/6] btrfs-progs: check/original: Detect and repair wrong inline ram_bytes

[Qu Wenruo]

It looks like that around 2014, btrfs kernel has a regression that would
cause offset-by-one ram_bytes for inline extent.

Add the ability to repair it in original mode.

Reported-by: Steve Leung <sjleung@shaw.ca>
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 check/main.c          | 44 +++++++++++++++++++++++++++++++++++++++++--
 check/mode-original.h |  1 +
 2 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/check/main.c b/check/main.c
index 52ef181add61..1374d8856d72 100644
--- a/check/main.c
+++ b/check/main.c
@@ -1483,6 +1483,9 @@ static int process_file_extent(struct btrfs_root *root,
 		} else {
 			if (num_bytes > max_inline_size)
 				rec->errors |= I_ERR_FILE_EXTENT_TOO_LARGE;
+			if (btrfs_file_extent_inline_item_len(eb, item) !=
+			    num_bytes)
+				rec->errors |= I_ERR_INLINE_RAM_BYTES_WRONG;
 		}
 		rec->found_size += num_bytes;
 		num_bytes = (num_bytes + mask) & ~mask;
@@ -2534,6 +2537,40 @@ out:
 	return ret;
 }
 
+static int repair_inline_ram_bytes(struct btrfs_trans_handle *trans,
+				   struct btrfs_root *root,
+				   struct btrfs_path *path,
+				   struct inode_record *rec)
+{
+	struct btrfs_key key;
+	struct btrfs_file_extent_item *fi;
+	struct btrfs_item *i;
+	u64 on_disk_item_len;
+	int ret;
+
+	key.objectid = rec->ino;
+	key.offset = 0;
+	key.type = BTRFS_EXTENT_DATA_KEY;
+
+	ret = btrfs_search_slot(trans, root, &key, path, 0, 1);
+	if (ret > 0)
+		ret = -ENOENT;
+	if (ret < 0)
+		goto out;
+
+	i = btrfs_item_nr(path->slots[0]);
+	on_disk_item_len = btrfs_file_extent_inline_item_len(path->nodes[0], i);
+	fi = btrfs_item_ptr(path->nodes[0], path->slots[0],
+			    struct btrfs_file_extent_item);
+	btrfs_set_file_extent_ram_bytes(path->nodes[0], fi, on_disk_item_len);
+	btrfs_mark_buffer_dirty(path->nodes[0]);
+	printf("Successfully repaired inline ram_bytes for root %llu ino %llu\n",
+		root->objectid, rec->ino);
+out:
+	btrfs_release_path(path);
+	return ret;
+}
+
 static int try_repair_inode(struct btrfs_root *root, struct inode_record *rec)
 {
 	struct btrfs_trans_handle *trans;
@@ -2545,8 +2582,9 @@ static int try_repair_inode(struct btrfs_root *root, struct inode_record *rec)
 			     I_ERR_LINK_COUNT_WRONG |
 			     I_ERR_NO_INODE_ITEM |
 			     I_ERR_FILE_EXTENT_ORPHAN |
-			     I_ERR_FILE_EXTENT_DISCOUNT|
-			     I_ERR_FILE_NBYTES_WRONG)))
+			     I_ERR_FILE_EXTENT_DISCOUNT |
+			     I_ERR_FILE_NBYTES_WRONG |
+			     I_ERR_INLINE_RAM_BYTES_WRONG)))
 		return rec->errors;
 
 	/*
@@ -2575,6 +2613,8 @@ static int try_repair_inode(struct btrfs_root *root, struct inode_record *rec)
 		ret = repair_inode_nlinks(trans, root, &path, rec);
 	if (!ret && rec->errors & I_ERR_FILE_NBYTES_WRONG)
 		ret = repair_inode_nbytes(trans, root, &path, rec);
+	if (!ret && rec->errors & I_ERR_INLINE_RAM_BYTES_WRONG)
+		ret = repair_inline_ram_bytes(trans, root, &path, rec);
 	btrfs_commit_transaction(trans, root);
 	btrfs_release_path(&path);
 	return ret;
diff --git a/check/mode-original.h b/check/mode-original.h
index 13cfa5b9e1b3..ec2842e0b3be 100644
--- a/check/mode-original.h
+++ b/check/mode-original.h
@@ -187,6 +187,7 @@ struct file_extent_hole {
 #define I_ERR_FILE_EXTENT_ORPHAN	(1 << 14)
 #define I_ERR_FILE_EXTENT_TOO_LARGE	(1 << 15)
 #define I_ERR_ODD_INODE_FLAGS		(1 << 16)
+#define I_ERR_INLINE_RAM_BYTES_WRONG	(1 << 17)
 
 struct inode_record {
 	struct list_head backrefs;
-- 
2.17.1

Re: [PATCH 3/6] btrfs-progs: check/original: Detect and repair wrong inline ram_bytes

[Su Yue]

[-- Attachment #1: Type: text/plain, Size: 4111 bytes --]



On 06/06/2018 03:27 PM, Qu Wenruo wrote:
> It looks like that around 2014, btrfs kernel has a regression that would
> cause offset-by-one ram_bytes for inline extent.
> 
> Add the ability to repair it in original mode.
> 
> Reported-by: Steve Leung <sjleung@shaw.ca>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  check/main.c          | 44 +++++++++++++++++++++++++++++++++++++++++--
>  check/mode-original.h |  1 +
>  2 files changed, 43 insertions(+), 2 deletions(-)
> 
> diff --git a/check/main.c b/check/main.c
> index 52ef181add61..1374d8856d72 100644
> --- a/check/main.c
> +++ b/check/main.c
> @@ -1483,6 +1483,9 @@ static int process_file_extent(struct btrfs_root *root,
>  		} else {
>  			if (num_bytes > max_inline_size)
>  				rec->errors |= I_ERR_FILE_EXTENT_TOO_LARGE;
> +			if (btrfs_file_extent_inline_item_len(eb, item) !=
> +			    num_bytes)
> +				rec->errors |= I_ERR_INLINE_RAM_BYTES_WRONG;
>  		}
>  		rec->found_size += num_bytes;
>  		num_bytes = (num_bytes + mask) & ~mask;
> @@ -2534,6 +2537,40 @@ out:
>  	return ret;
>  }
>  
> +static int repair_inline_ram_bytes(struct btrfs_trans_handle *trans,
> +				   struct btrfs_root *root,
> +				   struct btrfs_path *path,
> +				   struct inode_record *rec)
> +{
> +	struct btrfs_key key;
> +	struct btrfs_file_extent_item *fi;
> +	struct btrfs_item *i;
> +	u64 on_disk_item_len;
> +	int ret;
> +
> +	key.objectid = rec->ino;
> +	key.offset = 0;
> +	key.type = BTRFS_EXTENT_DATA_KEY;
> +
> +	ret = btrfs_search_slot(trans, root, &key, path, 0, 1);
> +	if (ret > 0)
> +		ret = -ENOENT;
> +	if (ret < 0)
> +		goto out;
> +
> +	i = btrfs_item_nr(path->slots[0]);
> +	on_disk_item_len = btrfs_file_extent_inline_item_len(path->nodes[0], i);
> +	fi = btrfs_item_ptr(path->nodes[0], path->slots[0],
> +			    struct btrfs_file_extent_item);
> +	btrfs_set_file_extent_ram_bytes(path->nodes[0], fi, on_disk_item_len);
> +	btrfs_mark_buffer_dirty(path->nodes[0]);
> +	printf("Successfully repaired inline ram_bytes for root %llu ino %llu\n",
> +		root->objectid, rec->ino);

Miss a line "rec->errors &= ~I_ERR_INLINE_RAM_BYTES_WRONG;"?.
Although the new test case passes, the fake appearance is due to
existed bug about returned codes of check_ino_rec().

And refer to outputs of existed code, no need for the word
'Successfully'.
> +out:
> +	btrfs_release_path(path);
> +	return ret;
> +}
> +
>  static int try_repair_inode(struct btrfs_root *root, struct inode_record *rec)
>  {
>  	struct btrfs_trans_handle *trans;
> @@ -2545,8 +2582,9 @@ static int try_repair_inode(struct btrfs_root *root, struct inode_record *rec)
>  			     I_ERR_LINK_COUNT_WRONG |
>  			     I_ERR_NO_INODE_ITEM |
>  			     I_ERR_FILE_EXTENT_ORPHAN |
> -			     I_ERR_FILE_EXTENT_DISCOUNT|
> -			     I_ERR_FILE_NBYTES_WRONG)))
> +			     I_ERR_FILE_EXTENT_DISCOUNT |
> +			     I_ERR_FILE_NBYTES_WRONG |
> +			     I_ERR_INLINE_RAM_BYTES_WRONG)))

It's better if you can add otehr infomation into print_inode_error().

Others look fine to me.

Thanks,
Su
>  		return rec->errors;
>  
>  	/*
> @@ -2575,6 +2613,8 @@ static int try_repair_inode(struct btrfs_root *root, struct inode_record *rec)
>  		ret = repair_inode_nlinks(trans, root, &path, rec);
>  	if (!ret && rec->errors & I_ERR_FILE_NBYTES_WRONG)
>  		ret = repair_inode_nbytes(trans, root, &path, rec);
> +	if (!ret && rec->errors & I_ERR_INLINE_RAM_BYTES_WRONG)
> +		ret = repair_inline_ram_bytes(trans, root, &path, rec);
>  	btrfs_commit_transaction(trans, root);
>  	btrfs_release_path(&path);
>  	return ret;
> diff --git a/check/mode-original.h b/check/mode-original.h
> index 13cfa5b9e1b3..ec2842e0b3be 100644
> --- a/check/mode-original.h
> +++ b/check/mode-original.h
> @@ -187,6 +187,7 @@ struct file_extent_hole {
>  #define I_ERR_FILE_EXTENT_ORPHAN	(1 << 14)
>  #define I_ERR_FILE_EXTENT_TOO_LARGE	(1 << 15)
>  #define I_ERR_ODD_INODE_FLAGS		(1 << 16)
> +#define I_ERR_INLINE_RAM_BYTES_WRONG	(1 << 17)
>  
>  struct inode_record {
>  	struct list_head backrefs;
> 



[-- Attachment #2: pEpkey.asc --]
[-- Type: application/pgp-keys, Size: 1791 bytes --]

Re: [PATCH 3/6] btrfs-progs: check/original: Detect and repair wrong inline ram_bytes

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 4363 bytes --]



On 2018年06月06日 16:08, Su Yue wrote:
> 
> 
> On 06/06/2018 03:27 PM, Qu Wenruo wrote:
>> It looks like that around 2014, btrfs kernel has a regression that would
>> cause offset-by-one ram_bytes for inline extent.
>>
>> Add the ability to repair it in original mode.
>>
>> Reported-by: Steve Leung <sjleung@shaw.ca>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>  check/main.c          | 44 +++++++++++++++++++++++++++++++++++++++++--
>>  check/mode-original.h |  1 +
>>  2 files changed, 43 insertions(+), 2 deletions(-)
>>
>> diff --git a/check/main.c b/check/main.c
>> index 52ef181add61..1374d8856d72 100644
>> --- a/check/main.c
>> +++ b/check/main.c
>> @@ -1483,6 +1483,9 @@ static int process_file_extent(struct btrfs_root *root,
>>  		} else {
>>  			if (num_bytes > max_inline_size)
>>  				rec->errors |= I_ERR_FILE_EXTENT_TOO_LARGE;
>> +			if (btrfs_file_extent_inline_item_len(eb, item) !=
>> +			    num_bytes)
>> +				rec->errors |= I_ERR_INLINE_RAM_BYTES_WRONG;
>>  		}
>>  		rec->found_size += num_bytes;
>>  		num_bytes = (num_bytes + mask) & ~mask;
>> @@ -2534,6 +2537,40 @@ out:
>>  	return ret;
>>  }
>>  
>> +static int repair_inline_ram_bytes(struct btrfs_trans_handle *trans,
>> +				   struct btrfs_root *root,
>> +				   struct btrfs_path *path,
>> +				   struct inode_record *rec)
>> +{
>> +	struct btrfs_key key;
>> +	struct btrfs_file_extent_item *fi;
>> +	struct btrfs_item *i;
>> +	u64 on_disk_item_len;
>> +	int ret;
>> +
>> +	key.objectid = rec->ino;
>> +	key.offset = 0;
>> +	key.type = BTRFS_EXTENT_DATA_KEY;
>> +
>> +	ret = btrfs_search_slot(trans, root, &key, path, 0, 1);
>> +	if (ret > 0)
>> +		ret = -ENOENT;
>> +	if (ret < 0)
>> +		goto out;
>> +
>> +	i = btrfs_item_nr(path->slots[0]);
>> +	on_disk_item_len = btrfs_file_extent_inline_item_len(path->nodes[0], i);
>> +	fi = btrfs_item_ptr(path->nodes[0], path->slots[0],
>> +			    struct btrfs_file_extent_item);
>> +	btrfs_set_file_extent_ram_bytes(path->nodes[0], fi, on_disk_item_len);
>> +	btrfs_mark_buffer_dirty(path->nodes[0]);
>> +	printf("Successfully repaired inline ram_bytes for root %llu ino %llu\n",
>> +		root->objectid, rec->ino);
> 
> Miss a line "rec->errors &= ~I_ERR_INLINE_RAM_BYTES_WRONG;"?.
> Although the new test case passes, the fake appearance is due to
> existed bug about returned codes of check_ino_rec().

Oh, I forgot this one, thanks for pointing out.

Thanks,
Qu

> 
> And refer to outputs of existed code, no need for the word
> 'Successfully'.
>> +out:
>> +	btrfs_release_path(path);
>> +	return ret;
>> +}
>> +
>>  static int try_repair_inode(struct btrfs_root *root, struct inode_record *rec)
>>  {
>>  	struct btrfs_trans_handle *trans;
>> @@ -2545,8 +2582,9 @@ static int try_repair_inode(struct btrfs_root *root, struct inode_record *rec)
>>  			     I_ERR_LINK_COUNT_WRONG |
>>  			     I_ERR_NO_INODE_ITEM |
>>  			     I_ERR_FILE_EXTENT_ORPHAN |
>> -			     I_ERR_FILE_EXTENT_DISCOUNT|
>> -			     I_ERR_FILE_NBYTES_WRONG)))
>> +			     I_ERR_FILE_EXTENT_DISCOUNT |
>> +			     I_ERR_FILE_NBYTES_WRONG |
>> +			     I_ERR_INLINE_RAM_BYTES_WRONG)))
> 
> It's better if you can add otehr infomation into print_inode_error().
> 
> Others look fine to me.
> 
> Thanks,
> Su
>>  		return rec->errors;
>>  
>>  	/*
>> @@ -2575,6 +2613,8 @@ static int try_repair_inode(struct btrfs_root *root, struct inode_record *rec)
>>  		ret = repair_inode_nlinks(trans, root, &path, rec);
>>  	if (!ret && rec->errors & I_ERR_FILE_NBYTES_WRONG)
>>  		ret = repair_inode_nbytes(trans, root, &path, rec);
>> +	if (!ret && rec->errors & I_ERR_INLINE_RAM_BYTES_WRONG)
>> +		ret = repair_inline_ram_bytes(trans, root, &path, rec);
>>  	btrfs_commit_transaction(trans, root);
>>  	btrfs_release_path(&path);
>>  	return ret;
>> diff --git a/check/mode-original.h b/check/mode-original.h
>> index 13cfa5b9e1b3..ec2842e0b3be 100644
>> --- a/check/mode-original.h
>> +++ b/check/mode-original.h
>> @@ -187,6 +187,7 @@ struct file_extent_hole {
>>  #define I_ERR_FILE_EXTENT_ORPHAN	(1 << 14)
>>  #define I_ERR_FILE_EXTENT_TOO_LARGE	(1 << 15)
>>  #define I_ERR_ODD_INODE_FLAGS		(1 << 16)
>> +#define I_ERR_INLINE_RAM_BYTES_WRONG	(1 << 17)
>>  
>>  struct inode_record {
>>  	struct list_head backrefs;
>>
> 
> 


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

[PATCH v2 3/6] btrfs-progs: check/original: Detect and repair wrong inline ram_bytes

[Qu Wenruo]

It looks like that around 2014, btrfs kernel has a regression that would
cause offset-by-one ram_bytes for inline extent.

Add the ability to repair it in original mode.

Reported-by: Steve Leung <sjleung@shaw.ca>
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
changelog:
v2:
  Add extra output for print_inode_error().
  Reword the repair output message.
  Clear the error bitmap if repaired.
---
 check/main.c          | 47 +++++++++++++++++++++++++++++++++++++++++--
 check/mode-original.h |  1 +
 2 files changed, 46 insertions(+), 2 deletions(-)

diff --git a/check/main.c b/check/main.c
index 52ef181add61..c739c094cef3 100644
--- a/check/main.c
+++ b/check/main.c
@@ -578,6 +578,8 @@ static void print_inode_error(struct btrfs_root *root, struct inode_record *rec)
 		fprintf(stderr, ", orphan file extent");
 	if (errors & I_ERR_ODD_INODE_FLAGS)
 		fprintf(stderr, ", odd inode flags");
+	if (errors & I_ERR_INLINE_RAM_BYTES_WRONG)
+		fprintf(stderr, ", invalid inline ram bytes");
 	fprintf(stderr, "\n");
 	/* Print the orphan extents if needed */
 	if (errors & I_ERR_FILE_EXTENT_ORPHAN)
@@ -1483,6 +1485,9 @@ static int process_file_extent(struct btrfs_root *root,
 		} else {
 			if (num_bytes > max_inline_size)
 				rec->errors |= I_ERR_FILE_EXTENT_TOO_LARGE;
+			if (btrfs_file_extent_inline_item_len(eb, item) !=
+			    num_bytes)
+				rec->errors |= I_ERR_INLINE_RAM_BYTES_WRONG;
 		}
 		rec->found_size += num_bytes;
 		num_bytes = (num_bytes + mask) & ~mask;
@@ -2534,6 +2539,41 @@ out:
 	return ret;
 }
 
+static int repair_inline_ram_bytes(struct btrfs_trans_handle *trans,
+				   struct btrfs_root *root,
+				   struct btrfs_path *path,
+				   struct inode_record *rec)
+{
+	struct btrfs_key key;
+	struct btrfs_file_extent_item *fi;
+	struct btrfs_item *i;
+	u64 on_disk_item_len;
+	int ret;
+
+	key.objectid = rec->ino;
+	key.offset = 0;
+	key.type = BTRFS_EXTENT_DATA_KEY;
+
+	ret = btrfs_search_slot(trans, root, &key, path, 0, 1);
+	if (ret > 0)
+		ret = -ENOENT;
+	if (ret < 0)
+		goto out;
+
+	i = btrfs_item_nr(path->slots[0]);
+	on_disk_item_len = btrfs_file_extent_inline_item_len(path->nodes[0], i);
+	fi = btrfs_item_ptr(path->nodes[0], path->slots[0],
+			    struct btrfs_file_extent_item);
+	btrfs_set_file_extent_ram_bytes(path->nodes[0], fi, on_disk_item_len);
+	btrfs_mark_buffer_dirty(path->nodes[0]);
+	printf("Repaired inline ram_bytes for root %llu ino %llu\n",
+		root->objectid, rec->ino);
+	rec->errors &= ~I_ERR_INLINE_RAM_BYTES_WRONG;
+out:
+	btrfs_release_path(path);
+	return ret;
+}
+
 static int try_repair_inode(struct btrfs_root *root, struct inode_record *rec)
 {
 	struct btrfs_trans_handle *trans;
@@ -2545,8 +2585,9 @@ static int try_repair_inode(struct btrfs_root *root, struct inode_record *rec)
 			     I_ERR_LINK_COUNT_WRONG |
 			     I_ERR_NO_INODE_ITEM |
 			     I_ERR_FILE_EXTENT_ORPHAN |
-			     I_ERR_FILE_EXTENT_DISCOUNT|
-			     I_ERR_FILE_NBYTES_WRONG)))
+			     I_ERR_FILE_EXTENT_DISCOUNT |
+			     I_ERR_FILE_NBYTES_WRONG |
+			     I_ERR_INLINE_RAM_BYTES_WRONG)))
 		return rec->errors;
 
 	/*
@@ -2575,6 +2616,8 @@ static int try_repair_inode(struct btrfs_root *root, struct inode_record *rec)
 		ret = repair_inode_nlinks(trans, root, &path, rec);
 	if (!ret && rec->errors & I_ERR_FILE_NBYTES_WRONG)
 		ret = repair_inode_nbytes(trans, root, &path, rec);
+	if (!ret && rec->errors & I_ERR_INLINE_RAM_BYTES_WRONG)
+		ret = repair_inline_ram_bytes(trans, root, &path, rec);
 	btrfs_commit_transaction(trans, root);
 	btrfs_release_path(&path);
 	return ret;
diff --git a/check/mode-original.h b/check/mode-original.h
index 13cfa5b9e1b3..ec2842e0b3be 100644
--- a/check/mode-original.h
+++ b/check/mode-original.h
@@ -187,6 +187,7 @@ struct file_extent_hole {
 #define I_ERR_FILE_EXTENT_ORPHAN	(1 << 14)
 #define I_ERR_FILE_EXTENT_TOO_LARGE	(1 << 15)
 #define I_ERR_ODD_INODE_FLAGS		(1 << 16)
+#define I_ERR_INLINE_RAM_BYTES_WRONG	(1 << 17)
 
 struct inode_record {
 	struct list_head backrefs;
-- 
2.17.1

Re: [PATCH v2 3/6] btrfs-progs: check/original: Detect and repair wrong inline ram_bytes

[Su Yue]

On 06/06/2018 04:26 PM, Qu Wenruo wrote:
> It looks like that around 2014, btrfs kernel has a regression that would
> cause offset-by-one ram_bytes for inline extent.
> 
> Add the ability to repair it in original mode.
> 
> Reported-by: Steve Leung <sjleung@shaw.ca>
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Reviewed-by: Su Yue <suy.fnst@cn.fujitsu.com>

> ---
> changelog:
> v2:
>   Add extra output for print_inode_error().
>   Reword the repair output message.
>   Clear the error bitmap if repaired.
> ---
>  check/main.c          | 47 +++++++++++++++++++++++++++++++++++++++++--
>  check/mode-original.h |  1 +
>  2 files changed, 46 insertions(+), 2 deletions(-)
> 
> diff --git a/check/main.c b/check/main.c
> index 52ef181add61..c739c094cef3 100644
> --- a/check/main.c
> +++ b/check/main.c
> @@ -578,6 +578,8 @@ static void print_inode_error(struct btrfs_root *root, struct inode_record *rec)
>  		fprintf(stderr, ", orphan file extent");
>  	if (errors & I_ERR_ODD_INODE_FLAGS)
>  		fprintf(stderr, ", odd inode flags");
> +	if (errors & I_ERR_INLINE_RAM_BYTES_WRONG)
> +		fprintf(stderr, ", invalid inline ram bytes");
>  	fprintf(stderr, "\n");
>  	/* Print the orphan extents if needed */
>  	if (errors & I_ERR_FILE_EXTENT_ORPHAN)
> @@ -1483,6 +1485,9 @@ static int process_file_extent(struct btrfs_root *root,
>  		} else {
>  			if (num_bytes > max_inline_size)
>  				rec->errors |= I_ERR_FILE_EXTENT_TOO_LARGE;
> +			if (btrfs_file_extent_inline_item_len(eb, item) !=
> +			    num_bytes)
> +				rec->errors |= I_ERR_INLINE_RAM_BYTES_WRONG;
>  		}
>  		rec->found_size += num_bytes;
>  		num_bytes = (num_bytes + mask) & ~mask;
> @@ -2534,6 +2539,41 @@ out:
>  	return ret;
>  }
>  
> +static int repair_inline_ram_bytes(struct btrfs_trans_handle *trans,
> +				   struct btrfs_root *root,
> +				   struct btrfs_path *path,
> +				   struct inode_record *rec)
> +{
> +	struct btrfs_key key;
> +	struct btrfs_file_extent_item *fi;
> +	struct btrfs_item *i;
> +	u64 on_disk_item_len;
> +	int ret;
> +
> +	key.objectid = rec->ino;
> +	key.offset = 0;
> +	key.type = BTRFS_EXTENT_DATA_KEY;
> +
> +	ret = btrfs_search_slot(trans, root, &key, path, 0, 1);
> +	if (ret > 0)
> +		ret = -ENOENT;
> +	if (ret < 0)
> +		goto out;
> +
> +	i = btrfs_item_nr(path->slots[0]);
> +	on_disk_item_len = btrfs_file_extent_inline_item_len(path->nodes[0], i);
> +	fi = btrfs_item_ptr(path->nodes[0], path->slots[0],
> +			    struct btrfs_file_extent_item);
> +	btrfs_set_file_extent_ram_bytes(path->nodes[0], fi, on_disk_item_len);
> +	btrfs_mark_buffer_dirty(path->nodes[0]);
> +	printf("Repaired inline ram_bytes for root %llu ino %llu\n",
> +		root->objectid, rec->ino);
> +	rec->errors &= ~I_ERR_INLINE_RAM_BYTES_WRONG;
> +out:
> +	btrfs_release_path(path);
> +	return ret;
> +}
> +
>  static int try_repair_inode(struct btrfs_root *root, struct inode_record *rec)
>  {
>  	struct btrfs_trans_handle *trans;
> @@ -2545,8 +2585,9 @@ static int try_repair_inode(struct btrfs_root *root, struct inode_record *rec)
>  			     I_ERR_LINK_COUNT_WRONG |
>  			     I_ERR_NO_INODE_ITEM |
>  			     I_ERR_FILE_EXTENT_ORPHAN |
> -			     I_ERR_FILE_EXTENT_DISCOUNT|
> -			     I_ERR_FILE_NBYTES_WRONG)))
> +			     I_ERR_FILE_EXTENT_DISCOUNT |
> +			     I_ERR_FILE_NBYTES_WRONG |
> +			     I_ERR_INLINE_RAM_BYTES_WRONG)))
>  		return rec->errors;
>  
>  	/*
> @@ -2575,6 +2616,8 @@ static int try_repair_inode(struct btrfs_root *root, struct inode_record *rec)
>  		ret = repair_inode_nlinks(trans, root, &path, rec);
>  	if (!ret && rec->errors & I_ERR_FILE_NBYTES_WRONG)
>  		ret = repair_inode_nbytes(trans, root, &path, rec);
> +	if (!ret && rec->errors & I_ERR_INLINE_RAM_BYTES_WRONG)
> +		ret = repair_inline_ram_bytes(trans, root, &path, rec);
>  	btrfs_commit_transaction(trans, root);
>  	btrfs_release_path(&path);
>  	return ret;
> diff --git a/check/mode-original.h b/check/mode-original.h
> index 13cfa5b9e1b3..ec2842e0b3be 100644
> --- a/check/mode-original.h
> +++ b/check/mode-original.h
> @@ -187,6 +187,7 @@ struct file_extent_hole {
>  #define I_ERR_FILE_EXTENT_ORPHAN	(1 << 14)
>  #define I_ERR_FILE_EXTENT_TOO_LARGE	(1 << 15)
>  #define I_ERR_ODD_INODE_FLAGS		(1 << 16)
> +#define I_ERR_INLINE_RAM_BYTES_WRONG	(1 << 17)
>  
>  struct inode_record {
>  	struct list_head backrefs;
> 

[PATCH 4/6] btrfs-progs: check/lowmem: Prepare check_file_extent() to handle repair

[Qu Wenruo]

Current check_file_extent() doesn't support later repair work, since it
doesn't accept btrfs_path structure as parameter, thus it can't modify
btrfs trees, or later check will still use the old and wrong path.

Use btrfs_path to replace btrfs_key, extent_buffer and slot parameters,
so we can modify @path directly for repair, and reduce the number of
parameters for check_file_extent().

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

diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index b3198fffa250..3280591396bd 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -1740,18 +1740,18 @@ static int punch_extent_hole(struct btrfs_root *root, u64 ino, u64 start,
  * check and update the last offset of the file extent.
  *
  * @root:	the root of fs/file tree.
- * @fkey:	the key of the file extent.
  * @nodatasum:	INODE_NODATASUM feature.
  * @size:	the sum of all EXTENT_DATA items size for this inode.
  * @end:	the offset of the last extent.
  *
  * Return 0 if no error occurred.
  */
-static int check_file_extent(struct btrfs_root *root, struct btrfs_key *fkey,
-			     struct extent_buffer *node, int slot,
+static int check_file_extent(struct btrfs_root *root, struct btrfs_path *path,
 			     unsigned int nodatasum, u64 *size, u64 *end)
 {
 	struct btrfs_file_extent_item *fi;
+	struct btrfs_key fkey;
+	struct extent_buffer *node = path->nodes[0];
 	u64 disk_bytenr;
 	u64 disk_num_bytes;
 	u64 extent_num_bytes;
@@ -1763,10 +1763,12 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_key *fkey,
 				BTRFS_MAX_INLINE_DATA_SIZE(root->fs_info));
 	unsigned int extent_type;
 	unsigned int is_hole;
+	int slot = path->slots[0];
 	int compressed = 0;
 	int ret;
 	int err = 0;
 
+	btrfs_item_key_to_cpu(node, &fkey, slot);
 	fi = btrfs_item_ptr(node, slot, struct btrfs_file_extent_item);
 
 	/* Check inline extent */
@@ -1781,23 +1783,23 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_key *fkey,
 		if (extent_num_bytes == 0) {
 			error(
 		"root %llu EXTENT_DATA[%llu %llu] has empty inline extent",
-				root->objectid, fkey->objectid, fkey->offset);
+				root->objectid, fkey.objectid, fkey.offset);
 			err |= FILE_EXTENT_ERROR;
 		}
 		if (compressed) {
 			if (extent_num_bytes > root->fs_info->sectorsize) {
 				error(
 "root %llu EXTENT_DATA[%llu %llu] too large inline extent ram size, have %llu, max: %u",
-					root->objectid, fkey->objectid,
-					fkey->offset, extent_num_bytes,
+					root->objectid, fkey.objectid,
+					fkey.offset, extent_num_bytes,
 					root->fs_info->sectorsize - 1);
 				err |= FILE_EXTENT_ERROR;
 			}
 			if (item_inline_len > max_inline_extent_size) {
 				error(
 "root %llu EXTENT_DATA[%llu %llu] too large inline extent on-disk size, have %u, max: %u",
-					root->objectid, fkey->objectid,
-					fkey->offset, item_inline_len,
+					root->objectid, fkey.objectid,
+					fkey.offset, item_inline_len,
 					max_inline_extent_size);
 				err |= FILE_EXTENT_ERROR;
 			}
@@ -1805,7 +1807,7 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_key *fkey,
 			if (extent_num_bytes > max_inline_extent_size) {
  			error(
  "root %llu EXTENT_DATA[%llu %llu] too large inline extent size, have %llu, max: %u",
- 				root->objectid, fkey->objectid, fkey->offset,
+ 				root->objectid, fkey.objectid, fkey.offset,
  				extent_num_bytes, max_inline_extent_size);
 				err |= FILE_EXTENT_ERROR;
 			}
@@ -1813,7 +1815,7 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_key *fkey,
 		if (!compressed && extent_num_bytes != item_inline_len) {
 			error(
 		"root %llu EXTENT_DATA[%llu %llu] wrong inline size, have: %llu, expected: %u",
-				root->objectid, fkey->objectid, fkey->offset,
+				root->objectid, fkey.objectid, fkey.offset,
 				extent_num_bytes, item_inline_len);
 			err |= FILE_EXTENT_ERROR;
 		}
@@ -1827,7 +1829,7 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_key *fkey,
 			extent_type != BTRFS_FILE_EXTENT_PREALLOC) {
 		err |= FILE_EXTENT_ERROR;
 		error("root %llu EXTENT_DATA[%llu %llu] type bad",
-		      root->objectid, fkey->objectid, fkey->offset);
+		      root->objectid, fkey.objectid, fkey.offset);
 		return err;
 	}
 
@@ -1864,12 +1866,12 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_key *fkey,
 	if (csum_found > 0 && nodatasum) {
 		err |= ODD_CSUM_ITEM;
 		error("root %llu EXTENT_DATA[%llu %llu] nodatasum shouldn't have datasum",
-		      root->objectid, fkey->objectid, fkey->offset);
+		      root->objectid, fkey.objectid, fkey.offset);
 	} else if (extent_type == BTRFS_FILE_EXTENT_REG && !nodatasum &&
 		   !is_hole && (ret < 0 || csum_found < search_len)) {
 		err |= CSUM_ITEM_MISSING;
 		error("root %llu EXTENT_DATA[%llu %llu] csum missing, have: %llu, expected: %llu",
-		      root->objectid, fkey->objectid, fkey->offset,
+		      root->objectid, fkey.objectid, fkey.offset,
 		      csum_found, search_len);
 	} else if (extent_type == BTRFS_FILE_EXTENT_PREALLOC &&
 		   csum_found > 0) {
@@ -1881,22 +1883,22 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_key *fkey,
 			err |= ODD_CSUM_ITEM;
 			error(
 "root %llu EXTENT_DATA[%llu %llu] prealloc shouldn't have csum, but has: %llu",
-			      root->objectid, fkey->objectid, fkey->offset,
+			      root->objectid, fkey.objectid, fkey.offset,
 			      csum_found);
 		}
 	}
 
 	/* Check EXTENT_DATA hole */
-	if (!no_holes && *end != fkey->offset) {
+	if (!no_holes && *end != fkey.offset) {
 		if (repair)
-			ret = punch_extent_hole(root, fkey->objectid,
-						*end, fkey->offset - *end);
+			ret = punch_extent_hole(root, fkey.objectid,
+						*end, fkey.offset - *end);
 		if (!repair || ret) {
 			err |= FILE_EXTENT_ERROR;
 			error(
 "root %llu EXTENT_DATA[%llu %llu] gap exists, expected: EXTENT_DATA[%llu %llu]",
-				root->objectid, fkey->objectid, fkey->offset,
-				fkey->objectid, *end);
+				root->objectid, fkey.objectid, fkey.offset,
+				fkey.objectid, *end);
 		}
 	}
 
@@ -2374,9 +2376,8 @@ static int check_inode_item(struct btrfs_root *root, struct btrfs_path *path)
 					root->objectid, inode_id, key.objectid,
 					key.offset);
 			}
-			ret = check_file_extent(root, &key, node, slot,
-						nodatasum, &extent_size,
-						&extent_end);
+			ret = check_file_extent(root, path, nodatasum,
+						&extent_size, &extent_end);
 			err |= ret;
 			break;
 		case BTRFS_XATTR_ITEM_KEY:
-- 
2.17.1

[PATCH 5/6] btrfs-progs: check/lowmem: Repair wrong inlien ram_bytes for uncompressed extent

[Qu Wenruo]

Similar to the original mode repair.

Reported-by: Steve Leung <sjleung@shaw.ca>
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 check/mode-lowmem.c | 73 ++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 72 insertions(+), 1 deletion(-)

diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index 3280591396bd..cb778ce6f789 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -1735,6 +1735,70 @@ static int punch_extent_hole(struct btrfs_root *root, u64 ino, u64 start,
 	return ret;
 }
 
+static int repair_inline_ram_bytes(struct btrfs_root *root,
+				   struct btrfs_path *path, u64 *ram_bytes_ret)
+{
+	struct btrfs_trans_handle *trans;
+	struct btrfs_key key;
+	struct btrfs_file_extent_item *fi;
+	struct btrfs_item *i;
+	u32 on_disk_data_len;
+	int ret;
+	int recover_ret;
+
+	trans = btrfs_start_transaction(root, 1);
+	if (IS_ERR(trans)) {
+		ret = PTR_ERR(trans);
+		return ret;
+	}
+	btrfs_item_key_to_cpu(path->nodes[0], &key, path->slots[0]);
+	btrfs_release_path(path);
+	ret = btrfs_search_slot(trans, root, &key, path, 0, 1);
+	/* Not really possible */
+	if (ret > 0) {
+		ret = -ENOENT;
+		btrfs_release_path(path);
+		goto recover;
+	}
+	if (ret < 0) {
+		goto recover;
+	}
+	i = btrfs_item_nr(path->slots[0]);
+	on_disk_data_len = btrfs_file_extent_inline_item_len(path->nodes[0], i);
+
+	fi = btrfs_item_ptr(path->nodes[0], path->slots[0],
+			    struct btrfs_file_extent_item);
+	if (btrfs_file_extent_type(path->nodes[0], fi) !=
+			BTRFS_FILE_EXTENT_INLINE ||
+	    btrfs_file_extent_compression(path->nodes[0], fi) !=
+			BTRFS_COMPRESS_NONE)
+		return -EINVAL;
+	btrfs_set_file_extent_ram_bytes(path->nodes[0], fi, on_disk_data_len);
+	btrfs_mark_buffer_dirty(path->nodes[0]);
+
+	ret = btrfs_commit_transaction(trans, root);
+	if (!ret) {
+		printf(
+	"Successfully repaired inline ram_bytes for root %llu ino %llu\n",
+			root->objectid, key.objectid);
+		*ram_bytes_ret = on_disk_data_len;
+	}
+	return ret;
+
+recover:
+	/*
+	 * CoW search failed, mostly due to the extra CoW work
+	 * (extent allocation, etc).
+	 * Since we have a good path before, readonly search should still
+	 * work, or later checks will fail due to empty path
+	 */
+	recover_ret = btrfs_search_slot(NULL, root, &key, path, 0, 0);
+
+	/* This really shouldn't happen, or we have a big trouble */
+	ASSERT(recover_ret == 0);
+	return ret;
+}
+
 /*
  * Check file extent datasum/hole, update the size of the file extents,
  * check and update the last offset of the file extent.
@@ -1817,7 +1881,14 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_path *path,
 		"root %llu EXTENT_DATA[%llu %llu] wrong inline size, have: %llu, expected: %u",
 				root->objectid, fkey.objectid, fkey.offset,
 				extent_num_bytes, item_inline_len);
-			err |= FILE_EXTENT_ERROR;
+			if (repair) {
+				ret = repair_inline_ram_bytes(root, path,
+							      &extent_num_bytes);
+				if (ret)
+					err |= FILE_EXTENT_ERROR;
+			} else {
+				err |= FILE_EXTENT_ERROR;
+			}
 		}
 		*end += extent_num_bytes;
 		*size += extent_num_bytes;
-- 
2.17.1

[PATCH 6/6] btrfs-progs: fsck-tests: Add test case for corrupted inline ram_bytes

[Qu Wenruo]

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 .../035-inline-bad-ram-bytes/offset_by_one.img   | Bin 0 -> 3072 bytes
 .../fsck-tests/035-inline-bad-ram-bytes/test.sh  |  11 +++++++++++
 2 files changed, 11 insertions(+)
 create mode 100644 tests/fsck-tests/035-inline-bad-ram-bytes/offset_by_one.img
 create mode 100755 tests/fsck-tests/035-inline-bad-ram-bytes/test.sh

diff --git a/tests/fsck-tests/035-inline-bad-ram-bytes/offset_by_one.img b/tests/fsck-tests/035-inline-bad-ram-bytes/offset_by_one.img
new file mode 100644
index 0000000000000000000000000000000000000000..2f58208eed71d5e0ee053d825d2780fe6842a3db
GIT binary patch
literal 3072
zcmeH}`8ONr8pka~h^564TeVt2sb#d>Vg%KyR)sRL#x}O9A*eNoYK>9*R)ebA(+OgY
zkw_DJ2es9xEfJJfl&B231i9+oALh*MKXA|Od*1VXpZ9ytbKd8C-sQZhzWJex%C1DN
z|1`d%k^8tG1+-(+=4j5v1vu8xW3A`;?2H}9PmcA?vA+Fme{Ek0d?oO|LLh49O}KHv
zP=bIeVqZ69EaGYNk~`IzZ#ddd1_bI{Jd?xs^HPaX;&|dxT)T&?tfq;i_6hAp?Hpb+
zod=eB=rgX3Tqg}pbh0Rb%HI&6yq^Pk&;7(FQj6gA1ZjCt5Zm)(yKY^~p&4vjzoV*0
zOq@4w#HE-XZhvuP;*D968)`CTW`ap00P7elRZGco#qT=LzSHu^uSw>s@Q_SPwAB{6
z-l=NBC!ndLr0*mn|GZVh^^oV+Sl*(Tp9cN+Q$37<8Pz)H+BfbeOm$}rmWcRo@h%S^
zXOVfhh@#d-Q8Vkajn-Wz^8KGUmc;;x6!oh*K?dT9spQ3k&MfUhZIB{aJG)`Qd{Gx9
z|5PwdxIP06KPjyz&40SHFUFw=_u}%$d1xpR#Xv=pRDN4a9#8g0h!SryT6SADH)^co
zw=~CXNoxZwf6$f*xHzIS<XL`jkk)Lo8A4Nf2<F`!yYZNbe+92(DlgK#!WIO2<iNon
z^!HLE@IwsQFQ1YGqWmMp*GBJ+Qx4b_{yS&{u_AMdS;g9!Gbpm}$An;4%=ci<?%wC`
zM)z~xzxZh6u%26UXUjg2Vua%8V#ivobegv9)6%Sdm&;}BqPV)y-*+8xpg{a3Y~{(H
z=9O2gQk?K0fh1V1YCugn?2?<bz&+F6YxL({hTjm-%m;A$$D%<}r{sG~FB+iFI07@g
z0!$HT%|ts}1Ok8qN3+}*ZbPL-u(VjnRR-mN%AgDqZUyhbh4f#N3S<ptrIFC}52dlV
zs6$gf`i@+7hn>L&x*=?JV}uCQ;$OjTd^CvO8>Lm;&yoh>)+WI2hlUpy3*-vK9tWzn
zWjqnUW1*2a&<7ztt3J=%>`dQ4eVOQzGHYs?u95nZi{`7%I<kvG*0evZOf(AS(IxD|
zcm1_Z*cdrM@XJ6=?g8n;y&^dLaO?%o@Eg+#Gqs|Egzik`s(|Fm%Zczc<K>Cs%4d02
zhS|?t=;^g`i`S4r^#veoQ-Z<5<j^N*>m;H4a2^BaFq{V!ITXvPEQxjrgPgH+Z2Ne|
zt7?Z#tWTSQ%mr6esLz&TL=WeVa7!4Q_shk|j{2_mSJtSw)ip~~!p+u`4k4viSy2)G
z%Ilq7#O>BT44T8b_m)#rH<AkjmZh!PLKII%(=|w5ohKsJ9k_{kG6%*F(tAxT{L^pu
zZ5rn<%m8=sP~s2d*5nv!4OQnk3))c}eZ@9lPJ<txI$If+vHKleUd^x}yiMqXgT*go
zN375nN^6)3*RSRu#89z?MVS1XQxcoL6?QwaY}XxxEAH|#%oP`_Tkv|h>3|Yk+}U>9
zEGf@)b#XjYRjo0s`<$O<hTEuxb6HOKHyc+Wcu|bH50o5CH=gKTbsw07<WV+JjTb^Y
zbK>`D-d%%)$sF;yu!~HP_DYoSK0EWl{od^AXVg{ByQTvkux(G_=Uc0DPtp{`Pb<`I
zceSY%*O1^dwYj6#^H9E^4-AmfE59bo>viL<Lhk^0m)n#_O6n{CCKWNMw$FXG?skh!
zLssg_tH5tJq6^X;3*X=M^(B=QvT2Whq+%nf8A;q=IpkYK*Ah6gt*zzCmAz2)N12Um
zqtkgyxeg_(6js=*Rz18!AGNkqT-FkN{_n89zwB^2lK&6n7H2(Rm&1UFO_&`<gwQi#
zfVJt|u-<pKFoeIQB%shz)z!m)s^d`ZeSt3x*{n#n`1plG%N51?*dS2@tO|!@^bZcC
zXh)<b<UC{J#4e}hCHu#4uNQ{XGg__#Ld@3t6W1^FY$^D?QD}^{@B=?$I(arueOkQ^
zSQ6p5{B=1R#2)&CWI0d5n>s*%SW=Qjs@Pf;G-*^dy;sFt&pmzy8BiO=ew`PN+KH)g
z@}WwcgY1{FMS6k+`bi-ZyS9fUx1eSUsT8uz!R!?KvDw?dkeEQs!!X^?y8;1nhaA17
zjI%mO9zQ!_eqNxZQ)TS{?UZBv(1362fKr99)Wc*Gc;RhdF)}Tb#U$)sf!}QEa374r
zkFZ#4_yZT&k&A+!;HPFPE+w@k)GjOQNF`)9GI%ba0z#047WaAP-D)FjuY6lS()*|G
zVbru!sW?U{r+r?@<&;ZOk;}W%)F%TkJUlV68Uj%Bj)oXJG**^={3HD<hJVMK0=AHU
puF+!R=BjYL?yM<NY4GZ$tofl?aXro|s-1^jddvL3v;L2qe*u%fSfl^|

literal 0
HcmV?d00001

diff --git a/tests/fsck-tests/035-inline-bad-ram-bytes/test.sh b/tests/fsck-tests/035-inline-bad-ram-bytes/test.sh
new file mode 100755
index 000000000000..6f6e2a5ee850
--- /dev/null
+++ b/tests/fsck-tests/035-inline-bad-ram-bytes/test.sh
@@ -0,0 +1,11 @@
+#!/bin/bash
+# Around 2014, btrfs kernel has a regression that create inline extent
+# with ram_bytes offset by one.
+# This old regression could be caught by tree-check code.
+# This test case will check if btrfs check could detect and repair it.
+
+source "$TEST_TOP/common"
+
+check_prereq btrfs
+
+check_all_images
-- 
2.17.1

Re: [PATCH 0/6] btrfs-progs: Fixes inline ram_bytes related bugs

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 2982 bytes --]

To Steve,

This patchset should fix your problem.

Would you please have a try?

Thanks,
Qu

On 2018年06月06日 15:27, Qu Wenruo wrote:
> The patchset can be fetched from github (*):
> https://github.com/adam900710/btrfs-progs/tree/inline_ram_bytes
> 
> It's based on David's devel branch, whose HEAD is:
> commit 0d1c5812e28e286648781c7b35b542311cc01aa4 (david/devel)
> Author: Matthias Benkard <matthias.benkard@egym.de>
> Date:   Wed Apr 25 16:34:54 2018 +0200
> 
>     btrfs-progs: mkfs: traverse_directory: Reset error code on continue
> 
> Reported-by Steve Leung <sjleung@shaw.ca>, his old btrfs (at least
> offending inodes are from 2014) has inline uncompressed extent, while
> its ram_bytes mismatch with item size.
> 
> Latest kernel tree check catches this bug, while we failed to detect by
> dump-tree.
> 
> It turns out that btrfs-progs is doing something evil to avoid reading
> ram_bytes from inline uncompressed extent.
> 
> 
> So this patchset will address all such ram_bytes related problems.
> 
> The 1st patch is a not-so-relative fix for restore, which is using
> ram_bytes for decompress. Although thanks to the compression header, we
> won't read out-of-boundary, but fixing it is never a bad thing.
> 
> The 2nd patch will get rid of the evil btrfs_file_extent_inline_len()
> which hides raw ram_bytes from us, and fooling us for a long long time.
> 
> The 3rd~5th patches introduce check/repair function for both original
> and lowmem mode (although lowmem mode can detect it even before this patch).
> 
> The last one is the test case for it as usual.
> 
> *: Or should I just migrate to gitlab after M$ acquired github?
> 
> Qu Wenruo (6):
>   btrfs-progs: restore: Fix wrong compressed item size for decompress()
>   btrfs-progs: Get rid of the confusing btrfs_file_extent_inline_len()
>   btrfs-progs: check/original: Detect and repair wrong inline ram_bytes
>   btrfs-progs: check/lowmem: Prepare check_file_extent() to handle
>     repair
>   btrfs-progs: check/lowmem: Repair wrong inlien ram_bytes for
>     uncompressed extent
>   btrfs-progs: fsck-tests: Add test case for corrupted inline ram_bytes
> 
>  check/main.c                                  |  46 ++++++-
>  check/mode-lowmem.c                           | 120 ++++++++++++++----
>  check/mode-original.h                         |   1 +
>  cmds-restore.c                                |   5 +-
>  ctree.h                                       |  22 ----
>  file.c                                        |   3 +-
>  print-tree.c                                  |   4 +-
>  .../offset_by_one.img                         | Bin 0 -> 3072 bytes
>  .../035-inline-bad-ram-bytes/test.sh          |  11 ++
>  9 files changed, 157 insertions(+), 55 deletions(-)
>  create mode 100644 tests/fsck-tests/035-inline-bad-ram-bytes/offset_by_one.img
>  create mode 100755 tests/fsck-tests/035-inline-bad-ram-bytes/test.sh
> 


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

Re: [PATCH 0/6] btrfs-progs: Fixes inline ram_bytes related bugs

[Steve Leung]

On 06/06/2018 01:27 AM, Qu Wenruo wrote:
> The patchset can be fetched from github (*):
> https://github.com/adam900710/btrfs-progs/tree/inline_ram_bytes
> 
> It's based on David's devel branch, whose HEAD is:
> commit 0d1c5812e28e286648781c7b35b542311cc01aa4 (david/devel)
> Author: Matthias Benkard <matthias.benkard@egym.de>
> Date:   Wed Apr 25 16:34:54 2018 +0200
> 
>      btrfs-progs: mkfs: traverse_directory: Reset error code on continue
> 
> Reported-by Steve Leung <sjleung@shaw.ca>, his old btrfs (at least
> offending inodes are from 2014) has inline uncompressed extent, while
> its ram_bytes mismatch with item size.

Took a while to run, but:

Tested-by: Steve Leung <sjleung@shaw.ca>

Verifying everything against backups now, but things look good so far.

I'm not 100% sure how to interpret the "btrfs check" output, but it 
sounded like it was going over each subvolume.  And since the corruption 
was referenced by many subvolumes, the same work essentially had to be 
duplicated many times.

Is that a correct assessment?  Would it have saved time to remove a 
bunch of snapshots first before doing "btrfs check"?

Either way, many thanks for getting everything going again!

Steve


> Latest kernel tree check catches this bug, while we failed to detect by
> dump-tree.
> 
> It turns out that btrfs-progs is doing something evil to avoid reading
> ram_bytes from inline uncompressed extent.
> 
> 
> So this patchset will address all such ram_bytes related problems.
> 
> The 1st patch is a not-so-relative fix for restore, which is using
> ram_bytes for decompress. Although thanks to the compression header, we
> won't read out-of-boundary, but fixing it is never a bad thing.
> 
> The 2nd patch will get rid of the evil btrfs_file_extent_inline_len()
> which hides raw ram_bytes from us, and fooling us for a long long time.
> 
> The 3rd~5th patches introduce check/repair function for both original
> and lowmem mode (although lowmem mode can detect it even before this patch).
> 
> The last one is the test case for it as usual.
> 
> *: Or should I just migrate to gitlab after M$ acquired github?
> 
> Qu Wenruo (6):
>    btrfs-progs: restore: Fix wrong compressed item size for decompress()
>    btrfs-progs: Get rid of the confusing btrfs_file_extent_inline_len()
>    btrfs-progs: check/original: Detect and repair wrong inline ram_bytes
>    btrfs-progs: check/lowmem: Prepare check_file_extent() to handle
>      repair
>    btrfs-progs: check/lowmem: Repair wrong inlien ram_bytes for
>      uncompressed extent
>    btrfs-progs: fsck-tests: Add test case for corrupted inline ram_bytes
> 
>   check/main.c                                  |  46 ++++++-
>   check/mode-lowmem.c                           | 120 ++++++++++++++----
>   check/mode-original.h                         |   1 +
>   cmds-restore.c                                |   5 +-
>   ctree.h                                       |  22 ----
>   file.c                                        |   3 +-
>   print-tree.c                                  |   4 +-
>   .../offset_by_one.img                         | Bin 0 -> 3072 bytes
>   .../035-inline-bad-ram-bytes/test.sh          |  11 ++
>   9 files changed, 157 insertions(+), 55 deletions(-)
>   create mode 100644 tests/fsck-tests/035-inline-bad-ram-bytes/offset_by_one.img
>   create mode 100755 tests/fsck-tests/035-inline-bad-ram-bytes/test.sh
> 

Re: [PATCH 0/6] btrfs-progs: Fixes inline ram_bytes related bugs

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 4364 bytes --]



On 2018年06月08日 12:37, Steve Leung wrote:
> On 06/06/2018 01:27 AM, Qu Wenruo wrote:
>> The patchset can be fetched from github (*):
>> https://github.com/adam900710/btrfs-progs/tree/inline_ram_bytes
>>
>> It's based on David's devel branch, whose HEAD is:
>> commit 0d1c5812e28e286648781c7b35b542311cc01aa4 (david/devel)
>> Author: Matthias Benkard <matthias.benkard@egym.de>
>> Date:   Wed Apr 25 16:34:54 2018 +0200
>>
>>      btrfs-progs: mkfs: traverse_directory: Reset error code on continue
>>
>> Reported-by Steve Leung <sjleung@shaw.ca>, his old btrfs (at least
>> offending inodes are from 2014) has inline uncompressed extent, while
>> its ram_bytes mismatch with item size.
> 
> Took a while to run, but:
> 
> Tested-by: Steve Leung <sjleung@shaw.ca>
> 
> Verifying everything against backups now, but things look good so far.
> 
> I'm not 100% sure how to interpret the "btrfs check" output, but it
> sounded like it was going over each subvolume.  And since the corruption
> was referenced by many subvolumes, the same work essentially had to be
> duplicated many times.

Yes, that's the case.

That would be a little like the way we do in kernel balance, thus it's
not implemented in btrfs-progs, so it would take a long time before we
have similar facility to fix it properly.

Or, I could fix it in another way, break the CoW behavior and do
in-place fix.
But if anyone wants to interrupt the fix, it may cause disaster.

> 
> Is that a correct assessment?  Would it have saved time to remove a
> bunch of snapshots first before doing "btrfs check"?

Yep, it would save a lot of time.

Thanks,
Qu

> 
> Either way, many thanks for getting everything going again!
> 
> Steve
> 
> 
>> Latest kernel tree check catches this bug, while we failed to detect by
>> dump-tree.
>>
>> It turns out that btrfs-progs is doing something evil to avoid reading
>> ram_bytes from inline uncompressed extent.
>>
>>
>> So this patchset will address all such ram_bytes related problems.
>>
>> The 1st patch is a not-so-relative fix for restore, which is using
>> ram_bytes for decompress. Although thanks to the compression header, we
>> won't read out-of-boundary, but fixing it is never a bad thing.
>>
>> The 2nd patch will get rid of the evil btrfs_file_extent_inline_len()
>> which hides raw ram_bytes from us, and fooling us for a long long time.
>>
>> The 3rd~5th patches introduce check/repair function for both original
>> and lowmem mode (although lowmem mode can detect it even before this
>> patch).
>>
>> The last one is the test case for it as usual.
>>
>> *: Or should I just migrate to gitlab after M$ acquired github?
>>
>> Qu Wenruo (6):
>>    btrfs-progs: restore: Fix wrong compressed item size for decompress()
>>    btrfs-progs: Get rid of the confusing btrfs_file_extent_inline_len()
>>    btrfs-progs: check/original: Detect and repair wrong inline ram_bytes
>>    btrfs-progs: check/lowmem: Prepare check_file_extent() to handle
>>      repair
>>    btrfs-progs: check/lowmem: Repair wrong inlien ram_bytes for
>>      uncompressed extent
>>    btrfs-progs: fsck-tests: Add test case for corrupted inline ram_bytes
>>
>>   check/main.c                                  |  46 ++++++-
>>   check/mode-lowmem.c                           | 120 ++++++++++++++----
>>   check/mode-original.h                         |   1 +
>>   cmds-restore.c                                |   5 +-
>>   ctree.h                                       |  22 ----
>>   file.c                                        |   3 +-
>>   print-tree.c                                  |   4 +-
>>   .../offset_by_one.img                         | Bin 0 -> 3072 bytes
>>   .../035-inline-bad-ram-bytes/test.sh          |  11 ++
>>   9 files changed, 157 insertions(+), 55 deletions(-)
>>   create mode 100644
>> tests/fsck-tests/035-inline-bad-ram-bytes/offset_by_one.img
>>   create mode 100755 tests/fsck-tests/035-inline-bad-ram-bytes/test.sh
>>
> 
> 


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

Re: [PATCH 0/6] btrfs-progs: Fixes inline ram_bytes related bugs

[David Sterba]

On Thu, Jun 07, 2018 at 10:37:16PM -0600, Steve Leung wrote:
> On 06/06/2018 01:27 AM, Qu Wenruo wrote:
> > The patchset can be fetched from github (*):
> > https://github.com/adam900710/btrfs-progs/tree/inline_ram_bytes
> > 
> > It's based on David's devel branch, whose HEAD is:
> > commit 0d1c5812e28e286648781c7b35b542311cc01aa4 (david/devel)
> > Author: Matthias Benkard <matthias.benkard@egym.de>
> > Date:   Wed Apr 25 16:34:54 2018 +0200
> > 
> >      btrfs-progs: mkfs: traverse_directory: Reset error code on continue
> > 
> > Reported-by Steve Leung <sjleung@shaw.ca>, his old btrfs (at least
> > offending inodes are from 2014) has inline uncompressed extent, while
> > its ram_bytes mismatch with item size.
> 
> Took a while to run, but:
> 
> Tested-by: Steve Leung <sjleung@shaw.ca>
> 
> Verifying everything against backups now, but things look good so far.

Thank you very much for testing!

Re: [PATCH 0/6] btrfs-progs: Fixes inline ram_bytes related bugs

[David Sterba]

On Wed, Jun 06, 2018 at 03:27:11PM +0800, Qu Wenruo wrote:
> The patchset can be fetched from github (*):
> https://github.com/adam900710/btrfs-progs/tree/inline_ram_bytes
> 
> It's based on David's devel branch, whose HEAD is:
> commit 0d1c5812e28e286648781c7b35b542311cc01aa4 (david/devel)
> Author: Matthias Benkard <matthias.benkard@egym.de>
> Date:   Wed Apr 25 16:34:54 2018 +0200
> 
>     btrfs-progs: mkfs: traverse_directory: Reset error code on continue
> 
> Reported-by Steve Leung <sjleung@shaw.ca>, his old btrfs (at least
> offending inodes are from 2014) has inline uncompressed extent, while
> its ram_bytes mismatch with item size.
> 
> Latest kernel tree check catches this bug, while we failed to detect by
> dump-tree.
> 
> It turns out that btrfs-progs is doing something evil to avoid reading
> ram_bytes from inline uncompressed extent.
> 
> 
> So this patchset will address all such ram_bytes related problems.

Thanks, series applied.

> The 1st patch is a not-so-relative fix for restore, which is using
> ram_bytes for decompress. Although thanks to the compression header, we
> won't read out-of-boundary, but fixing it is never a bad thing.
> 
> The 2nd patch will get rid of the evil btrfs_file_extent_inline_len()
> which hides raw ram_bytes from us, and fooling us for a long long time.
> 
> The 3rd~5th patches introduce check/repair function for both original
> and lowmem mode (although lowmem mode can detect it even before this patch).
> 
> The last one is the test case for it as usual.
> 
> *: Or should I just migrate to gitlab after M$ acquired github?

About that, I have a gitlab account and auto-synced the btrfs-progs
repository from github, so I guess it can be used to fork the code.  The
issues are disabled, that's kind of tied to github, but mail bugreports
work and I think technically we don't have to stick to just one issue
tracker though it's a bit of work on our side to update the status if
the reports are duplicated.