On Tue, Feb 23, 2021 at 04:47:48PM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@xxxxxxxxxx>
>
> On debug kernels, we call xfs_dir3_leaf_check_int() multiple times
> on every directory modification. The robust hash ordering checks it
> does on every entry in the leaf on every call results in a massive
> CPU overhead which slows down debug kernels by a large amount.
>
> We use xfs_dir3_leaf_check_int() for the verifiers as well, so we
> can't just gut the function to reduce overhead. What we can do,
> however, is reduce the work it does when it is called from the
> debug interfaces, just leaving the high level checks in place and
> leaving the robust validation to the verifiers. This means the debug
> checks will catch gross errors, but subtle bugs might not be caught
> until a verifier is run.
>
> It is easy enough to restore the existing debug behaviour if the
> developer needs it (just change a call parameter in the debug code),
> but overwise the overhead makes testing large directory block sizes
> on debug kernels very slow.
>
> Profile at an unlink rate of ~80k file/s on a 64k block size
> filesystem before the patch:
>
> 40.30% [kernel] [k] xfs_dir3_leaf_check_int
> 10.98% [kernel] [k] __xfs_dir3_data_check
> 8.10% [kernel] [k] xfs_verify_dir_ino
> 4.42% [kernel] [k] memcpy
> 2.22% [kernel] [k] xfs_dir2_data_get_ftype
> 1.52% [kernel] [k] do_raw_spin_lock
>
> Profile after, at an unlink rate of ~125k files/s (+50% improvement)
> has largely dropped the leaf verification debug overhead out of the
> profile.
>
> 16.53% [kernel] [k] __xfs_dir3_data_check
> 12.53% [kernel] [k] xfs_verify_dir_ino
> 7.97% [kernel] [k] memcpy
> 3.36% [kernel] [k] xfs_dir2_data_get_ftype
> 2.86% [kernel] [k] __pv_queued_spin_lock_slowpath
>
> Create shows a similar change in profile and a +25% improvement in
> performance.
>
> Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
Looks good,
Reviewed-by: Darrick J. Wong <djwong@xxxxxxxxxx>
--D
> ---
> fs/xfs/libxfs/xfs_dir2_leaf.c | 10 +++++++---
> fs/xfs/libxfs/xfs_dir2_node.c | 2 +-
> fs/xfs/libxfs/xfs_dir2_priv.h | 3 ++-
> 3 files changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_dir2_leaf.c b/fs/xfs/libxfs/xfs_dir2_leaf.c
> index 95d2a3f92d75..ccd8d0aa62b8 100644
> --- a/fs/xfs/libxfs/xfs_dir2_leaf.c
> +++ b/fs/xfs/libxfs/xfs_dir2_leaf.c
> @@ -113,7 +113,7 @@ xfs_dir3_leaf1_check(
> } else if (leafhdr.magic != XFS_DIR2_LEAF1_MAGIC)
> return __this_address;
>
> - return xfs_dir3_leaf_check_int(dp->i_mount, &leafhdr, leaf);
> + return xfs_dir3_leaf_check_int(dp->i_mount, &leafhdr, leaf, false);
> }
>
> static inline void
> @@ -139,7 +139,8 @@ xfs_failaddr_t
> xfs_dir3_leaf_check_int(
> struct xfs_mount *mp,
> struct xfs_dir3_icleaf_hdr *hdr,
> - struct xfs_dir2_leaf *leaf)
> + struct xfs_dir2_leaf *leaf,
> + bool expensive_checking)
> {
> struct xfs_da_geometry *geo = mp->m_dir_geo;
> xfs_dir2_leaf_tail_t *ltp;
> @@ -162,6 +163,9 @@ xfs_dir3_leaf_check_int(
> (char *)&hdr->ents[hdr->count] > (char *)xfs_dir2_leaf_bests_p(ltp))
> return __this_address;
>
> + if (!expensive_checking)
> + return NULL;
> +
> /* Check hash value order, count stale entries. */
> for (i = stale = 0; i < hdr->count; i++) {
> if (i + 1 < hdr->count) {
> @@ -195,7 +199,7 @@ xfs_dir3_leaf_verify(
> return fa;
>
> xfs_dir2_leaf_hdr_from_disk(mp, &leafhdr, bp->b_addr);
> - return xfs_dir3_leaf_check_int(mp, &leafhdr, bp->b_addr);
> + return xfs_dir3_leaf_check_int(mp, &leafhdr, bp->b_addr, true);
> }
>
> static void
> diff --git a/fs/xfs/libxfs/xfs_dir2_node.c b/fs/xfs/libxfs/xfs_dir2_node.c
> index 5d51265d29d6..80a64117b460 100644
> --- a/fs/xfs/libxfs/xfs_dir2_node.c
> +++ b/fs/xfs/libxfs/xfs_dir2_node.c
> @@ -73,7 +73,7 @@ xfs_dir3_leafn_check(
> } else if (leafhdr.magic != XFS_DIR2_LEAFN_MAGIC)
> return __this_address;
>
> - return xfs_dir3_leaf_check_int(dp->i_mount, &leafhdr, leaf);
> + return xfs_dir3_leaf_check_int(dp->i_mount, &leafhdr, leaf, false);
> }
>
> static inline void
> diff --git a/fs/xfs/libxfs/xfs_dir2_priv.h b/fs/xfs/libxfs/xfs_dir2_priv.h
> index 44c6a77cba05..94943ce49cab 100644
> --- a/fs/xfs/libxfs/xfs_dir2_priv.h
> +++ b/fs/xfs/libxfs/xfs_dir2_priv.h
> @@ -127,7 +127,8 @@ xfs_dir3_leaf_find_entry(struct xfs_dir3_icleaf_hdr *leafhdr,
> extern int xfs_dir2_node_to_leaf(struct xfs_da_state *state);
>
> extern xfs_failaddr_t xfs_dir3_leaf_check_int(struct xfs_mount *mp,
> - struct xfs_dir3_icleaf_hdr *hdr, struct xfs_dir2_leaf *leaf);
> + struct xfs_dir3_icleaf_hdr *hdr, struct xfs_dir2_leaf *leaf,
> + bool expensive_checks);
>
> /* xfs_dir2_node.c */
> void xfs_dir2_free_hdr_from_disk(struct xfs_mount *mp,
> --
> 2.28.0
>
