linux-xfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: allison.henderson@oracle.com
To: linux-xfs@vger.kernel.org
Subject: [PATCH v10 27/32] xfs: fix unit conversion error in xfs_log_calc_max_attrsetm_res
Date: Wed,  8 Mar 2023 15:37:49 -0700	[thread overview]
Message-ID: <20230308223754.1455051-28-allison.henderson@oracle.com> (raw)
In-Reply-To: <20230308223754.1455051-1-allison.henderson@oracle.com>

From: Allison Henderson <allison.henderson@oracle.com>

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@kernel.org>
Reviewed-by: Allison Henderson <allison.henderson@oracle.com>
Signed-off-by: Allison Henderson <allison.henderson@oracle.com>
---
 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


  parent reply	other threads:[~2023-03-08 22:38 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-08 22:37 [PATCH v10 00/32] Parent Pointers allison.henderson
2023-03-08 22:37 ` [PATCH v10 01/32] xfs: Add new name to attri/d allison.henderson
2023-03-08 22:37 ` [PATCH v10 02/32] xfs: Hold inode locks in xfs_ialloc allison.henderson
2023-03-08 22:37 ` [PATCH v10 03/32] xfs: Hold inode locks in xfs_trans_alloc_dir allison.henderson
2023-03-08 22:37 ` [PATCH v10 04/32] xfs: Hold inode locks in xfs_rename allison.henderson
2023-03-08 22:37 ` [PATCH v10 05/32] xfs: Expose init_xattrs in xfs_create_tmpfile allison.henderson
2023-03-08 22:37 ` [PATCH v10 06/32] xfs: Increase XFS_DEFER_OPS_NR_INODES to 5 allison.henderson
2023-03-08 22:37 ` [PATCH v10 07/32] xfs: Increase XFS_QM_TRANS_MAXDQS " allison.henderson
2023-03-08 22:37 ` [PATCH v10 08/32] xfs: get directory offset when adding directory name allison.henderson
2023-03-08 22:37 ` [PATCH v10 09/32] xfs: get directory offset when removing " allison.henderson
2023-03-08 22:37 ` [PATCH v10 10/32] xfs: get directory offset when replacing a " allison.henderson
2023-03-08 22:37 ` [PATCH v10 11/32] xfs: add parent pointer support to attribute code allison.henderson
2023-03-08 22:37 ` [PATCH v10 12/32] xfs: define parent pointer xattr format allison.henderson
2023-03-08 22:37 ` [PATCH v10 13/32] xfs: Add xfs_verify_pptr allison.henderson
2023-03-08 22:37 ` [PATCH v10 14/32] xfs: extend transaction reservations for parent attributes allison.henderson
2023-03-08 22:37 ` [PATCH v10 15/32] xfs: parent pointer attribute creation allison.henderson
2023-03-08 22:37 ` [PATCH v10 16/32] xfs: add parent attributes to link allison.henderson
2023-03-08 22:37 ` [PATCH v10 17/32] xfs: add parent attributes to symlink allison.henderson
2023-03-08 22:37 ` [PATCH v10 18/32] xfs: remove parent pointers in unlink allison.henderson
2023-03-08 22:37 ` [PATCH v10 19/32] xfs: Indent xfs_rename allison.henderson
2023-03-08 22:37 ` [PATCH v10 20/32] xfs: Add parent pointers to rename allison.henderson
2023-03-08 22:37 ` [PATCH v10 21/32] xfs: Add parent pointers to xfs_cross_rename allison.henderson
2023-03-08 22:37 ` [PATCH v10 22/32] xfs: Add the parent pointer support to the superblock version 5 allison.henderson
2023-03-08 22:37 ` [PATCH v10 23/32] xfs: Add helper function xfs_attr_list_context_init allison.henderson
2023-03-08 22:37 ` [PATCH v10 24/32] xfs: Filter XFS_ATTR_PARENT for getfattr allison.henderson
2023-03-08 22:37 ` [PATCH v10 25/32] xfs: pass the attr value to put_listent when possible allison.henderson
2023-03-08 22:37 ` [PATCH v10 26/32] xfs: Add parent pointer ioctl allison.henderson
2023-03-08 22:37 ` allison.henderson [this message]
2023-03-08 22:37 ` [PATCH v10 28/32] xfs: drop compatibility minimum log size computations for reflink allison.henderson
2023-03-08 22:37 ` [PATCH v10 29/32] xfs: add xfs_trans_mod_sb tracing allison.henderson
2023-03-08 22:37 ` [PATCH v10 30/32] xfs: move/add parent pointer validators to xfs_parent allison.henderson
2023-03-08 22:37 ` [PATCH v10 31/32] xfs: directory lookups should return diroffsets too allison.henderson
2023-03-08 22:37 ` [PATCH v10 32/32] xfs: don't remove the attr fork when parent pointers are enabled allison.henderson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230308223754.1455051-28-allison.henderson@oracle.com \
    --to=allison.henderson@oracle.com \
    --cc=linux-xfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).