Re: [PATCH 04/13] btrfs-progs: lowmem: fix false alert about the existence of gaps in the check_file_extent

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

On 2018/10/23 下午5:41, Su Yue wrote:
> From: Lu Fengqi <lufq.fnst@cn.fujitsu.com>
> 
> The 'end' parameter of check_file_extent tracks the ending offset of the
> last checked extent. This is used to detect gaps between adjacent extents.
> 
> Currently such gaps are wrongly detected since for regular extents only
> the size of the extent is added to the 'end' parameter. This results in
> wrongly considering all extents of a file as having gaps between them
> when only 2 of them really have a gap as seen in the example below.
> 
> Solution:
> The extent_end variable should set to the sum of the offset and the
> extent_num_bytes of the file extent.
> 
> Example:
> Suppose that lowmem check the following file extent of inode 257.
> 
>         item 6 key (257 EXTENT_DATA 0) itemoff 15813 itemsize 53
>                 generation 6 type 1 (regular)
>                 extent data disk byte 13631488 nr 4096
>                 extent data offset 0 nr 4096 ram 4096
>                 extent compression 0 (none)
>         item 7 key (257 EXTENT_DATA 8192) itemoff 15760 itemsize 53
>                 generation 6 type 1 (regular)
>                 extent data disk byte 13631488 nr 4096
>                 extent data offset 0 nr 4096 ram 4096
>                 extent compression 0 (none)
>         item 8 key (257 EXTENT_DATA 12288) itemoff 15707 itemsize 53
>                 generation 6 type 1 (regular)
>                 extent data disk byte 13631488 nr 4096
>                 extent data offset 0 nr 4096 ram 4096
>                 extent compression 0 (none)
> 
> For inode 257, check_inode_item set extent_end to 0, then call
> check_file_extent to check item {6,7,8}.
> item 6)
> 	offset(0) == extent_end(0)
> 	extent_end = extent_end(0) + extent_num_bytes(4096)
> item 7)
> 	offset(8192) != extent_end(4096)
> 	extent_end = extent_end(4096) + extent_num_bytes(4096)
> 			^^^

The problem is all about a gap screwing up all later accounting.

So it's indeed a problem.

> 	The old extent_end should replace by offset(8192).
> item 8)
> 	offset(12288) != extent_end(8192)
> 		^^^
> 	But there is no gap between item {7,8}.
> 
> Fixes: d88da10ddd42 ("btrfs-progs: check: introduce function to check file extent")
> Signed-off-by: Lu Fengqi <lufq.fnst@cn.fujitsu.com>

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

Thanks,
Qu

> ---
>  check/mode-lowmem.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
> index 769b3141de8b..35fe1adf58e6 100644
> --- a/check/mode-lowmem.c
> +++ b/check/mode-lowmem.c
> @@ -1985,7 +1985,7 @@ static int check_file_extent(struct btrfs_root *root, struct btrfs_path *path,
>  		}
>  	}
>  
> -	*end += extent_num_bytes;
> +	*end = fkey.offset + extent_num_bytes;
>  	if (!is_hole)
>  		*size += extent_num_bytes;
>  
>