[RFC PATCH 0/2] xfs: add error tags for log attribute replay test

[RFC PATCH 0/2] xfs: add error tags for log attribute replay test

[Catherine Hoang]

Hi all,

These are the corresponding kernel changes for the new log attribute replay
test. These are built on top of Allison’s logged attribute patch sets, which
can be viewed here:
https://github.com/allisonhenderson/xfs_work/tree/delayed_attrs_v25_extended

This set adds the new error tags leaf_split and leaf_to_node, which are used
to inject errors in the tests. 

Suggestions and feedback are appreciated!

Catherine

Catherine Hoang (2):
  xfs: add leaf split error tag
  xfs: add leaf to node error tag

 fs/xfs/libxfs/xfs_attr_leaf.c | 6 ++++++
 fs/xfs/libxfs/xfs_da_btree.c  | 6 ++++++
 fs/xfs/libxfs/xfs_errortag.h  | 6 +++++-
 fs/xfs/xfs_error.c            | 6 ++++++
 4 files changed, 23 insertions(+), 1 deletion(-)

-- 
2.25.1

[RFC PATCH 1/2] xfs: add leaf split error tag

[Catherine Hoang]

Add an error tag on xfs_da3_split to test log attribute recovery
and replay.

Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com>
---
 fs/xfs/libxfs/xfs_da_btree.c | 6 ++++++
 fs/xfs/libxfs/xfs_errortag.h | 4 +++-
 fs/xfs/xfs_error.c           | 3 +++
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
index dd7a2dbce1d1..000101783648 100644
--- a/fs/xfs/libxfs/xfs_da_btree.c
+++ b/fs/xfs/libxfs/xfs_da_btree.c
@@ -22,6 +22,7 @@
 #include "xfs_trace.h"
 #include "xfs_buf_item.h"
 #include "xfs_log.h"
+#include "xfs_errortag.h"
 
 /*
  * xfs_da_btree.c
@@ -482,6 +483,11 @@ xfs_da3_split(
 
 	trace_xfs_da_split(state->args);
 
+	if (XFS_TEST_ERROR(false, state->mp, XFS_ERRTAG_LEAF_SPLIT)) {
+		error = -EIO;
+		return error;
+	}
+
 	/*
 	 * Walk back up the tree splitting/inserting/adjusting as necessary.
 	 * If we need to insert and there isn't room, split the node, then
diff --git a/fs/xfs/libxfs/xfs_errortag.h b/fs/xfs/libxfs/xfs_errortag.h
index c15d2340220c..31aeeb94dd5b 100644
--- a/fs/xfs/libxfs/xfs_errortag.h
+++ b/fs/xfs/libxfs/xfs_errortag.h
@@ -60,7 +60,8 @@
 #define XFS_ERRTAG_BMAP_ALLOC_MINLEN_EXTENT		37
 #define XFS_ERRTAG_AG_RESV_FAIL				38
 #define XFS_ERRTAG_LARP					39
-#define XFS_ERRTAG_MAX					40
+#define XFS_ERRTAG_LEAF_SPLIT				40
+#define XFS_ERRTAG_MAX					41
 
 /*
  * Random factors for above tags, 1 means always, 2 means 1/2 time, etc.
@@ -105,5 +106,6 @@
 #define XFS_RANDOM_BMAP_ALLOC_MINLEN_EXTENT		1
 #define XFS_RANDOM_AG_RESV_FAIL				1
 #define XFS_RANDOM_LARP					1
+#define XFS_RANDOM_LEAF_SPLIT				1
 
 #endif /* __XFS_ERRORTAG_H_ */
diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
index d4b2256ba00b..732cb66236c1 100644
--- a/fs/xfs/xfs_error.c
+++ b/fs/xfs/xfs_error.c
@@ -58,6 +58,7 @@ static unsigned int xfs_errortag_random_default[] = {
 	XFS_RANDOM_BMAP_ALLOC_MINLEN_EXTENT,
 	XFS_RANDOM_AG_RESV_FAIL,
 	XFS_RANDOM_LARP,
+	XFS_RANDOM_LEAF_SPLIT,
 };
 
 struct xfs_errortag_attr {
@@ -172,6 +173,7 @@ XFS_ERRORTAG_ATTR_RW(reduce_max_iextents,	XFS_ERRTAG_REDUCE_MAX_IEXTENTS);
 XFS_ERRORTAG_ATTR_RW(bmap_alloc_minlen_extent,	XFS_ERRTAG_BMAP_ALLOC_MINLEN_EXTENT);
 XFS_ERRORTAG_ATTR_RW(ag_resv_fail, XFS_ERRTAG_AG_RESV_FAIL);
 XFS_ERRORTAG_ATTR_RW(larp,		XFS_ERRTAG_LARP);
+XFS_ERRORTAG_ATTR_RW(leaf_split,	XFS_ERRTAG_LEAF_SPLIT);
 
 static struct attribute *xfs_errortag_attrs[] = {
 	XFS_ERRORTAG_ATTR_LIST(noerror),
@@ -214,6 +216,7 @@ static struct attribute *xfs_errortag_attrs[] = {
 	XFS_ERRORTAG_ATTR_LIST(bmap_alloc_minlen_extent),
 	XFS_ERRORTAG_ATTR_LIST(ag_resv_fail),
 	XFS_ERRORTAG_ATTR_LIST(larp),
+	XFS_ERRORTAG_ATTR_LIST(leaf_split),
 	NULL,
 };
 
-- 
2.25.1

[RFC PATCH 2/2] xfs: add leaf to node error tag

[Catherine Hoang]

Add an error tag on xfs_attr3_leaf_to_node to test log attribute
recovery and replay.

Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com>
---
 fs/xfs/libxfs/xfs_attr_leaf.c | 6 ++++++
 fs/xfs/libxfs/xfs_errortag.h  | 4 +++-
 fs/xfs/xfs_error.c            | 3 +++
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
index 74b76b09509f..fdeb09de74ca 100644
--- a/fs/xfs/libxfs/xfs_attr_leaf.c
+++ b/fs/xfs/libxfs/xfs_attr_leaf.c
@@ -28,6 +28,7 @@
 #include "xfs_dir2.h"
 #include "xfs_log.h"
 #include "xfs_ag.h"
+#include "xfs_errortag.h"
 
 
 /*
@@ -1189,6 +1190,11 @@ xfs_attr3_leaf_to_node(
 
 	trace_xfs_attr_leaf_to_node(args);
 
+	if (XFS_TEST_ERROR(false, mp, XFS_ERRTAG_LEAF_TO_NODE)) {
+		error = -EIO;
+		goto out;
+	}
+
 	error = xfs_da_grow_inode(args, &blkno);
 	if (error)
 		goto out;
diff --git a/fs/xfs/libxfs/xfs_errortag.h b/fs/xfs/libxfs/xfs_errortag.h
index 31aeeb94dd5b..cc1650b58723 100644
--- a/fs/xfs/libxfs/xfs_errortag.h
+++ b/fs/xfs/libxfs/xfs_errortag.h
@@ -61,7 +61,8 @@
 #define XFS_ERRTAG_AG_RESV_FAIL				38
 #define XFS_ERRTAG_LARP					39
 #define XFS_ERRTAG_LEAF_SPLIT				40
-#define XFS_ERRTAG_MAX					41
+#define XFS_ERRTAG_LEAF_TO_NODE				41
+#define XFS_ERRTAG_MAX					42
 
 /*
  * Random factors for above tags, 1 means always, 2 means 1/2 time, etc.
@@ -107,5 +108,6 @@
 #define XFS_RANDOM_AG_RESV_FAIL				1
 #define XFS_RANDOM_LARP					1
 #define XFS_RANDOM_LEAF_SPLIT				1
+#define XFS_RANDOM_LEAF_TO_NODE				1
 
 #endif /* __XFS_ERRORTAG_H_ */
diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
index 732cb66236c1..7f2a71218a82 100644
--- a/fs/xfs/xfs_error.c
+++ b/fs/xfs/xfs_error.c
@@ -59,6 +59,7 @@ static unsigned int xfs_errortag_random_default[] = {
 	XFS_RANDOM_AG_RESV_FAIL,
 	XFS_RANDOM_LARP,
 	XFS_RANDOM_LEAF_SPLIT,
+	XFS_RANDOM_LEAF_TO_NODE,
 };
 
 struct xfs_errortag_attr {
@@ -174,6 +175,7 @@ XFS_ERRORTAG_ATTR_RW(bmap_alloc_minlen_extent,	XFS_ERRTAG_BMAP_ALLOC_MINLEN_EXTE
 XFS_ERRORTAG_ATTR_RW(ag_resv_fail, XFS_ERRTAG_AG_RESV_FAIL);
 XFS_ERRORTAG_ATTR_RW(larp,		XFS_ERRTAG_LARP);
 XFS_ERRORTAG_ATTR_RW(leaf_split,	XFS_ERRTAG_LEAF_SPLIT);
+XFS_ERRORTAG_ATTR_RW(leaf_to_node,	XFS_ERRTAG_LEAF_TO_NODE);
 
 static struct attribute *xfs_errortag_attrs[] = {
 	XFS_ERRORTAG_ATTR_LIST(noerror),
@@ -217,6 +219,7 @@ static struct attribute *xfs_errortag_attrs[] = {
 	XFS_ERRORTAG_ATTR_LIST(ag_resv_fail),
 	XFS_ERRORTAG_ATTR_LIST(larp),
 	XFS_ERRORTAG_ATTR_LIST(leaf_split),
+	XFS_ERRORTAG_ATTR_LIST(leaf_to_node),
 	NULL,
 };
 
-- 
2.25.1

Re: [RFC PATCH 1/2] xfs: add leaf split error tag

[Darrick J. Wong]

On Thu, Nov 11, 2021 at 12:17:15AM +0000, Catherine Hoang wrote:
> Add an error tag on xfs_da3_split to test log attribute recovery
> and replay.
> 
> Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com>
> ---
>  fs/xfs/libxfs/xfs_da_btree.c | 6 ++++++
>  fs/xfs/libxfs/xfs_errortag.h | 4 +++-
>  fs/xfs/xfs_error.c           | 3 +++
>  3 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
> index dd7a2dbce1d1..000101783648 100644
> --- a/fs/xfs/libxfs/xfs_da_btree.c
> +++ b/fs/xfs/libxfs/xfs_da_btree.c
> @@ -22,6 +22,7 @@
>  #include "xfs_trace.h"
>  #include "xfs_buf_item.h"
>  #include "xfs_log.h"
> +#include "xfs_errortag.h"
>  
>  /*
>   * xfs_da_btree.c
> @@ -482,6 +483,11 @@ xfs_da3_split(
>  
>  	trace_xfs_da_split(state->args);
>  
> +	if (XFS_TEST_ERROR(false, state->mp, XFS_ERRTAG_LEAF_SPLIT)) {
> +		error = -EIO;
> +		return error;

Nit:

if (XFS_TEST_ERROR(...))
	return -EIO;

--D

> +	}
> +
>  	/*
>  	 * Walk back up the tree splitting/inserting/adjusting as necessary.
>  	 * If we need to insert and there isn't room, split the node, then
> diff --git a/fs/xfs/libxfs/xfs_errortag.h b/fs/xfs/libxfs/xfs_errortag.h
> index c15d2340220c..31aeeb94dd5b 100644
> --- a/fs/xfs/libxfs/xfs_errortag.h
> +++ b/fs/xfs/libxfs/xfs_errortag.h
> @@ -60,7 +60,8 @@
>  #define XFS_ERRTAG_BMAP_ALLOC_MINLEN_EXTENT		37
>  #define XFS_ERRTAG_AG_RESV_FAIL				38
>  #define XFS_ERRTAG_LARP					39
> -#define XFS_ERRTAG_MAX					40
> +#define XFS_ERRTAG_LEAF_SPLIT				40
> +#define XFS_ERRTAG_MAX					41
>  
>  /*
>   * Random factors for above tags, 1 means always, 2 means 1/2 time, etc.
> @@ -105,5 +106,6 @@
>  #define XFS_RANDOM_BMAP_ALLOC_MINLEN_EXTENT		1
>  #define XFS_RANDOM_AG_RESV_FAIL				1
>  #define XFS_RANDOM_LARP					1
> +#define XFS_RANDOM_LEAF_SPLIT				1
>  
>  #endif /* __XFS_ERRORTAG_H_ */
> diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
> index d4b2256ba00b..732cb66236c1 100644
> --- a/fs/xfs/xfs_error.c
> +++ b/fs/xfs/xfs_error.c
> @@ -58,6 +58,7 @@ static unsigned int xfs_errortag_random_default[] = {
>  	XFS_RANDOM_BMAP_ALLOC_MINLEN_EXTENT,
>  	XFS_RANDOM_AG_RESV_FAIL,
>  	XFS_RANDOM_LARP,
> +	XFS_RANDOM_LEAF_SPLIT,
>  };
>  
>  struct xfs_errortag_attr {
> @@ -172,6 +173,7 @@ XFS_ERRORTAG_ATTR_RW(reduce_max_iextents,	XFS_ERRTAG_REDUCE_MAX_IEXTENTS);
>  XFS_ERRORTAG_ATTR_RW(bmap_alloc_minlen_extent,	XFS_ERRTAG_BMAP_ALLOC_MINLEN_EXTENT);
>  XFS_ERRORTAG_ATTR_RW(ag_resv_fail, XFS_ERRTAG_AG_RESV_FAIL);
>  XFS_ERRORTAG_ATTR_RW(larp,		XFS_ERRTAG_LARP);
> +XFS_ERRORTAG_ATTR_RW(leaf_split,	XFS_ERRTAG_LEAF_SPLIT);
>  
>  static struct attribute *xfs_errortag_attrs[] = {
>  	XFS_ERRORTAG_ATTR_LIST(noerror),
> @@ -214,6 +216,7 @@ static struct attribute *xfs_errortag_attrs[] = {
>  	XFS_ERRORTAG_ATTR_LIST(bmap_alloc_minlen_extent),
>  	XFS_ERRORTAG_ATTR_LIST(ag_resv_fail),
>  	XFS_ERRORTAG_ATTR_LIST(larp),
> +	XFS_ERRORTAG_ATTR_LIST(leaf_split),
>  	NULL,
>  };
>  
> -- 
> 2.25.1
> 

Re: [RFC PATCH 2/2] xfs: add leaf to node error tag

[Darrick J. Wong]

On Thu, Nov 11, 2021 at 12:17:16AM +0000, Catherine Hoang wrote:
> Add an error tag on xfs_attr3_leaf_to_node to test log attribute
> recovery and replay.
> 
> Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com>
> ---
>  fs/xfs/libxfs/xfs_attr_leaf.c | 6 ++++++
>  fs/xfs/libxfs/xfs_errortag.h  | 4 +++-
>  fs/xfs/xfs_error.c            | 3 +++
>  3 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> index 74b76b09509f..fdeb09de74ca 100644
> --- a/fs/xfs/libxfs/xfs_attr_leaf.c
> +++ b/fs/xfs/libxfs/xfs_attr_leaf.c
> @@ -28,6 +28,7 @@
>  #include "xfs_dir2.h"
>  #include "xfs_log.h"
>  #include "xfs_ag.h"
> +#include "xfs_errortag.h"
>  
>  
>  /*
> @@ -1189,6 +1190,11 @@ xfs_attr3_leaf_to_node(
>  
>  	trace_xfs_attr_leaf_to_node(args);
>  
> +	if (XFS_TEST_ERROR(false, mp, XFS_ERRTAG_LEAF_TO_NODE)) {

Hmmm... any plans to do a similar thing for directories too?

(FWIW the rest of the errortag injecting stuff looks ok...)

--D

> +		error = -EIO;
> +		goto out;
> +	}
> +
>  	error = xfs_da_grow_inode(args, &blkno);
>  	if (error)
>  		goto out;
> diff --git a/fs/xfs/libxfs/xfs_errortag.h b/fs/xfs/libxfs/xfs_errortag.h
> index 31aeeb94dd5b..cc1650b58723 100644
> --- a/fs/xfs/libxfs/xfs_errortag.h
> +++ b/fs/xfs/libxfs/xfs_errortag.h
> @@ -61,7 +61,8 @@
>  #define XFS_ERRTAG_AG_RESV_FAIL				38
>  #define XFS_ERRTAG_LARP					39
>  #define XFS_ERRTAG_LEAF_SPLIT				40
> -#define XFS_ERRTAG_MAX					41
> +#define XFS_ERRTAG_LEAF_TO_NODE				41
> +#define XFS_ERRTAG_MAX					42
>  
>  /*
>   * Random factors for above tags, 1 means always, 2 means 1/2 time, etc.
> @@ -107,5 +108,6 @@
>  #define XFS_RANDOM_AG_RESV_FAIL				1
>  #define XFS_RANDOM_LARP					1
>  #define XFS_RANDOM_LEAF_SPLIT				1
> +#define XFS_RANDOM_LEAF_TO_NODE				1
>  
>  #endif /* __XFS_ERRORTAG_H_ */
> diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
> index 732cb66236c1..7f2a71218a82 100644
> --- a/fs/xfs/xfs_error.c
> +++ b/fs/xfs/xfs_error.c
> @@ -59,6 +59,7 @@ static unsigned int xfs_errortag_random_default[] = {
>  	XFS_RANDOM_AG_RESV_FAIL,
>  	XFS_RANDOM_LARP,
>  	XFS_RANDOM_LEAF_SPLIT,
> +	XFS_RANDOM_LEAF_TO_NODE,
>  };
>  
>  struct xfs_errortag_attr {
> @@ -174,6 +175,7 @@ XFS_ERRORTAG_ATTR_RW(bmap_alloc_minlen_extent,	XFS_ERRTAG_BMAP_ALLOC_MINLEN_EXTE
>  XFS_ERRORTAG_ATTR_RW(ag_resv_fail, XFS_ERRTAG_AG_RESV_FAIL);
>  XFS_ERRORTAG_ATTR_RW(larp,		XFS_ERRTAG_LARP);
>  XFS_ERRORTAG_ATTR_RW(leaf_split,	XFS_ERRTAG_LEAF_SPLIT);
> +XFS_ERRORTAG_ATTR_RW(leaf_to_node,	XFS_ERRTAG_LEAF_TO_NODE);
>  
>  static struct attribute *xfs_errortag_attrs[] = {
>  	XFS_ERRORTAG_ATTR_LIST(noerror),
> @@ -217,6 +219,7 @@ static struct attribute *xfs_errortag_attrs[] = {
>  	XFS_ERRORTAG_ATTR_LIST(ag_resv_fail),
>  	XFS_ERRORTAG_ATTR_LIST(larp),
>  	XFS_ERRORTAG_ATTR_LIST(leaf_split),
> +	XFS_ERRORTAG_ATTR_LIST(leaf_to_node),
>  	NULL,
>  };
>  
> -- 
> 2.25.1
> 

Re: [RFC PATCH 1/2] xfs: add leaf split error tag

[Dave Chinner]

On Thu, Nov 11, 2021 at 12:17:15AM +0000, Catherine Hoang wrote:
> Add an error tag on xfs_da3_split to test log attribute recovery
> and replay.
> 
> Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com>
> ---
>  fs/xfs/libxfs/xfs_da_btree.c | 6 ++++++
>  fs/xfs/libxfs/xfs_errortag.h | 4 +++-
>  fs/xfs/xfs_error.c           | 3 +++
>  3 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
> index dd7a2dbce1d1..000101783648 100644
> --- a/fs/xfs/libxfs/xfs_da_btree.c
> +++ b/fs/xfs/libxfs/xfs_da_btree.c
> @@ -22,6 +22,7 @@
>  #include "xfs_trace.h"
>  #include "xfs_buf_item.h"
>  #include "xfs_log.h"
> +#include "xfs_errortag.h"
>  
>  /*
>   * xfs_da_btree.c
> @@ -482,6 +483,11 @@ xfs_da3_split(
>  
>  	trace_xfs_da_split(state->args);
>  
> +	if (XFS_TEST_ERROR(false, state->mp, XFS_ERRTAG_LEAF_SPLIT)) {
> +		error = -EIO;
> +		return error;
> +	}
> +
>  	/*
>  	 * Walk back up the tree splitting/inserting/adjusting as necessary.
>  	 * If we need to insert and there isn't room, split the node, then
> diff --git a/fs/xfs/libxfs/xfs_errortag.h b/fs/xfs/libxfs/xfs_errortag.h
> index c15d2340220c..31aeeb94dd5b 100644
> --- a/fs/xfs/libxfs/xfs_errortag.h
> +++ b/fs/xfs/libxfs/xfs_errortag.h
> @@ -60,7 +60,8 @@
>  #define XFS_ERRTAG_BMAP_ALLOC_MINLEN_EXTENT		37
>  #define XFS_ERRTAG_AG_RESV_FAIL				38
>  #define XFS_ERRTAG_LARP					39
> -#define XFS_ERRTAG_MAX					40
> +#define XFS_ERRTAG_LEAF_SPLIT				40

What leaf is being split?

This looks to be a DA btree split that it the error injection is
being applied to, not a allocbt, rmapbt, etc split. And it's not
really a "leaf split" because xfs_da3_split() walks the entire path
back up the tree splitting nodes as well.

So, really, it's a da tree split, not a generic "leaf split" error
injection point.

And, I suspect this won't always work as intended, because it can
trigger on directory operations as well as attribute ops. Hence it
could be difficult to direct this for testing attr fork operations
during stress at the attr fork....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [RFC PATCH 2/2] xfs: add leaf to node error tag

[Dave Chinner]

On Thu, Nov 11, 2021 at 12:17:16AM +0000, Catherine Hoang wrote:
> Add an error tag on xfs_attr3_leaf_to_node to test log attribute
> recovery and replay.
> 
> Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com>
> ---
>  fs/xfs/libxfs/xfs_attr_leaf.c | 6 ++++++
>  fs/xfs/libxfs/xfs_errortag.h  | 4 +++-
>  fs/xfs/xfs_error.c            | 3 +++
>  3 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
> index 74b76b09509f..fdeb09de74ca 100644
> --- a/fs/xfs/libxfs/xfs_attr_leaf.c
> +++ b/fs/xfs/libxfs/xfs_attr_leaf.c
> @@ -28,6 +28,7 @@
>  #include "xfs_dir2.h"
>  #include "xfs_log.h"
>  #include "xfs_ag.h"
> +#include "xfs_errortag.h"
>  
>  
>  /*
> @@ -1189,6 +1190,11 @@ xfs_attr3_leaf_to_node(
>  
>  	trace_xfs_attr_leaf_to_node(args);
>  
> +	if (XFS_TEST_ERROR(false, mp, XFS_ERRTAG_LEAF_TO_NODE)) {
> +		error = -EIO;
> +		goto out;
> +	}
> +
>  	error = xfs_da_grow_inode(args, &blkno);
>  	if (error)
>  		goto out;
> diff --git a/fs/xfs/libxfs/xfs_errortag.h b/fs/xfs/libxfs/xfs_errortag.h
> index 31aeeb94dd5b..cc1650b58723 100644
> --- a/fs/xfs/libxfs/xfs_errortag.h
> +++ b/fs/xfs/libxfs/xfs_errortag.h
> @@ -61,7 +61,8 @@
>  #define XFS_ERRTAG_AG_RESV_FAIL				38
>  #define XFS_ERRTAG_LARP					39
>  #define XFS_ERRTAG_LEAF_SPLIT				40
> -#define XFS_ERRTAG_MAX					41
> +#define XFS_ERRTAG_LEAF_TO_NODE				41
> +#define XFS_ERRTAG_MAX					42

Same again about naming. THis is an attribute fork injection point,
not a generic "leaf-to-node" error injection point.

Whihc makes me wonder: this is testing just the initial leaf split
shape change. There are other shape changes - inline -> leaf, leaf
-> multi-level tree (xfs_da3_split()) and multi-level -> single
level leaf, leaf -> inline - for the attr tree, and there are even
more shape changes for the directory tree (inline, leaf, leafN,
node)

As a general infrastructure question, are we really going to now add
separate error injections for each different shape change on each
different type of btree? That explodes the number of injection
points quite quickly....

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

Re: [RFC PATCH 1/2] xfs: add leaf split error tag

[Allison Henderson]

On 11/11/21 4:17 PM, Dave Chinner wrote:
> On Thu, Nov 11, 2021 at 12:17:15AM +0000, Catherine Hoang wrote:
>> Add an error tag on xfs_da3_split to test log attribute recovery
>> and replay.
>>
>> Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com>
>> ---
>>   fs/xfs/libxfs/xfs_da_btree.c | 6 ++++++
>>   fs/xfs/libxfs/xfs_errortag.h | 4 +++-
>>   fs/xfs/xfs_error.c           | 3 +++
>>   3 files changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
>> index dd7a2dbce1d1..000101783648 100644
>> --- a/fs/xfs/libxfs/xfs_da_btree.c
>> +++ b/fs/xfs/libxfs/xfs_da_btree.c
>> @@ -22,6 +22,7 @@
>>   #include "xfs_trace.h"
>>   #include "xfs_buf_item.h"
>>   #include "xfs_log.h"
>> +#include "xfs_errortag.h"
>>   
>>   /*
>>    * xfs_da_btree.c
>> @@ -482,6 +483,11 @@ xfs_da3_split(
>>   
>>   	trace_xfs_da_split(state->args);
>>   
>> +	if (XFS_TEST_ERROR(false, state->mp, XFS_ERRTAG_LEAF_SPLIT)) {
>> +		error = -EIO;
>> +		return error;
>> +	}
>> +
>>   	/*
>>   	 * Walk back up the tree splitting/inserting/adjusting as necessary.
>>   	 * If we need to insert and there isn't room, split the node, then
>> diff --git a/fs/xfs/libxfs/xfs_errortag.h b/fs/xfs/libxfs/xfs_errortag.h
>> index c15d2340220c..31aeeb94dd5b 100644
>> --- a/fs/xfs/libxfs/xfs_errortag.h
>> +++ b/fs/xfs/libxfs/xfs_errortag.h
>> @@ -60,7 +60,8 @@
>>   #define XFS_ERRTAG_BMAP_ALLOC_MINLEN_EXTENT		37
>>   #define XFS_ERRTAG_AG_RESV_FAIL				38
>>   #define XFS_ERRTAG_LARP					39
>> -#define XFS_ERRTAG_MAX					40
>> +#define XFS_ERRTAG_LEAF_SPLIT				40
> 
> What leaf is being split?
> 
> This looks to be a DA btree split that it the error injection is
> being applied to, not a allocbt, rmapbt, etc split. And it's not
> really a "leaf split" because xfs_da3_split() walks the entire path
> back up the tree splitting nodes as well.
> 
> So, really, it's a da tree split, not a generic "leaf split" error
> injection point.
> 
> And, I suspect this won't always work as intended, because it can
> trigger on directory operations as well as attribute ops. Hence it
> could be difficult to direct this for testing attr fork operations
> during stress at the attr fork....

Well, the intent isnt to use these during a stress test.  The idea it to 
set the tag right before an attribute operation, and then see if the 
attribute is played back as it should.  Having the different points of 
failure demonstrates that the replay is successful even when the failure 
occurs during the different attribute forms.

That context of being in an attr operation is set up by the testcase. 
We create the dirs, and files first and then set the error tag before 
the attr operation.  So these error tags wouldn't otherwise trigger on a 
dir operation unless for some reason we wrote another test case that did 
that.  But that doesn't seem like it would be a very meaningful 
testcase?  Or it would have a different objective that it was trying to 
accomplish say the least.

Perhaps all we need here is a name scheme to make the intention clear?
How do people feel about this naming scheme?

XFS_ERRTAG_LARP			# Test log attr replay
XFS_ERRTAG_LARP_LEAF_SPLIT	# Test log attr replay on leaf split
XFS_ERRTAG_LARP_LEAF_TO_NODE	# Test log attr replay on leaf to node

That would also help to make clear that these are meant to trigger on 
the attribute code path for a replay. Let me know what people think. 
Thanks!


Allison

> 
> Cheers,
> 
> Dave.
> 

Re: [RFC PATCH 2/2] xfs: add leaf to node error tag

[Allison Henderson]

On 11/11/21 4:24 PM, Dave Chinner wrote:
> On Thu, Nov 11, 2021 at 12:17:16AM +0000, Catherine Hoang wrote:
>> Add an error tag on xfs_attr3_leaf_to_node to test log attribute
>> recovery and replay.
>>
>> Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com>
>> ---
>>   fs/xfs/libxfs/xfs_attr_leaf.c | 6 ++++++
>>   fs/xfs/libxfs/xfs_errortag.h  | 4 +++-
>>   fs/xfs/xfs_error.c            | 3 +++
>>   3 files changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/xfs/libxfs/xfs_attr_leaf.c b/fs/xfs/libxfs/xfs_attr_leaf.c
>> index 74b76b09509f..fdeb09de74ca 100644
>> --- a/fs/xfs/libxfs/xfs_attr_leaf.c
>> +++ b/fs/xfs/libxfs/xfs_attr_leaf.c
>> @@ -28,6 +28,7 @@
>>   #include "xfs_dir2.h"
>>   #include "xfs_log.h"
>>   #include "xfs_ag.h"
>> +#include "xfs_errortag.h"
>>   
>>   
>>   /*
>> @@ -1189,6 +1190,11 @@ xfs_attr3_leaf_to_node(
>>   
>>   	trace_xfs_attr_leaf_to_node(args);
>>   
>> +	if (XFS_TEST_ERROR(false, mp, XFS_ERRTAG_LEAF_TO_NODE)) {
>> +		error = -EIO;
>> +		goto out;
>> +	}
>> +
>>   	error = xfs_da_grow_inode(args, &blkno);
>>   	if (error)
>>   		goto out;
>> diff --git a/fs/xfs/libxfs/xfs_errortag.h b/fs/xfs/libxfs/xfs_errortag.h
>> index 31aeeb94dd5b..cc1650b58723 100644
>> --- a/fs/xfs/libxfs/xfs_errortag.h
>> +++ b/fs/xfs/libxfs/xfs_errortag.h
>> @@ -61,7 +61,8 @@
>>   #define XFS_ERRTAG_AG_RESV_FAIL				38
>>   #define c					39
>>   #define XFS_ERRTAG_LEAF_SPLIT				40
>> -#define XFS_ERRTAG_MAX					41
>> +#define XFS_ERRTAG_LEAF_TO_NODE				41
>> +#define XFS_ERRTAG_MAX					42
> 
> Same again about naming. THis is an attribute fork injection point,
> not a generic "leaf-to-node" error injection point.
> 
> Whihc makes me wonder: this is testing just the initial leaf split
> shape change. There are other shape changes - inline -> leaf, leaf
> -> multi-level tree (xfs_da3_split()) and multi-level -> single
> level leaf, leaf -> inline - for the attr tree, and there are even
> more shape changes for the directory tree (inline, leaf, leafN,
> node)
> 
> As a general infrastructure question, are we really going to now add
> separate error injections for each different shape change on each
> different type of btree? That explodes the number of injection
> points quite quickly....
Well, that certainly seem like a bit much.  But I don't think that just 
because we've added a few error tags that we necessarily have to put one 
on every transformation.  I think it's a fair compromise to simply 
identify a few points of interest, and if we later find a need to make 
the tests more rigorous we can expand the infrastructure we've set up here?

Allison

> 
> Cheers,
> 
> Dave.
>