On Wed, Sep 21, 2022 at 10:44:57PM -0700, allison.henderson@xxxxxxxxxx wrote:
> From: Allison Henderson <allison.henderson@xxxxxxxxxx>
Er, did you change this patch much?
I was really hoping you'd *RVB* tag it and send it back out. :)
--D
>
> Dave and I were discussing some recent test regressions as a result of
> me turning on nrext64=1 on realtime filesystems, when we noticed that
> the minimum log size of a 32M filesystem jumped from 954 blocks to 4287
> blocks.
>
> Digging through xfs_log_calc_max_attrsetm_res, Dave noticed that @size
> contains the maximum estimated amount of space needed for a local format
> xattr, in bytes, but we feed this quantity to XFS_NEXTENTADD_SPACE_RES,
> which requires units of blocks. This has resulted in an overestimation
> of the minimum log size over the years.
>
> We should nominally correct this, but there's a backwards compatibility
> problem -- if we enable it now, the minimum log size will decrease. If
> a corrected mkfs formats a filesystem with this new smaller log size, a
> user will encounter mount failures on an uncorrected kernel due to the
> larger minimum log size computations there.
>
> However, the large extent counters feature is still EXPERIMENTAL, so we
> can gate the correction on that feature (or any features that get added
> after that) being enabled. Any filesystem with nrext64 or any of the
> as-yet-undefined feature bits turned on will be rejected by old
> uncorrected kernels, so this should be safe even in the upgrade case.
>
> Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx>
> Signed-off-by: Allison Henderson <allison.henderson@xxxxxxxxxx>
> ---
> fs/xfs/libxfs/xfs_log_rlimit.c | 43 ++++++++++++++++++++++++++++++++++
> 1 file changed, 43 insertions(+)
>
> diff --git a/fs/xfs/libxfs/xfs_log_rlimit.c b/fs/xfs/libxfs/xfs_log_rlimit.c
> index 9975b93a7412..e5c606fb7a6a 100644
> --- a/fs/xfs/libxfs/xfs_log_rlimit.c
> +++ b/fs/xfs/libxfs/xfs_log_rlimit.c
> @@ -16,6 +16,39 @@
> #include "xfs_bmap_btree.h"
> #include "xfs_trace.h"
>
> +/*
> + * Decide if the filesystem has the parent pointer feature or any feature
> + * added after that.
> + */
> +static inline bool
> +xfs_has_parent_or_newer_feature(
> + struct xfs_mount *mp)
> +{
> + if (!xfs_sb_is_v5(&mp->m_sb))
> + return false;
> +
> + if (xfs_sb_has_compat_feature(&mp->m_sb, ~0))
> + return true;
> +
> + if (xfs_sb_has_ro_compat_feature(&mp->m_sb,
> + ~(XFS_SB_FEAT_RO_COMPAT_FINOBT |
> + XFS_SB_FEAT_RO_COMPAT_RMAPBT |
> + XFS_SB_FEAT_RO_COMPAT_REFLINK |
> + XFS_SB_FEAT_RO_COMPAT_INOBTCNT)))
> + return true;
> +
> + if (xfs_sb_has_incompat_feature(&mp->m_sb,
> + ~(XFS_SB_FEAT_INCOMPAT_FTYPE |
> + XFS_SB_FEAT_INCOMPAT_SPINODES |
> + XFS_SB_FEAT_INCOMPAT_META_UUID |
> + XFS_SB_FEAT_INCOMPAT_BIGTIME |
> + XFS_SB_FEAT_INCOMPAT_NEEDSREPAIR |
> + XFS_SB_FEAT_INCOMPAT_NREXT64)))
> + return true;
> +
> + return false;
> +}
> +
> /*
> * Calculate the maximum length in bytes that would be required for a local
> * attribute value as large attributes out of line are not logged.
> @@ -31,6 +64,16 @@ xfs_log_calc_max_attrsetm_res(
> MAXNAMELEN - 1;
> nblks = XFS_DAENTER_SPACE_RES(mp, XFS_ATTR_FORK);
> nblks += XFS_B_TO_FSB(mp, size);
> +
> + /*
> + * Starting with the parent pointer feature, every new fs feature
> + * corrects a unit conversion error in the xattr transaction
> + * reservation code that resulted in oversized minimum log size
> + * computations.
> + */
> + if (xfs_has_parent_or_newer_feature(mp))
> + size = XFS_B_TO_FSB(mp, size);
> +
> nblks += XFS_NEXTENTADD_SPACE_RES(mp, size, XFS_ATTR_FORK);
>
> return M_RES(mp)->tr_attrsetm.tr_logres +
> --
> 2.25.1
>
