Re: [PATCH 2/2] xfs/242: fix test failure due to incorrect filtering in _filter_bmap

["Darrick J. Wong" <djwong@kernel.org>]

On Fri, Jul 12, 2024 at 02:47:16PM +0800, Long Li wrote:
> I got a failure in xfs/242 as follows, it can be easily reproduced
> when I run xfs/242 as a cyclic test.
> 
>   13. data -> unwritten -> data
>   0: [0..127]: data
>   -1: [128..511]: unwritten
>   -2: [512..639]: data
>   +1: [128..639]: unwritten
> 
> The root cause, as Dave pointed out in previous email [1], is that
> _filter_bmap may incorrectly match the AG-OFFSET in column 5 for datadev
> files. On the other hand, _filter_bmap missing a "next" to jump out when
> it matches "data" in the 5th column, otherwise it might print the result
> twice. The issue was introduced by commit 7d5d3f77154e ("xfs/242: fix
> _filter_bmap for xfs_io bmap that does rt file properly"). The failure
> disappeared when I retest xfs/242 by reverted commit 7d5d3f77154e.
> 
> Fix it by matching the 7th column first and then the 5th column in
> _filter_bmap, because the rtdev file only has 5 columns in the `bmap -vp`
> output.
> 
> [1] https://lore.kernel.org/all/Zh9UkHEesvrpSQ7J@dread.disaster.area/
> Signed-off-by: Long Li <leo.lilong@huawei.com>
> ---
>  common/punch | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/common/punch b/common/punch
> index 9e730404..43ccab69 100644
> --- a/common/punch
> +++ b/common/punch
> @@ -188,7 +188,10 @@ _filter_hole_fiemap()
>  	_coalesce_extents
>  }
>  
> -# Column 7 for datadev files and column 5 for rtdev files
> +# Column 7 for datadev files and column 5 for rtdev files, To prevent the
> +# 5th column in datadev files from being potentially matched incorrectly,
> +# we need to match Column 7 for datadev files first, because the rtdev
> +# file only has 5 columns in the `bmap -vp` output.

Yeah, checking column 7 before 5 seems reasonable.  Longer term, maybe
these tools should emit json to cut down on the amount of string parsing
that fstests has to do.

Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

>  #     10000 Unwritten preallocated extent
>  #     01000 Doesn't begin on stripe unit
>  #     00100 Doesn't end   on stripe unit
> @@ -201,18 +204,19 @@ _filter_bmap()
>  			print $1, $2, $3;
>  			next;
>  		}
> -		$5 ~ /1[01][01][01][01]/ {
> +		$7 ~ /1[01][01][01][01]/ {
>  			print $1, $2, "unwritten";
>  			next;
>  		}
> -		$5 ~ /0[01][01][01][01]/ {
> +		$7 ~ /0[01][01][01][01]/ {
>  			print $1, $2, "data"
> +			next;
>  		}
> -		$7 ~ /1[01][01][01][01]/ {
> +		$5 ~ /1[01][01][01][01]/ {
>  			print $1, $2, "unwritten";
>  			next;
>  		}
> -		$7 ~ /0[01][01][01][01]/ {
> +		$5 ~ /0[01][01][01][01]/ {
>  			print $1, $2, "data"
>  		}' |
>  	_coalesce_extents
> -- 
> 2.39.2
> 
>