| Message ID | 20220110212454.359752-2-catherine.hoang@oracle.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xfs: add error tags for log attribute replay test | expand |
On Mon, Jan 10, 2022 at 09:24:53PM +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> > Reviewed-by: Allison Henderson <allison.henderson@oracle.com> > --- > fs/xfs/libxfs/xfs_da_btree.c | 5 +++++ > fs/xfs/libxfs/xfs_errortag.h | 4 +++- > fs/xfs/xfs_error.c | 3 +++ > 3 files changed, 11 insertions(+), 1 deletion(-) > > diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c > index dd7a2dbce1d1..258a5fef64b2 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,10 @@ xfs_da3_split( > > trace_xfs_da_split(state->args); > > + if (XFS_TEST_ERROR(false, state->mp, XFS_ERRTAG_LARP_LEAF_SPLIT)) { This error injection knob is in the middle of the dabtree code, so it really ought to be namespaced _DA_ and not _LARP_: XFS_ERRTAG_DA_LEAF_SPLIT A bit of background: in XFS, directories and extended file attributes both start their lives as arrays of variable length records that map a name to some sort of binary data. Directory entries map a human- readable bytestring to an inode number, and xattrs map a namespaced human-readable bytestring to a blob of binary data. To speed up lookups by name, both structures support adding a btree index that maps the human-readable name to a hash value, then maps the hash value(s) to positions within the array. Within the xfs codebase, that btree is called the "dabtree" to distinguish it from xfs_btree, which is a totally different animal. Hence, any error injection knobs touching xfs_da* functions really should be namespaced _DA_ to match. (And for everyone else following along at home, "LARP" refers to Logging extended Attributes that are Replayable on Purpose or something like that.) > + return -EIO; > + } Nit: don't need braces for a single-line if body. Other than those two things, this looks good to me. --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..970f3a3f3750 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_LARP_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_LARP_LEAF_SPLIT 1 > > #endif /* __XFS_ERRORTAG_H_ */ > diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c > index d4b2256ba00b..9cb6743a5ae3 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_LARP_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(larp_leaf_split, XFS_ERRTAG_LARP_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(larp_leaf_split), > NULL, > }; > > -- > 2.25.1 >
> On Jan 18, 2022, at 8:47 PM, Darrick J. Wong <djwong@kernel.org> wrote: > > On Mon, Jan 10, 2022 at 09:24:53PM +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> >> Reviewed-by: Allison Henderson <allison.henderson@oracle.com> >> --- >> fs/xfs/libxfs/xfs_da_btree.c | 5 +++++ >> fs/xfs/libxfs/xfs_errortag.h | 4 +++- >> fs/xfs/xfs_error.c | 3 +++ >> 3 files changed, 11 insertions(+), 1 deletion(-) >> >> diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c >> index dd7a2dbce1d1..258a5fef64b2 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,10 @@ xfs_da3_split( >> >> trace_xfs_da_split(state->args); >> >> + if (XFS_TEST_ERROR(false, state->mp, XFS_ERRTAG_LARP_LEAF_SPLIT)) { > > This error injection knob is in the middle of the dabtree code, so it > really ought to be namespaced _DA_ and not _LARP_: > > XFS_ERRTAG_DA_LEAF_SPLIT Ok, this makes sense. Will fix, thanks for the explanation! > > A bit of background: in XFS, directories and extended file attributes > both start their lives as arrays of variable length records that map a > name to some sort of binary data. Directory entries map a human- > readable bytestring to an inode number, and xattrs map a namespaced > human-readable bytestring to a blob of binary data. > > To speed up lookups by name, both structures support adding a btree > index that maps the human-readable name to a hash value, then maps the > hash value(s) to positions within the array. Within the xfs codebase, > that btree is called the "dabtree" to distinguish it from xfs_btree, > which is a totally different animal. > > Hence, any error injection knobs touching xfs_da* functions really > should be namespaced _DA_ to match. > > (And for everyone else following along at home, "LARP" refers to Logging > extended Attributes that are Replayable on Purpose or something like > that.) > >> + return -EIO; >> + } > > Nit: don't need braces for a single-line if body. > > Other than those two things, this looks good to me. > > --D Sure, I’ll remove the braces here. > >> + >> /* >> * 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..970f3a3f3750 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_LARP_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_LARP_LEAF_SPLIT 1 >> >> #endif /* __XFS_ERRORTAG_H_ */ >> diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c >> index d4b2256ba00b..9cb6743a5ae3 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_LARP_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(larp_leaf_split, XFS_ERRTAG_LARP_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(larp_leaf_split), >> NULL, >> }; >> >> -- >> 2.25.1 >>
diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c index dd7a2dbce1d1..258a5fef64b2 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,10 @@ xfs_da3_split( trace_xfs_da_split(state->args); + if (XFS_TEST_ERROR(false, state->mp, XFS_ERRTAG_LARP_LEAF_SPLIT)) { + return -EIO; + } + /* * 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..970f3a3f3750 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_LARP_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_LARP_LEAF_SPLIT 1 #endif /* __XFS_ERRORTAG_H_ */ diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c index d4b2256ba00b..9cb6743a5ae3 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_LARP_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(larp_leaf_split, XFS_ERRTAG_LARP_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(larp_leaf_split), NULL, };