[PATCH 0/3] Misc refactoring of check_file_extent

[PATCH 0/3] Misc refactoring of check_file_extent

[Nikolay Borisov]

While looking at check_file_extent I thought that the code might be a bit 
cleaner than it actually is and cleaner as well. The first patch factors out 
the code dealing with inline extents into a separate function aptly named 
check_file_extent_inline. This allows to remove some inline-specific variable 
from check_file_extent. Patch 2 just moves the final check in the new function 
into the already existing branch handling the !compressed case. Finally 
the check which detects unknown extent types is moved first in check_file_extent, 
followed by the code to handle inline extents and finally the existing code to 
handle regular/prealloc extents is left intact. 

This patchset brings no functional changes. 

Nikolay Borisov (3):
  btrfs-progs: check: lowmem: Factor out inline extent checking code in
    its own function
  btrfs-progs: check: lowmem: Refactor extent len test in
    check_file_extent_inline
  btrfs-progs: check: lowmem: Refactor extent type checks in
    check_file_extent

 check/mode-lowmem.c | 151 ++++++++++++++++++++++++++------------------
 1 file changed, 89 insertions(+), 62 deletions(-)

-- 
2.17.1

[PATCH 1/3] btrfs-progs: check: lowmem: Factor out inline extent checking code in its own function

[Nikolay Borisov]

Since the inline extent code can be largely self-sufficient, factor
it out from check_file_extent. No functional changes.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 check/mode-lowmem.c | 142 ++++++++++++++++++++++++++------------------
 1 file changed, 83 insertions(+), 59 deletions(-)

diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index 1bce44f5658a..48c1537e7440 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -1800,6 +1800,87 @@ static int repair_inline_ram_bytes(struct btrfs_root *root,
 	return ret;
 }
 
+
+static int check_file_extent_inline(struct btrfs_root *root,
+				    struct btrfs_path *path, u64 *size,
+				    u64 *end)
+{
+	u32 max_inline_extent_size = min_t(u32, root->fs_info->sectorsize - 1,
+				BTRFS_MAX_INLINE_DATA_SIZE(root->fs_info));
+	struct extent_buffer *node = path->nodes[0];
+	struct btrfs_item *e = btrfs_item_nr(0);
+	struct btrfs_file_extent_item *fi;
+	struct btrfs_key fkey;
+	u64 extent_num_bytes;
+	u32 item_inline_len;
+	int ret;
+	int compressed = 0;
+	int err = 0;
+
+	fi = btrfs_item_ptr(node, path->slots[0],
+			    struct btrfs_file_extent_item);
+	item_inline_len = btrfs_file_extent_inline_item_len(node, e);
+	extent_num_bytes = btrfs_file_extent_ram_bytes(node, fi);
+	compressed = btrfs_file_extent_compression(node, fi);
+	btrfs_item_key_to_cpu(node, &fkey, path->slots[0]);
+
+	if (extent_num_bytes == 0) {
+		error(
+"root %llu EXTENT_DATA[%llu %llu] has empty inline extent",
+				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->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,
+				max_inline_extent_size);
+				err |= FILE_EXTENT_ERROR;
+		}
+
+	} else {
+
+		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,
+				extent_num_bytes, max_inline_extent_size);
+				err |= FILE_EXTENT_ERROR;
+		}
+
+	}
+	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,
+				extent_num_bytes, item_inline_len);
+		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;
+
+	return err;
+}
+
 /*
  * Check file extent datasum/hole, update the size of the file extents,
  * check and update the last offset of the file extent.
@@ -1824,8 +1905,6 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_path *path,
 	u64 csum_found;		/* In byte size, sectorsize aligned */
 	u64 search_start;	/* Logical range start we search for csum */
 	u64 search_len;		/* Logical range len we search for csum */
-	u32 max_inline_extent_size = min_t(u32, root->fs_info->sectorsize - 1,
-				BTRFS_MAX_INLINE_DATA_SIZE(root->fs_info));
 	unsigned int extent_type;
 	unsigned int is_hole;
 	int slot = path->slots[0];
@@ -1838,63 +1917,8 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_path *path,
 
 	/* Check inline extent */
 	extent_type = btrfs_file_extent_type(node, fi);
