Re: [PATCH V4 10/10] xfs: Introduce error injection to reduce maximum inode fork extent count

Darrick J. Wong
2020-09-18T15:39:38Z

On Fri, Sep 18, 2020 at 03:17:59PM +0530, Chandan Babu R wrote:
> This commit adds XFS_ERRTAG_REDUCE_MAX_IEXTENTS error tag which enables
> userspace programs to test "Inode fork extent count overflow detection"
> by reducing maximum possible inode fork extent count to
> 10 (i.e. MAXERRTAGEXTNUM).
>
> This commit makes the following additional changes to enable writing
> deterministic userspace tests for checking inode extent count overflow,
> 1. xfs_bmap_add_extent_hole_real()
> File & disk offsets at which extents are allocated by Directory,
> Xattr and Realtime code cannot be controlled explicitly from
> userspace. When XFS_ERRTAG_REDUCE_MAX_IEXTENTS error tag is enabled,
> xfs_bmap_add_extent_hole_real() prevents extents from being merged
> even though the new extent might be contiguous and have the same
> state as its neighbours.

That sounds like fs corruption to me, since btree records are supposed
to be maximally sized.

> 2. xfs_growfs_rt_alloc()
> This function allocates as large an extent as possible to fit in the
> additional bitmap/summary blocks. We now force allocation of block
> sized extents when XFS_ERRTAG_REDUCE_MAX_IEXTENTS error tag is
> enabled.

Ah, so your goal is to dramatically cut the MAX?EXTNUM and then force
the allocator to fragment the fs, so that it will quickly hit that
maximum.

/me suspects that "maximally fragment" ought to be a separate error
injector that teaches the alloctor to satisfy the minimum required
allocation, and to look only in the short end of the cntbt.

