diff mbox series

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

Message ID 20240712064716.3385793-2-leo.lilong@huawei.com (mailing list archive)
State Accepted, archived
Headers show
Series [1/2] xfs/016: fix test fail when head equal to near_end_min | expand

Commit Message

Long Li July 12, 2024, 6:47 a.m. UTC
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(-)

Comments

Darrick J. Wong July 12, 2024, 3:39 p.m. UTC | #1
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
> 
>
diff mbox series

Patch

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.
 #     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