-	if (extent_type == BTRFS_FILE_EXTENT_INLINE) {
-		struct btrfs_item *e = btrfs_item_nr(slot);
-		u32 item_inline_len;
-
-		item_inline_len = btrfs_file_extent_inline_item_len(node, e);
-		extent_num_bytes = btrfs_file_extent_ram_bytes(node, fi);
-		compressed = btrfs_file_extent_compression(node, fi);
-		if (extent_num_bytes == 0) {
-			error(
-		"root %llu EXTENT_DATA[%llu %llu] has empty inline extent",
-				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->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,
-					max_inline_extent_size);
-				err |= FILE_EXTENT_ERROR;
-			}
-		} else {
-			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,
- 				extent_num_bytes, max_inline_extent_size);
-				err |= FILE_EXTENT_ERROR;
-			}
-		}
-		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,
-				extent_num_bytes, item_inline_len);
-			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;
-		return err;
-	}
+	if (extent_type == BTRFS_FILE_EXTENT_INLINE)
+		return check_file_extent_inline(root, path, size, end);
 
 	/* Check extent type */
 	if (extent_type != BTRFS_FILE_EXTENT_REG &&
-- 
2.17.1

Re: [PATCH 1/3] btrfs-progs: check: lowmem: Factor out inline extent checking code in its own function

[Qu Wenruo]

On 2018/9/13 下午8:05, Nikolay Borisov wrote:
> Since the inline extent code can be largely self-sufficient, factor
> it out from check_file_extent. No functional changes.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Indeed makes more sense to refactor inline check out.

Thanks,
Qu

> ---
>  check/mode-lowmem.c | 142 ++++++++++++++++++++++++++------------------
>  1 file changed, 83 insertions(+), 59 deletions(-)
> 
> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
> index 1bce44f5658a..48c1537e7440 100644
> --- a/check/mode-lowmem.c
> +++ b/check/mode-lowmem.c
> @@ -1800,6 +1800,87 @@ static int repair_inline_ram_bytes(struct btrfs_root *root,
>  	return ret;
>  }
>  
> +
> +static int check_file_extent_inline(struct btrfs_root *root,
> +				    struct btrfs_path *path, u64 *size,
> +				    u64 *end)
> +{
> +	u32 max_inline_extent_size = min_t(u32, root->fs_info->sectorsize - 1,
> +				BTRFS_MAX_INLINE_DATA_SIZE(root->fs_info));
> +	struct extent_buffer *node = path->nodes[0];
> +	struct btrfs_item *e = btrfs_item_nr(0);
> +	struct btrfs_file_extent_item *fi;
> +	struct btrfs_key fkey;
> +	u64 extent_num_bytes;
> +	u32 item_inline_len;
> +	int ret;
> +	int compressed = 0;
> +	int err = 0;
> +
> +	fi = btrfs_item_ptr(node, path->slots[0],
> +			    struct btrfs_file_extent_item);
> +	item_inline_len = btrfs_file_extent_inline_item_len(node, e);
> +	extent_num_bytes = btrfs_file_extent_ram_bytes(node, fi);
> +	compressed = btrfs_file_extent_compression(node, fi);
> +	btrfs_item_key_to_cpu(node, &fkey, path->slots[0]);
> +
> +	if (extent_num_bytes == 0) {
> +		error(
> +"root %llu EXTENT_DATA[%llu %llu] has empty inline extent",
> +				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->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,
> +				max_inline_extent_size);
> +				err |= FILE_EXTENT_ERROR;
> +		}
> +
> +	} else {
> +
> +		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,
> +				extent_num_bytes, max_inline_extent_size);
> +				err |= FILE_EXTENT_ERROR;
> +		}
> +
> +	}
> +	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,
> +				extent_num_bytes, item_inline_len);
> +		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;
> +
> +	return err;
> +}
> +
>  /*
>   * Check file extent datasum/hole, update the size of the file extents,
>   * check and update the last offset of the file extent.
> @@ -1824,8 +1905,6 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_path *path,
>  	u64 csum_found;		/* In byte size, sectorsize aligned */
>  	u64 search_start;	/* Logical range start we search for csum */
>  	u64 search_len;		/* Logical range len we search for csum */
> -	u32 max_inline_extent_size = min_t(u32, root->fs_info->sectorsize - 1,
> -				BTRFS_MAX_INLINE_DATA_SIZE(root->fs_info));
>  	unsigned int extent_type;
>  	unsigned int is_hole;
>  	int slot = path->slots[0];
> @@ -1838,63 +1917,8 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_path *path,
>  
>  	/* Check inline extent */
>  	extent_type = btrfs_file_extent_type(node, fi);
> -	if (extent_type == BTRFS_FILE_EXTENT_INLINE) {
> -		struct btrfs_item *e = btrfs_item_nr(slot);
> -		u32 item_inline_len;
> -
> -		item_inline_len = btrfs_file_extent_inline_item_len(node, e);
> -		extent_num_bytes = btrfs_file_extent_ram_bytes(node, fi);
> -		compressed = btrfs_file_extent_compression(node, fi);
> -		if (extent_num_bytes == 0) {
> -			error(
> -		"root %llu EXTENT_DATA[%llu %llu] has empty inline extent",
> -				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->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,
> -					max_inline_extent_size);
> -				err |= FILE_EXTENT_ERROR;
> -			}
> -		} else {
> -			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,
> - 				extent_num_bytes, max_inline_extent_size);
> -				err |= FILE_EXTENT_ERROR;
> -			}
> -		}
> -		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,
> -				extent_num_bytes, item_inline_len);
> -			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;
> -		return err;
> -	}
> +	if (extent_type == BTRFS_FILE_EXTENT_INLINE)
> +		return check_file_extent_inline(root, path, size, end);
>  
>  	/* Check extent type */
>  	if (extent_type != BTRFS_FILE_EXTENT_REG &&
> 

Re: [PATCH 1/3] btrfs-progs: check: lowmem: Factor out inline extent checking code in its own function

[misono.tomohiro]

Hello,

fsck-test 006 fails for low-mem mode in current devel branch and bisect points this.

> Since the inline extent code can be largely self-sufficient, factor
> it out from check_file_extent. No functional changes.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
>  check/mode-lowmem.c | 142 ++++++++++++++++++++++++++------------------
>  1 file changed, 83 insertions(+), 59 deletions(-)
> 
> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
> index 1bce44f5658a..48c1537e7440 100644
> --- a/check/mode-lowmem.c
> +++ b/check/mode-lowmem.c
> @@ -1800,6 +1800,87 @@ static int repair_inline_ram_bytes(struct btrfs_root *root,
>  	return ret;
>  }
>  
> +
> +static int check_file_extent_inline(struct btrfs_root *root,
> +				    struct btrfs_path *path, u64 *size,
> +				    u64 *end)
> +{
> +	u32 max_inline_extent_size = min_t(u32, root->fs_info->sectorsize - 1,
> +				BTRFS_MAX_INLINE_DATA_SIZE(root->fs_info));
> +	struct extent_buffer *node = path->nodes[0];
> +	struct btrfs_item *e = btrfs_item_nr(0);
                               btrfs_item_nr(path->slots[0])

I think this fixes the problem.
Thanks.

Misono

> +	struct btrfs_file_extent_item *fi;
> +	struct btrfs_key fkey;
> +	u64 extent_num_bytes;
> +	u32 item_inline_len;
> +	int ret;
> +	int compressed = 0;
> +	int err = 0;
> +
> +	fi = btrfs_item_ptr(node, path->slots[0],
> +			    struct btrfs_file_extent_item);
> +	item_inline_len = btrfs_file_extent_inline_item_len(node, e);
> +	extent_num_bytes = btrfs_file_extent_ram_bytes(node, fi);
> +	compressed = btrfs_file_extent_compression(node, fi);
> +	btrfs_item_key_to_cpu(node, &fkey, path->slots[0]);
> +
> +	if (extent_num_bytes == 0) {
> +		error(
> +"root %llu EXTENT_DATA[%llu %llu] has empty inline extent",
> +				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->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,
> +				max_inline_extent_size);
> +				err |= FILE_EXTENT_ERROR;
> +		}
> +
> +	} else {
> +
> +		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,
> +				extent_num_bytes, max_inline_extent_size);
> +				err |= FILE_EXTENT_ERROR;
> +		}
> +
> +	}
> +	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,
> +				extent_num_bytes, item_inline_len);
> +		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;
> +
> +	return err;
> +}
> +
>  /*
>   * Check file extent datasum/hole, update the size of the file extents,
>   * check and update the last offset of the file extent.
> @@ -1824,8 +1905,6 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_path *path,
>  	u64 csum_found;		/* In byte size, sectorsize aligned */
>  	u64 search_start;	/* Logical range start we search for csum */
>  	u64 search_len;		/* Logical range len we search for csum */
> -	u32 max_inline_extent_size = min_t(u32, root->fs_info->sectorsize - 1,
> -				BTRFS_MAX_INLINE_DATA_SIZE(root->fs_info));
>  	unsigned int extent_type;
>  	unsigned int is_hole;
>  	int slot = path->slots[0];
> @@ -1838,63 +1917,8 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_path *path,
>  
>  	/* Check inline extent */
>  	extent_type = btrfs_file_extent_type(node, fi);
> -	if (extent_type == BTRFS_FILE_EXTENT_INLINE) {
> -		struct btrfs_item *e = btrfs_item_nr(slot);
> -		u32 item_inline_len;
> -
> -		item_inline_len = btrfs_file_extent_inline_item_len(node, e);
> -		extent_num_bytes = btrfs_file_extent_ram_bytes(node, fi);
> -		compressed = btrfs_file_extent_compression(node, fi);
> -		if (extent_num_bytes == 0) {
> -			error(
> -		"root %llu EXTENT_DATA[%llu %llu] has empty inline extent",
> -				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->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,
> -					max_inline_extent_size);
> -				err |= FILE_EXTENT_ERROR;
> -			}
> -		} else {
> -			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,
> - 				extent_num_bytes, max_inline_extent_size);
> -				err |= FILE_EXTENT_ERROR;
> -			}
> -		}
> -		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,
> -				extent_num_bytes, item_inline_len);
> -			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;
> -		return err;
> -	}
> +	if (extent_type == BTRFS_FILE_EXTENT_INLINE)
> +		return check_file_extent_inline(root, path, size, end);
>  
>  	/* Check extent type */
>  	if (extent_type != BTRFS_FILE_EXTENT_REG &&
> 

Re: [PATCH 1/3] btrfs-progs: check: lowmem: Factor out inline extent checking code in its own function

[Nikolay Borisov]

On 31.10.18 г. 11:35 ч., misono.tomohiro@fujitsu.com wrote:
> Hello,
> 
> fsck-test 006 fails for low-mem mode in current devel branch and bisect points this.
> 
>> Since the inline extent code can be largely self-sufficient, factor
>> it out from check_file_extent. No functional changes.
>>
>> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
>> ---
>>  check/mode-lowmem.c | 142 ++++++++++++++++++++++++++------------------
>>  1 file changed, 83 insertions(+), 59 deletions(-)
>>
>> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
>> index 1bce44f5658a..48c1537e7440 100644
>> --- a/check/mode-lowmem.c
>> +++ b/check/mode-lowmem.c
>> @@ -1800,6 +1800,87 @@ static int repair_inline_ram_bytes(struct btrfs_root *root,
>>  	return ret;
>>  }
>>  
>> +
>> +static int check_file_extent_inline(struct btrfs_root *root,
>> +				    struct btrfs_path *path, u64 *size,
>> +				    u64 *end)
>> +{
>> +	u32 max_inline_extent_size = min_t(u32, root->fs_info->sectorsize - 1,
>> +				BTRFS_MAX_INLINE_DATA_SIZE(root->fs_info));
>> +	struct extent_buffer *node = path->nodes[0];
>> +	struct btrfs_item *e = btrfs_item_nr(0);
>                                btrfs_item_nr(path->slots[0])
> 
> I think this fixes the problem.

Indeed, the original code uses path->slots[0] as the slot whereas I've
fixed that at slow 0, which of course is not always going to be the case.

David will you fold this in the original patch ?

> Thanks.
> 
> Misono
> 
>> +	struct btrfs_file_extent_item *fi;
>> +	struct btrfs_key fkey;
>> +	u64 extent_num_bytes;
>> +	u32 item_inline_len;
>> +	int ret;
>> +	int compressed = 0;
>> +	int err = 0;
>> +
>> +	fi = btrfs_item_ptr(node, path->slots[0],
>> +			    struct btrfs_file_extent_item);
>> +	item_inline_len = btrfs_file_extent_inline_item_len(node, e);
>> +	extent_num_bytes = btrfs_file_extent_ram_bytes(node, fi);
>> +	compressed = btrfs_file_extent_compression(node, fi);
>> +	btrfs_item_key_to_cpu(node, &fkey, path->slots[0]);
>> +
>> +	if (extent_num_bytes == 0) {
>> +		error(
>> +"root %llu EXTENT_DATA[%llu %llu] has empty inline extent",
>> +				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->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,
>> +				max_inline_extent_size);
>> +				err |= FILE_EXTENT_ERROR;
>> +		}
>> +
>> +	} else {
>> +
>> +		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,
>> +				extent_num_bytes, max_inline_extent_size);
>> +				err |= FILE_EXTENT_ERROR;
>> +		}
>> +
>> +	}
>> +	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,
>> +				extent_num_bytes, item_inline_len);
>> +		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;
>> +
>> +	return err;
>> +}
>> +
>>  /*
>>   * Check file extent datasum/hole, update the size of the file extents,
>>   * check and update the last offset of the file extent.
>> @@ -1824,8 +1905,6 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_path *path,
>>  	u64 csum_found;		/* In byte size, sectorsize aligned */
>>  	u64 search_start;	/* Logical range start we search for csum */
>>  	u64 search_len;		/* Logical range len we search for csum */
>> -	u32 max_inline_extent_size = min_t(u32, root->fs_info->sectorsize - 1,
>> -				BTRFS_MAX_INLINE_DATA_SIZE(root->fs_info));
>>  	unsigned int extent_type;
>>  	unsigned int is_hole;
>>  	int slot = path->slots[0];
>> @@ -1838,63 +1917,8 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_path *path,
>>  
>>  	/* Check inline extent */
>>  	extent_type = btrfs_file_extent_type(node, fi);
>> -	if (extent_type == BTRFS_FILE_EXTENT_INLINE) {
>> -		struct btrfs_item *e = btrfs_item_nr(slot);
>> -		u32 item_inline_len;
>> -
>> -		item_inline_len = btrfs_file_extent_inline_item_len(node, e);
>> -		extent_num_bytes = btrfs_file_extent_ram_bytes(node, fi);
>> -		compressed = btrfs_file_extent_compression(node, fi);
>> -		if (extent_num_bytes == 0) {
>> -			error(
>> -		"root %llu EXTENT_DATA[%llu %llu] has empty inline extent",
>> -				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->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,
>> -					max_inline_extent_size);
>> -				err |= FILE_EXTENT_ERROR;
>> -			}
>> -		} else {
>> -			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,
>> - 				extent_num_bytes, max_inline_extent_size);
>> -				err |= FILE_EXTENT_ERROR;
>> -			}
>> -		}
>> -		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,
>> -				extent_num_bytes, item_inline_len);
>> -			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;
>> -		return err;
>> -	}
>> +	if (extent_type == BTRFS_FILE_EXTENT_INLINE)
>> +		return check_file_extent_inline(root, path, size, end);
>>  
>>  	/* Check extent type */
>>  	if (extent_type != BTRFS_FILE_EXTENT_REG &&

Re: [PATCH 1/3] btrfs-progs: check: lowmem: Factor out inline extent checking code in its own function

[David Sterba]

On Wed, Oct 31, 2018 at 11:43:20AM +0200, Nikolay Borisov wrote:
> >> --- a/check/mode-lowmem.c
> >> +++ b/check/mode-lowmem.c
> >> @@ -1800,6 +1800,87 @@ static int repair_inline_ram_bytes(struct btrfs_root *root,
> >>  	return ret;
> >>  }
> >>  
> >> +
> >> +static int check_file_extent_inline(struct btrfs_root *root,
> >> +				    struct btrfs_path *path, u64 *size,
> >> +				    u64 *end)
> >> +{
> >> +	u32 max_inline_extent_size = min_t(u32, root->fs_info->sectorsize - 1,
> >> +				BTRFS_MAX_INLINE_DATA_SIZE(root->fs_info));
> >> +	struct extent_buffer *node = path->nodes[0];
> >> +	struct btrfs_item *e = btrfs_item_nr(0);
> >                                btrfs_item_nr(path->slots[0])
> > 
> > I think this fixes the problem.
> 
> Indeed, the original code uses path->slots[0] as the slot whereas I've
> fixed that at slow 0, which of course is not always going to be the case.
> 
> David will you fold this in the original patch ?

Yes I'll fold it. Thanks.

Re: [PATCH 1/3] btrfs-progs: check: lowmem: Factor out inline extent checking code in its own function

[misono.tomohiro]

Hello,

fsck-test 006 fails for low-mem mode in current devel branch and bisect points this.

On 2018/09/13 21:05, Nikolay Borisov wrote:
> Since the inline extent code can be largely self-sufficient, factor
> it out from check_file_extent. No functional changes.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>
> ---
>  check/mode-lowmem.c | 142 ++++++++++++++++++++++++++------------------
>  1 file changed, 83 insertions(+), 59 deletions(-)
> 
> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
> index 1bce44f5658a..48c1537e7440 100644
> --- a/check/mode-lowmem.c
> +++ b/check/mode-lowmem.c
> @@ -1800,6 +1800,87 @@ static int repair_inline_ram_bytes(struct btrfs_root *root,
>  	return ret;
>  }
>  
> +
> +static int check_file_extent_inline(struct btrfs_root *root,
> +				    struct btrfs_path *path, u64 *size,
> +				    u64 *end)
> +{
> +	u32 max_inline_extent_size = min_t(u32, root->fs_info->sectorsize - 1,
> +				BTRFS_MAX_INLINE_DATA_SIZE(root->fs_info));
> +	struct extent_buffer *node = path->nodes[0];
> +	struct btrfs_item *e = btrfs_item_nr(0);
                               btrfs_item_nr(path->slots[0])

I think this fixes the problem.
Thanks.

Misono


> +	struct btrfs_file_extent_item *fi;
> +	struct btrfs_key fkey;
> +	u64 extent_num_bytes;
> +	u32 item_inline_len;
> +	int ret;
> +	int compressed = 0;
> +	int err = 0;
> +
> +	fi = btrfs_item_ptr(node, path->slots[0],
> +			    struct btrfs_file_extent_item);
> +	item_inline_len = btrfs_file_extent_inline_item_len(node, e);
> +	extent_num_bytes = btrfs_file_extent_ram_bytes(node, fi);
> +	compressed = btrfs_file_extent_compression(node, fi);
> +	btrfs_item_key_to_cpu(node, &fkey, path->slots[0]);
> +
> +	if (extent_num_bytes == 0) {
> +		error(
> +"root %llu EXTENT_DATA[%llu %llu] has empty inline extent",
> +				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->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,
> +				max_inline_extent_size);
> +				err |= FILE_EXTENT_ERROR;
> +		}
> +
> +	} else {
> +
> +		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,
> +				extent_num_bytes, max_inline_extent_size);
> +				err |= FILE_EXTENT_ERROR;
> +		}
> +
> +	}
> +	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,
> +				extent_num_bytes, item_inline_len);
> +		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;
> +
> +	return err;
> +}
> +
>  /*
>   * Check file extent datasum/hole, update the size of the file extents,
>   * check and update the last offset of the file extent.
> @@ -1824,8 +1905,6 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_path *path,
>  	u64 csum_found;		/* In byte size, sectorsize aligned */
>  	u64 search_start;	/* Logical range start we search for csum */
>  	u64 search_len;		/* Logical range len we search for csum */
> -	u32 max_inline_extent_size = min_t(u32, root->fs_info->sectorsize - 1,
> -				BTRFS_MAX_INLINE_DATA_SIZE(root->fs_info));
>  	unsigned int extent_type;
>  	unsigned int is_hole;
>  	int slot = path->slots[0];
> @@ -1838,63 +1917,8 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_path *path,
>  
>  	/* Check inline extent */
>  	extent_type = btrfs_file_extent_type(node, fi);
> -	if (extent_type == BTRFS_FILE_EXTENT_INLINE) {
> -		struct btrfs_item *e = btrfs_item_nr(slot);
> -		u32 item_inline_len;
> -
> -		item_inline_len = btrfs_file_extent_inline_item_len(node, e);
> -		extent_num_bytes = btrfs_file_extent_ram_bytes(node, fi);
> -		compressed = btrfs_file_extent_compression(node, fi);
> -		if (extent_num_bytes == 0) {
> -			error(
> -		"root %llu EXTENT_DATA[%llu %llu] has empty inline extent",
> -				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->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,
> -					max_inline_extent_size);
> -				err |= FILE_EXTENT_ERROR;
> -			}
> -		} else {
> -			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,
> - 				extent_num_bytes, max_inline_extent_size);
> -				err |= FILE_EXTENT_ERROR;
> -			}
> -		}
> -		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,
> -				extent_num_bytes, item_inline_len);
> -			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;
> -		return err;
> -	}
> +	if (extent_type == BTRFS_FILE_EXTENT_INLINE)
> +		return check_file_extent_inline(root, path, size, end);
>  
>  	/* Check extent type */
>  	if (extent_type != BTRFS_FILE_EXTENT_REG &&
> 

[PATCH 2/3] btrfs-progs: check: lowmem: Refactor extent len test in check_file_extent_inline

[Nikolay Borisov]

Instead of having another top-level if which checks for
'extent_num_bytes != item_inline_len' only if we are !compressed, just
move the 'if' inside the 'else' branch of the first top-level if, since
it has already checked for !compressed or not. No functional changes.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 check/mode-lowmem.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index 48c1537e7440..3a6fbb33c858 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -1860,19 +1860,20 @@ static int check_file_extent_inline(struct btrfs_root *root,
 				err |= FILE_EXTENT_ERROR;
 		}
 
-	}
-	if (!compressed && extent_num_bytes != item_inline_len) {
-		error(
+
+		if (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,
 				extent_num_bytes, item_inline_len);
-		if (repair) {
-			ret = repair_inline_ram_bytes(root, path,
-						      &extent_num_bytes);
-			if (ret)
+			if (repair) {
+				ret = repair_inline_ram_bytes(root, path,
+							      &extent_num_bytes);
+				if (ret)
+					err |= FILE_EXTENT_ERROR;
+			} else {
 				err |= FILE_EXTENT_ERROR;
-		} else {
-			err |= FILE_EXTENT_ERROR;
+			}
 		}
 	}
 	*end += extent_num_bytes;
-- 
2.17.1

Re: [PATCH 2/3] btrfs-progs: check: lowmem: Refactor extent len test in check_file_extent_inline

[Qu Wenruo]

On 2018/9/13 下午8:05, Nikolay Borisov wrote:
> Instead of having another top-level if which checks for
> 'extent_num_bytes != item_inline_len' only if we are !compressed, just
> move the 'if' inside the 'else' branch of the first top-level if, since
> it has already checked for !compressed or not. No functional changes.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>  check/mode-lowmem.c | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
> index 48c1537e7440..3a6fbb33c858 100644
> --- a/check/mode-lowmem.c
> +++ b/check/mode-lowmem.c
> @@ -1860,19 +1860,20 @@ static int check_file_extent_inline(struct btrfs_root *root,
>  				err |= FILE_EXTENT_ERROR;
>  		}
>  
> -	}
> -	if (!compressed && extent_num_bytes != item_inline_len) {
> -		error(
> +
> +		if (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,
>  				extent_num_bytes, item_inline_len);
> -		if (repair) {
> -			ret = repair_inline_ram_bytes(root, path,
> -						      &extent_num_bytes);
> -			if (ret)
> +			if (repair) {
> +				ret = repair_inline_ram_bytes(root, path,
> +							      &extent_num_bytes);
> +				if (ret)
> +					err |= FILE_EXTENT_ERROR;
> +			} else {
>  				err |= FILE_EXTENT_ERROR;
> -		} else {
> -			err |= FILE_EXTENT_ERROR;
> +			}
>  		}
>  	}
>  	*end += extent_num_bytes;
> 

[PATCH 3/3] btrfs-progs: check: lowmem: Refactor extent type checks in check_file_extent

[Nikolay Borisov]

Make the checks in check_file_extent a bit more explicit. First we check
for unknown type and fail accordingly. Then we check for inline extent
and handle it in the newly introduced check_file_extent_inline. Finally
if none of the above checks triggered then we must have a regular or
prealloc extents.

Signed-off-by: Nikolay Borisov <nborisov@suse.com>
---
 check/mode-lowmem.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index 3a6fbb33c858..aa946a1307de 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -1915,21 +1915,23 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_path *path,
 
 	btrfs_item_key_to_cpu(node, &fkey, slot);
 	fi = btrfs_item_ptr(node, slot, struct btrfs_file_extent_item);
-
-	/* Check inline extent */
 	extent_type = btrfs_file_extent_type(node, fi);
-	if (extent_type == BTRFS_FILE_EXTENT_INLINE)
-		return check_file_extent_inline(root, path, size, end);
 
 	/* Check extent type */
 	if (extent_type != BTRFS_FILE_EXTENT_REG &&
-			extent_type != BTRFS_FILE_EXTENT_PREALLOC) {
+	    extent_type != BTRFS_FILE_EXTENT_PREALLOC &&
+	    extent_type != BTRFS_FILE_EXTENT_INLINE) {
 		err |= FILE_EXTENT_ERROR;
 		error("root %llu EXTENT_DATA[%llu %llu] type bad",
 		      root->objectid, fkey.objectid, fkey.offset);
 		return err;
 	}
 
+	/* Check inline extent */
+	if (extent_type == BTRFS_FILE_EXTENT_INLINE)
+		return check_file_extent_inline(root, path, size, end);
+
+
 	/* Check REG_EXTENT/PREALLOC_EXTENT */
 	disk_bytenr = btrfs_file_extent_disk_bytenr(node, fi);
 	disk_num_bytes = btrfs_file_extent_disk_num_bytes(node, fi);
-- 
2.17.1

Re: [PATCH 3/3] btrfs-progs: check: lowmem: Refactor extent type checks in check_file_extent

[Qu Wenruo]

On 2018/9/13 下午8:05, Nikolay Borisov wrote:
> Make the checks in check_file_extent a bit more explicit. First we check
> for unknown type and fail accordingly. Then we check for inline extent
> and handle it in the newly introduced check_file_extent_inline. Finally
> if none of the above checks triggered then we must have a regular or
> prealloc extents.
> 
> Signed-off-by: Nikolay Borisov <nborisov@suse.com>

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>  check/mode-lowmem.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
> index 3a6fbb33c858..aa946a1307de 100644
> --- a/check/mode-lowmem.c
> +++ b/check/mode-lowmem.c
> @@ -1915,21 +1915,23 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_path *path,
>  
>  	btrfs_item_key_to_cpu(node, &fkey, slot);
>  	fi = btrfs_item_ptr(node, slot, struct btrfs_file_extent_item);
> -
> -	/* Check inline extent */
>  	extent_type = btrfs_file_extent_type(node, fi);
> -	if (extent_type == BTRFS_FILE_EXTENT_INLINE)
> -		return check_file_extent_inline(root, path, size, end);
>  
>  	/* Check extent type */
>  	if (extent_type != BTRFS_FILE_EXTENT_REG &&
> -			extent_type != BTRFS_FILE_EXTENT_PREALLOC) {
> +	    extent_type != BTRFS_FILE_EXTENT_PREALLOC &&
> +	    extent_type != BTRFS_FILE_EXTENT_INLINE) {
>  		err |= FILE_EXTENT_ERROR;
>  		error("root %llu EXTENT_DATA[%llu %llu] type bad",
>  		      root->objectid, fkey.objectid, fkey.offset);
>  		return err;
>  	}
>  
> +	/* Check inline extent */
> +	if (extent_type == BTRFS_FILE_EXTENT_INLINE)
> +		return check_file_extent_inline(root, path, size, end);
> +
> +
>  	/* Check REG_EXTENT/PREALLOC_EXTENT */
>  	disk_bytenr = btrfs_file_extent_disk_bytenr(node, fi);
>  	disk_num_bytes = btrfs_file_extent_disk_num_bytes(node, fi);
> 

Re: [PATCH 0/3] Misc refactoring of check_file_extent

[Lu Fengqi]

On Thu, Sep 13, 2018 at 03:05:04PM +0300, Nikolay Borisov wrote:
>While looking at check_file_extent I thought that the code might be a bit 
>cleaner than it actually is and cleaner as well. The first patch factors out 
>the code dealing with inline extents into a separate function aptly named 
>check_file_extent_inline. This allows to remove some inline-specific variable 
>from check_file_extent. Patch 2 just moves the final check in the new function 
>into the already existing branch handling the !compressed case. Finally 
>the check which detects unknown extent types is moved first in check_file_extent, 
>followed by the code to handle inline extents and finally the existing code to 
>handle regular/prealloc extents is left intact. 
>
>This patchset brings no functional changes. 

For the series,

Reviewed-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>

-- 
Thanks,
Lu

>
>Nikolay Borisov (3):
>  btrfs-progs: check: lowmem: Factor out inline extent checking code in
>    its own function
>  btrfs-progs: check: lowmem: Refactor extent len test in
>    check_file_extent_inline
>  btrfs-progs: check: lowmem: Refactor extent type checks in
>    check_file_extent
>
> check/mode-lowmem.c | 151 ++++++++++++++++++++++++++------------------
> 1 file changed, 89 insertions(+), 62 deletions(-)
>
>-- 
>2.17.1
>
>
>

Re: [PATCH 0/3] Misc refactoring of check_file_extent

[David Sterba]

On Thu, Sep 13, 2018 at 03:05:04PM +0300, Nikolay Borisov wrote:
> While looking at check_file_extent I thought that the code might be a bit 
> cleaner than it actually is and cleaner as well. The first patch factors out 
> the code dealing with inline extents into a separate function aptly named 
> check_file_extent_inline. This allows to remove some inline-specific variable 
> from check_file_extent. Patch 2 just moves the final check in the new function 
> into the already existing branch handling the !compressed case. Finally 
> the check which detects unknown extent types is moved first in check_file_extent, 
> followed by the code to handle inline extents and finally the existing code to 
> handle regular/prealloc extents is left intact. 
> 
> This patchset brings no functional changes. 
> 
> Nikolay Borisov (3):
>   btrfs-progs: check: lowmem: Factor out inline extent checking code in
>     its own function
>   btrfs-progs: check: lowmem: Refactor extent len test in
>     check_file_extent_inline
>   btrfs-progs: check: lowmem: Refactor extent type checks in
>     check_file_extent

Applied, thanks.