> Signed-off-by: Chandan Babu R <chandanrlinux@gmail.com>
> ---
> fs/xfs/libxfs/xfs_bmap.c | 9 +++++++--
> fs/xfs/libxfs/xfs_errortag.h | 4 +++-
> fs/xfs/libxfs/xfs_inode_fork.c | 4 ++++
> fs/xfs/libxfs/xfs_types.h | 1 +
> fs/xfs/xfs_error.c | 3 +++
> fs/xfs/xfs_rtalloc.c | 16 ++++++++++++++--
> 6 files changed, 32 insertions(+), 5 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 9c665e379dfc..287f0c4f6d33 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -2729,11 +2729,14 @@ xfs_bmap_add_extent_hole_real(
> int rval=0; /* return value (logging flags) */
> int state = xfs_bmap_fork_to_state(whichfork);
> struct xfs_bmbt_irec old;
> + int test_iext_overflow;
>
> ASSERT(!isnullstartblock(new->br_startblock));
> ASSERT(!cur || !(cur->bc_ino.flags & XFS_BTCUR_BMBT_WASDEL));
>
> XFS_STATS_INC(mp, xs_add_exlist);
> + test_iext_overflow = XFS_TEST_ERROR(false, ip->i_mount,
> + XFS_ERRTAG_REDUCE_MAX_IEXTENTS);
>
> /*
> * Check and set flags if this segment has a left neighbor.
> @@ -2762,7 +2765,8 @@ xfs_bmap_add_extent_hole_real(
> left.br_startoff + left.br_blockcount == new->br_startoff &&
> left.br_startblock + left.br_blockcount == new->br_startblock &&
> left.br_state == new->br_state &&
> - left.br_blockcount + new->br_blockcount <= MAXEXTLEN)
> + left.br_blockcount + new->br_blockcount <= MAXEXTLEN &&
> + !test_iext_overflow)
> state |= BMAP_LEFT_CONTIG;
>
> if ((state & BMAP_RIGHT_VALID) && !(state & BMAP_RIGHT_DELAY) &&
> @@ -2772,7 +2776,8 @@ xfs_bmap_add_extent_hole_real(
> new->br_blockcount + right.br_blockcount <= MAXEXTLEN &&
> (!(state & BMAP_LEFT_CONTIG) ||
> left.br_blockcount + new->br_blockcount +
> - right.br_blockcount <= MAXEXTLEN))
> + right.br_blockcount <= MAXEXTLEN) &&
> + !test_iext_overflow)
> state |= BMAP_RIGHT_CONTIG;
>
> error = 0;
> diff --git a/fs/xfs/libxfs/xfs_errortag.h b/fs/xfs/libxfs/xfs_errortag.h
> index 53b305dea381..1c56fcceeea6 100644
> --- a/fs/xfs/libxfs/xfs_errortag.h
> +++ b/fs/xfs/libxfs/xfs_errortag.h
> @@ -56,7 +56,8 @@
> #define XFS_ERRTAG_FORCE_SUMMARY_RECALC 33
> #define XFS_ERRTAG_IUNLINK_FALLBACK 34
> #define XFS_ERRTAG_BUF_IOERROR 35
> -#define XFS_ERRTAG_MAX 36
> +#define XFS_ERRTAG_REDUCE_MAX_IEXTENTS 36
> +#define XFS_ERRTAG_MAX 37
>
> /*
> * Random factors for above tags, 1 means always, 2 means 1/2 time, etc.
> @@ -97,5 +98,6 @@
> #define XFS_RANDOM_FORCE_SUMMARY_RECALC 1
> #define XFS_RANDOM_IUNLINK_FALLBACK (XFS_RANDOM_DEFAULT/10)
> #define XFS_RANDOM_BUF_IOERROR XFS_RANDOM_DEFAULT
> +#define XFS_RANDOM_REDUCE_MAX_IEXTENTS 1
>
> #endif /* __XFS_ERRORTAG_H_ */
> diff --git a/fs/xfs/libxfs/xfs_inode_fork.c b/fs/xfs/libxfs/xfs_inode_fork.c
> index 8d48716547e5..14389d10c597 100644
> --- a/fs/xfs/libxfs/xfs_inode_fork.c
> +++ b/fs/xfs/libxfs/xfs_inode_fork.c
> @@ -24,6 +24,7 @@
> #include "xfs_dir2_priv.h"
> #include "xfs_attr_leaf.h"
> #include "xfs_types.h"
> +#include "xfs_errortag.h"
>
> kmem_zone_t *xfs_ifork_zone;
>
> @@ -745,6 +746,9 @@ xfs_iext_count_may_overflow(
>
> max_exts = (whichfork == XFS_ATTR_FORK) ? MAXAEXTNUM : MAXEXTNUM;
>
> + if (XFS_TEST_ERROR(false, ip->i_mount, XFS_ERRTAG_REDUCE_MAX_IEXTENTS))
> + max_exts = MAXERRTAGEXTNUM;
> +
> nr_exts = ifp->if_nextents + nr_to_add;
> if (nr_exts < ifp->if_nextents || nr_exts > max_exts)
> return -EFBIG;
> diff --git a/fs/xfs/libxfs/xfs_types.h b/fs/xfs/libxfs/xfs_types.h
> index 397d94775440..f2d6736b72e0 100644
> --- a/fs/xfs/libxfs/xfs_types.h
> +++ b/fs/xfs/libxfs/xfs_types.h
> @@ -61,6 +61,7 @@ typedef void * xfs_failaddr_t;
> #define MAXEXTLEN ((xfs_extlen_t)0x001fffff) /* 21 bits */
> #define MAXEXTNUM ((xfs_extnum_t)0x7fffffff) /* signed int */
> #define MAXAEXTNUM ((xfs_aextnum_t)0x7fff) /* signed short */
> +#define MAXERRTAGEXTNUM ((xfs_extnum_t)0xa)

FWIW you could probably just hardcode this in _count_may_overflow.

--D

>
> /*
> * Minimum and maximum blocksize and sectorsize.
> diff --git a/fs/xfs/xfs_error.c b/fs/xfs/xfs_error.c
> index 7f6e20899473..3780b118cc47 100644
> --- a/fs/xfs/xfs_error.c
> +++ b/fs/xfs/xfs_error.c
> @@ -54,6 +54,7 @@ static unsigned int xfs_errortag_random_default[] = {
> XFS_RANDOM_FORCE_SUMMARY_RECALC,
> XFS_RANDOM_IUNLINK_FALLBACK,
> XFS_RANDOM_BUF_IOERROR,
> + XFS_RANDOM_REDUCE_MAX_IEXTENTS,
> };
>
> struct xfs_errortag_attr {
> @@ -164,6 +165,7 @@ XFS_ERRORTAG_ATTR_RW(force_repair, XFS_ERRTAG_FORCE_SCRUB_REPAIR);
> XFS_ERRORTAG_ATTR_RW(bad_summary, XFS_ERRTAG_FORCE_SUMMARY_RECALC);
> XFS_ERRORTAG_ATTR_RW(iunlink_fallback, XFS_ERRTAG_IUNLINK_FALLBACK);
> XFS_ERRORTAG_ATTR_RW(buf_ioerror, XFS_ERRTAG_BUF_IOERROR);
> +XFS_ERRORTAG_ATTR_RW(reduce_max_iextents, XFS_ERRTAG_REDUCE_MAX_IEXTENTS);
>
> static struct attribute *xfs_errortag_attrs[] = {
> XFS_ERRORTAG_ATTR_LIST(noerror),
> @@ -202,6 +204,7 @@ static struct attribute *xfs_errortag_attrs[] = {
> XFS_ERRORTAG_ATTR_LIST(bad_summary),
> XFS_ERRORTAG_ATTR_LIST(iunlink_fallback),
> XFS_ERRORTAG_ATTR_LIST(buf_ioerror),
> + XFS_ERRORTAG_ATTR_LIST(reduce_max_iextents),
> NULL,
> };
>
> diff --git a/fs/xfs/xfs_rtalloc.c b/fs/xfs/xfs_rtalloc.c
> index 3e841a75f272..29a519fc30fb 100644
> --- a/fs/xfs/xfs_rtalloc.c
> +++ b/fs/xfs/xfs_rtalloc.c
> @@ -18,6 +18,8 @@
> #include "xfs_trans_space.h"
> #include "xfs_icache.h"
> #include "xfs_rtalloc.h"
> +#include "xfs_error.h"
> +#include "xfs_errortag.h"
>
>
> /*
> @@ -780,17 +782,27 @@ xfs_growfs_rt_alloc(
> int resblks; /* space reservation */
> enum xfs_blft buf_type;
> struct xfs_trans *tp;
> + xfs_extlen_t nr_blks_alloc;
> + int test_iext_overflow;
>
> if (ip == mp->m_rsumip)
> buf_type = XFS_BLFT_RTSUMMARY_BUF;
> else
> buf_type = XFS_BLFT_RTBITMAP_BUF;
>
> + test_iext_overflow = XFS_TEST_ERROR(false, ip->i_mount,
> + XFS_ERRTAG_REDUCE_MAX_IEXTENTS);
> +
> /*
> * Allocate space to the file, as necessary.
> */
> while (oblocks < nblocks) {
> - resblks = XFS_GROWFSRT_SPACE_RES(mp, nblocks - oblocks);
> + if (likely(!test_iext_overflow))
> + nr_blks_alloc = nblocks - oblocks;
> + else
> + nr_blks_alloc = 1;
> +
> + resblks = XFS_GROWFSRT_SPACE_RES(mp, nr_blks_alloc);
> /*
> * Reserve space & log for one extent added to the file.
> */
> @@ -813,7 +825,7 @@ xfs_growfs_rt_alloc(
> * Allocate blocks to the bitmap file.
> */
> nmap = 1;
> - error = xfs_bmapi_write(tp, ip, oblocks, nblocks - oblocks,
> + error = xfs_bmapi_write(tp, ip, oblocks, nr_blks_alloc,
> XFS_BMAPI_METADATA, 0, &map, &nmap);
> if (!error && nmap < 1)
> error = -ENOSPC;
> --
> 2.28.0
>