[PATCH 6.1 0/6] backport xfs fix patches reported by xfs/179/270/557/606

[PATCH 6.1 0/6] backport xfs fix patches reported by xfs/179/270/557/606

[Mahmoud Adam]

Hi,

 These patches fix and reported by xfstests tests xfs/179 xfs/270
xfs/557 xfs/606, the patchset were tested to confirm they fix those
tests. all are clean picks.

thanks,
MNAdam

[PATCH 6.1 1/6] xfs: allow inode inactivation during a ro mount log recovery

[Mahmoud Adam]

From: "Darrick J. Wong" <djwong@kernel.org>

commit 76e589013fec672c3587d6314f2d1f0aeddc26d9 upstream.

In the next patch, we're going to prohibit log recovery if the primary
superblock contains an unrecognized rocompat feature bit even on
readonly mounts.  This requires removing all the code in the log
mounting process that temporarily disables the readonly state.

Unfortunately, inode inactivation disables itself on readonly mounts.
Clearing the iunlinked lists after log recovery needs inactivation to
run to free the unreferenced inodes, which (AFAICT) is the only reason
why log mounting plays games with the readonly state in the first place.

Therefore, change the inactivation predicates to allow inactivation
during log recovery of a readonly mount.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Mahmoud Adam <mngyadam@amazon.com>
---
 fs/xfs/xfs_inode.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
index aa303be11576..0468243bcee6 100644
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -1652,8 +1652,11 @@ xfs_inode_needs_inactive(
 	if (VFS_I(ip)->i_mode == 0)
 		return false;

-	/* If this is a read-only mount, don't do this (would generate I/O) */
-	if (xfs_is_readonly(mp))
+	/*
+	 * If this is a read-only mount, don't do this (would generate I/O)
+	 * unless we're in log recovery and cleaning the iunlinked list.
+	 */
+	if (xfs_is_readonly(mp) && !xlog_recovery_needed(mp->m_log))
 		return false;

 	/* If the log isn't running, push inodes straight to reclaim. */
@@ -1713,8 +1716,11 @@ xfs_inactive(
 	mp = ip->i_mount;
 	ASSERT(!xfs_iflags_test(ip, XFS_IRECOVERY));

-	/* If this is a read-only mount, don't do this (would generate I/O) */
-	if (xfs_is_readonly(mp))
+	/*
+	 * If this is a read-only mount, don't do this (would generate I/O)
+	 * unless we're in log recovery and cleaning the iunlinked list.
+	 */
+	if (xfs_is_readonly(mp) && !xlog_recovery_needed(mp->m_log))
 		goto out;

 	/* Metadata inodes require explicit resource cleanup. */
--
2.40.1

[PATCH 6.1 2/6] xfs: fix log recovery when unknown rocompat bits are set

[Mahmoud Adam]

From: "Darrick J. Wong" <djwong@kernel.org>

commit 74ad4693b6473950e971b3dc525b5ee7570e05d0 upstream.

Log recovery has always run on read only mounts, even where the primary
superblock advertises unknown rocompat bits.  Due to a misunderstanding
between Eric and Darrick back in 2018, we accidentally changed the
superblock write verifier to shutdown the fs over that exact scenario.
As a result, the log cleaning that occurs at the end of the mounting
process fails if there are unknown rocompat bits set.

As we now allow writing of the superblock if there are unknown rocompat
bits set on a RO mount, we no longer want to turn off RO state to allow
log recovery to succeed on a RO mount.  Hence we also remove all the
(now unnecessary) RO state toggling from the log recovery path.

Fixes: 9e037cb7972f ("xfs: check for unknown v5 feature bits in superblock write verifier"
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Mahmoud Adam <mngyadam@amazon.com>
---
 fs/xfs/libxfs/xfs_sb.c |  3 ++-
 fs/xfs/xfs_log.c       | 17 -----------------
 2 files changed, 2 insertions(+), 18 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_sb.c b/fs/xfs/libxfs/xfs_sb.c
index b6a584e044be..7fae732486a9 100644
--- a/fs/xfs/libxfs/xfs_sb.c
+++ b/fs/xfs/libxfs/xfs_sb.c
@@ -266,7 +266,8 @@ xfs_validate_sb_write(
 		return -EFSCORRUPTED;
 	}

-	if (xfs_sb_has_ro_compat_feature(sbp, XFS_SB_FEAT_RO_COMPAT_UNKNOWN)) {
+	if (!xfs_is_readonly(mp) &&
+	    xfs_sb_has_ro_compat_feature(sbp, XFS_SB_FEAT_RO_COMPAT_UNKNOWN)) {
 		xfs_alert(mp,
 "Corruption detected in superblock read-only compatible features (0x%x)!",
 			(sbp->sb_features_ro_compat &
diff --git a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c
index f02a0dd522b3..83381d38efbe 100644
--- a/fs/xfs/xfs_log.c
+++ b/fs/xfs/xfs_log.c
@@ -730,15 +730,7 @@ xfs_log_mount(
 	 * just worked.
 	 */
 	if (!xfs_has_norecovery(mp)) {
-		/*
-		 * log recovery ignores readonly state and so we need to clear
-		 * mount-based read only state so it can write to disk.
-		 */
-		bool	readonly = test_and_clear_bit(XFS_OPSTATE_READONLY,
-						&mp->m_opstate);
 		error = xlog_recover(log);
-		if (readonly)
-			set_bit(XFS_OPSTATE_READONLY, &mp->m_opstate);
 		if (error) {
 			xfs_warn(mp, "log mount/recovery failed: error %d",
 				error);
@@ -787,7 +779,6 @@ xfs_log_mount_finish(
 	struct xfs_mount	*mp)
 {
 	struct xlog		*log = mp->m_log;
-	bool			readonly;
 	int			error = 0;

 	if (xfs_has_norecovery(mp)) {
@@ -795,12 +786,6 @@ xfs_log_mount_finish(
 		return 0;
 	}

-	/*
-	 * log recovery ignores readonly state and so we need to clear
-	 * mount-based read only state so it can write to disk.
-	 */
-	readonly = test_and_clear_bit(XFS_OPSTATE_READONLY, &mp->m_opstate);
-
 	/*
 	 * During the second phase of log recovery, we need iget and
 	 * iput to behave like they do for an active filesystem.
@@ -850,8 +835,6 @@ xfs_log_mount_finish(
 	xfs_buftarg_drain(mp->m_ddev_targp);

 	clear_bit(XLOG_RECOVERY_NEEDED, &log->l_opstate);
-	if (readonly)
-		set_bit(XFS_OPSTATE_READONLY, &mp->m_opstate);

 	/* Make sure the log is dead if we're returning failure. */
 	ASSERT(!error || xlog_is_shutdown(log));
--
2.40.1

[PATCH 6.1 3/6] xfs: get root inode correctly at bulkstat

[Mahmoud Adam]

From: Hironori Shiina <shiina.hironori@gmail.com>

commit 817644fa4525258992f17fecf4f1d6cdd2e1b731 upstream.

The root inode number should be set to `breq->startino` for getting stat
information of the root when XFS_BULK_IREQ_SPECIAL_ROOT is used.
Otherwise, the inode search is started from 1
(XFS_BULK_IREQ_SPECIAL_ROOT) and the inode with the lowest number in a
filesystem is returned.

Fixes: bf3cb3944792 ("xfs: allow single bulkstat of special inodes")
Signed-off-by: Hironori Shiina <shiina.hironori@fujitsu.com>
Reviewed-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Signed-off-by: Mahmoud Adam <mngyadam@amazon.com>
---
 fs/xfs/xfs_ioctl.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/xfs_ioctl.c b/fs/xfs/xfs_ioctl.c
index 1f783e979629..85fbb3b71d1c 100644
--- a/fs/xfs/xfs_ioctl.c
+++ b/fs/xfs/xfs_ioctl.c
@@ -754,7 +754,7 @@ xfs_bulkstat_fmt(
 static int
 xfs_bulk_ireq_setup(
 	struct xfs_mount	*mp,
-	struct xfs_bulk_ireq	*hdr,
+	const struct xfs_bulk_ireq *hdr,
 	struct xfs_ibulk	*breq,
 	void __user		*ubuffer)
 {
@@ -780,7 +780,7 @@ xfs_bulk_ireq_setup(

 		switch (hdr->ino) {
 		case XFS_BULK_IREQ_SPECIAL_ROOT:
-			hdr->ino = mp->m_sb.sb_rootino;
+			breq->startino = mp->m_sb.sb_rootino;
 			break;
 		default:
 			return -EINVAL;
--
2.40.1

[PATCH 6.1 4/6] xfs: short circuit xfs_growfs_data_private() if delta is zero

[Mahmoud Adam]

From: Eric Sandeen <sandeen@redhat.com>

commit 84712492e6dab803bf595fb8494d11098b74a652 upstream.

Although xfs_growfs_data() doesn't call xfs_growfs_data_private()
if in->newblocks == mp->m_sb.sb_dblocks, xfs_growfs_data_private()
further massages the new block count so that we don't i.e. try
to create a too-small new AG.

This may lead to a delta of "0" in xfs_growfs_data_private(), so
we end up in the shrink case and emit the EXPERIMENTAL warning
even if we're not changing anything at all.

Fix this by returning straightaway if the block delta is zero.

(nb: in older kernels, the result of entering the shrink case
with delta == 0 may actually let an -ENOSPC escape to userspace,
which is confusing for users.)

Fixes: fb2fc1720185 ("xfs: support shrinking unused space in the last AG")
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
Signed-off-by: Chandan Babu R <chandanbabu@kernel.org>
Signed-off-by: Mahmoud Adam <mngyadam@amazon.com>
---
 fs/xfs/xfs_fsops.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/fs/xfs/xfs_fsops.c b/fs/xfs/xfs_fsops.c
index 13851c0d640b..332da0d7b85c 100644
--- a/fs/xfs/xfs_fsops.c
+++ b/fs/xfs/xfs_fsops.c
@@ -129,6 +129,10 @@ xfs_growfs_data_private(
 	if (delta < 0 && nagcount < 2)
 		return -EINVAL;

+	/* No work to do */
+	if (delta == 0)
+		return 0;
+
 	oagcount = mp->m_sb.sb_agcount;
 	/* allocate the new per-ag structures */
 	if (nagcount > oagcount) {
--
2.40.1

[PATCH 6.1 5/6] xfs: hoist refcount record merge predicates

[Mahmoud Adam]

From: "Darrick J. Wong" <djwong@kernel.org>

commit 9d720a5a658f5135861773f26e927449bef93d61 upstream.

Hoist these multiline conditionals into separate static inline helpers
to improve readability and set the stage for corruption fixes that will
be introduced in the next patch.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Xiao Yang <yangx.jy@fujitsu.com>
Signed-off-by: Mahmoud Adam <mngyadam@amazon.com>
---
 fs/xfs/libxfs/xfs_refcount.c | 129 ++++++++++++++++++++++++++++++-----
 1 file changed, 113 insertions(+), 16 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_refcount.c b/fs/xfs/libxfs/xfs_refcount.c
index 3f34bafe18dd..4408893333a6 100644
--- a/fs/xfs/libxfs/xfs_refcount.c
+++ b/fs/xfs/libxfs/xfs_refcount.c
@@ -815,11 +815,119 @@ xfs_refcount_find_right_extents(
 /* Is this extent valid? */
 static inline bool
 xfs_refc_valid(
-	struct xfs_refcount_irec	*rc)
+	const struct xfs_refcount_irec	*rc)
 {
 	return rc->rc_startblock != NULLAGBLOCK;
 }

+static inline bool
+xfs_refc_want_merge_center(
+	const struct xfs_refcount_irec	*left,
+	const struct xfs_refcount_irec	*cleft,
+	const struct xfs_refcount_irec	*cright,
+	const struct xfs_refcount_irec	*right,
+	bool				cleft_is_cright,
+	enum xfs_refc_adjust_op		adjust,
+	unsigned long long		*ulenp)
+{
+	unsigned long long		ulen = left->rc_blockcount;
+
+	/*
+	 * To merge with a center record, both shoulder records must be
+	 * adjacent to the record we want to adjust.  This is only true if
+	 * find_left and find_right made all four records valid.
+	 */
+	if (!xfs_refc_valid(left)  || !xfs_refc_valid(right) ||
+	    !xfs_refc_valid(cleft) || !xfs_refc_valid(cright))
+		return false;
+
+	/* There must only be one record for the entire range. */
+	if (!cleft_is_cright)
+		return false;
+
+	/* The shoulder record refcounts must match the new refcount. */
+	if (left->rc_refcount != cleft->rc_refcount + adjust)
+		return false;
+	if (right->rc_refcount != cleft->rc_refcount + adjust)
+		return false;
+
+	/*
+	 * The new record cannot exceed the max length.  ulen is a ULL as the
+	 * individual record block counts can be up to (u32 - 1) in length
+	 * hence we need to catch u32 addition overflows here.
+	 */
+	ulen += cleft->rc_blockcount + right->rc_blockcount;
+	if (ulen >= MAXREFCEXTLEN)
+		return false;
+
+	*ulenp = ulen;
+	return true;
+}
+
+static inline bool
+xfs_refc_want_merge_left(
+	const struct xfs_refcount_irec	*left,
+	const struct xfs_refcount_irec	*cleft,
+	enum xfs_refc_adjust_op		adjust)
+{
+	unsigned long long		ulen = left->rc_blockcount;
+
+	/*
+	 * For a left merge, the left shoulder record must be adjacent to the
+	 * start of the range.  If this is true, find_left made left and cleft
+	 * contain valid contents.
+	 */
+	if (!xfs_refc_valid(left) || !xfs_refc_valid(cleft))
+		return false;
+
+	/* Left shoulder record refcount must match the new refcount. */
+	if (left->rc_refcount != cleft->rc_refcount + adjust)
+		return false;
+
+	/*
+	 * The new record cannot exceed the max length.  ulen is a ULL as the
+	 * individual record block counts can be up to (u32 - 1) in length
+	 * hence we need to catch u32 addition overflows here.
+	 */
+	ulen += cleft->rc_blockcount;
+	if (ulen >= MAXREFCEXTLEN)
+		return false;
+
+	return true;
+}
+
+static inline bool
+xfs_refc_want_merge_right(
+	const struct xfs_refcount_irec	*cright,
+	const struct xfs_refcount_irec	*right,
+	enum xfs_refc_adjust_op		adjust)
+{
+	unsigned long long		ulen = right->rc_blockcount;
+
+	/*
+	 * For a right merge, the right shoulder record must be adjacent to the
+	 * end of the range.  If this is true, find_right made cright and right
+	 * contain valid contents.
+	 */
+	if (!xfs_refc_valid(right) || !xfs_refc_valid(cright))
+		return false;
+
+	/* Right shoulder record refcount must match the new refcount. */
+	if (right->rc_refcount != cright->rc_refcount + adjust)
+		return false;
+
+	/*
+	 * The new record cannot exceed the max length.  ulen is a ULL as the
+	 * individual record block counts can be up to (u32 - 1) in length
+	 * hence we need to catch u32 addition overflows here.
+	 */
+	ulen += cright->rc_blockcount;
+	if (ulen >= MAXREFCEXTLEN)
+		return false;
+
+	return true;
+}
+
 /*
  * Try to merge with any extents on the boundaries of the adjustment range.
  */
@@ -861,23 +969,15 @@ xfs_refcount_merge_extents(
 		 (cleft.rc_blockcount == cright.rc_blockcount);

 	/* Try to merge left, cleft, and right.  cleft must == cright. */
-	ulen = (unsigned long long)left.rc_blockcount + cleft.rc_blockcount +
-			right.rc_blockcount;
-	if (xfs_refc_valid(&left) && xfs_refc_valid(&right) &&
-	    xfs_refc_valid(&cleft) && xfs_refc_valid(&cright) && cequal &&
-	    left.rc_refcount == cleft.rc_refcount + adjust &&
-	    right.rc_refcount == cleft.rc_refcount + adjust &&
-	    ulen < MAXREFCEXTLEN) {
+	if (xfs_refc_want_merge_center(&left, &cleft, &cright, &right, cequal,
+				adjust, &ulen)) {
 		*shape_changed = true;
 		return xfs_refcount_merge_center_extents(cur, &left, &cleft,
 				&right, ulen, aglen);
 	}

 	/* Try to merge left and cleft. */
-	ulen = (unsigned long long)left.rc_blockcount + cleft.rc_blockcount;
-	if (xfs_refc_valid(&left) && xfs_refc_valid(&cleft) &&
-	    left.rc_refcount == cleft.rc_refcount + adjust &&
-	    ulen < MAXREFCEXTLEN) {
+	if (xfs_refc_want_merge_left(&left, &cleft, adjust)) {
 		*shape_changed = true;
 		error = xfs_refcount_merge_left_extent(cur, &left, &cleft,
 				agbno, aglen);
@@ -893,10 +993,7 @@ xfs_refcount_merge_extents(
 	}

 	/* Try to merge cright and right. */
-	ulen = (unsigned long long)right.rc_blockcount + cright.rc_blockcount;
-	if (xfs_refc_valid(&right) && xfs_refc_valid(&cright) &&
-	    right.rc_refcount == cright.rc_refcount + adjust &&
-	    ulen < MAXREFCEXTLEN) {
+	if (xfs_refc_want_merge_right(&cright, &right, adjust)) {
 		*shape_changed = true;
 		return xfs_refcount_merge_right_extent(cur, &right, &cright,
 				aglen);
--
2.40.1

[PATCH 6.1 6/6] xfs: estimate post-merge refcounts correctly

[Mahmoud Adam]

From: "Darrick J. Wong" <djwong@kernel.org>

commit b25d1984aa884fc91a73a5a407b9ac976d441e9b upstream.

Upon enabling fsdax + reflink for XFS, xfs/179 began to report refcount
metadata corruptions after being run.  Specifically, xfs_repair noticed
single-block refcount records that could be combined but had not been.

The root cause of this is improper MAXREFCOUNT edge case handling in
xfs_refcount_merge_extents.  When we're trying to find candidates for a
refcount btree record merge, we compute the refcount attribute of the
merged record, but we fail to account for the fact that once a record
hits rc_refcount == MAXREFCOUNT, it is pinned that way forever.  Hence
the computed refcount is wrong, and we fail to merge the extents.

Fix this by adjusting the merge predicates to compute the adjusted
refcount correctly.

Fixes: 3172725814f9 ("xfs: adjust refcount of an extent of blocks in refcount btree")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Xiao Yang <yangx.jy@fujitsu.com>
Signed-off-by: Mahmoud Adam <mngyadam@amazon.com>
---
 fs/xfs/libxfs/xfs_refcount.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_refcount.c b/fs/xfs/libxfs/xfs_refcount.c
index 4408893333a6..6f7ed9288fe4 100644
--- a/fs/xfs/libxfs/xfs_refcount.c
+++ b/fs/xfs/libxfs/xfs_refcount.c
@@ -820,6 +820,17 @@ xfs_refc_valid(
 	return rc->rc_startblock != NULLAGBLOCK;
 }

+static inline xfs_nlink_t
+xfs_refc_merge_refcount(
+	const struct xfs_refcount_irec	*irec,
+	enum xfs_refc_adjust_op		adjust)
+{
+	/* Once a record hits MAXREFCOUNT, it is pinned there forever */
+	if (irec->rc_refcount == MAXREFCOUNT)
+		return MAXREFCOUNT;
+	return irec->rc_refcount + adjust;
+}
+
 static inline bool
 xfs_refc_want_merge_center(
 	const struct xfs_refcount_irec	*left,
@@ -831,6 +842,7 @@ xfs_refc_want_merge_center(
 	unsigned long long		*ulenp)
 {
 	unsigned long long		ulen = left->rc_blockcount;
+	xfs_nlink_t			new_refcount;

 	/*
 	 * To merge with a center record, both shoulder records must be
@@ -846,9 +858,10 @@ xfs_refc_want_merge_center(
 		return false;

 	/* The shoulder record refcounts must match the new refcount. */
-	if (left->rc_refcount != cleft->rc_refcount + adjust)
+	new_refcount = xfs_refc_merge_refcount(cleft, adjust);
+	if (left->rc_refcount != new_refcount)
 		return false;
-	if (right->rc_refcount != cleft->rc_refcount + adjust)
+	if (right->rc_refcount != new_refcount)
 		return false;

 	/*
@@ -871,6 +884,7 @@ xfs_refc_want_merge_left(
 	enum xfs_refc_adjust_op		adjust)
 {
 	unsigned long long		ulen = left->rc_blockcount;
+	xfs_nlink_t			new_refcount;

 	/*
 	 * For a left merge, the left shoulder record must be adjacent to the
@@ -881,7 +895,8 @@ xfs_refc_want_merge_left(
 		return false;

 	/* Left shoulder record refcount must match the new refcount. */
-	if (left->rc_refcount != cleft->rc_refcount + adjust)
+	new_refcount = xfs_refc_merge_refcount(cleft, adjust);
+	if (left->rc_refcount != new_refcount)
 		return false;

 	/*
@@ -903,6 +918,7 @@ xfs_refc_want_merge_right(
 	enum xfs_refc_adjust_op		adjust)
 {
 	unsigned long long		ulen = right->rc_blockcount;
+	xfs_nlink_t			new_refcount;

 	/*
 	 * For a right merge, the right shoulder record must be adjacent to the
@@ -913,7 +929,8 @@ xfs_refc_want_merge_right(
 		return false;

 	/* Right shoulder record refcount must match the new refcount. */
-	if (right->rc_refcount != cright->rc_refcount + adjust)
+	new_refcount = xfs_refc_merge_refcount(cright, adjust);
+	if (right->rc_refcount != new_refcount)
 		return false;

 	/*
--
2.40.1

Re: [PATCH 6.1 0/6] backport xfs fix patches reported by xfs/179/270/557/606

[Darrick J. Wong]

On Wed, Apr 03, 2024 at 02:59:44PM +0200, Mahmoud Adam wrote:
> Hi,
> 
>  These patches fix and reported by xfstests tests xfs/179 xfs/270
> xfs/557 xfs/606, the patchset were tested to confirm they fix those
> tests. all are clean picks.

Hi!  Thanks for the backports!

Normally I'd pass these on to the 6.1 XFS maintainer, but I'm not sure
who's actually taking care of that at the moment.  To find out, I've
cc'd all the people who have either sent 6.1 backports or made noises
about doing so.

To the group: Who's the appropriate person to handle these?

Mahmoud: If the answer to the above is "???" or silence, would you be
willing to take on stable testing and maintenance?

Also FYI the normal practice (I think) is to cc linux-xfs, pick up some
acks, and then resend with the acks and cc'd to stable.

The six patches you sent along look ok to me, so
Acked-by: Darrick J. Wong <djwong@kernel.org>

--D

> thanks,
> MNAdam

Re: [PATCH 6.1 0/6] backport xfs fix patches reported by xfs/179/270/557/606

[Amir Goldstein]

On Wed, Apr 3, 2024 at 9:18 PM Darrick J. Wong <djwong@kernel.org> wrote:
>
> On Wed, Apr 03, 2024 at 02:59:44PM +0200, Mahmoud Adam wrote:
> > Hi,
> >
> >  These patches fix and reported by xfstests tests xfs/179 xfs/270
> > xfs/557 xfs/606, the patchset were tested to confirm they fix those
> > tests. all are clean picks.
>
> Hi!  Thanks for the backports!
>
> Normally I'd pass these on to the 6.1 XFS maintainer, but I'm not sure
> who's actually taking care of that at the moment.  To find out, I've
> cc'd all the people who have either sent 6.1 backports or made noises
> about doing so.

Leah has claimed that she will take over 6.1.y ;)
Leah, do you have any staged backports for 6.1.y already?
Can easily fire up a test run of these backports?

https://lore.kernel.org/stable/20240403125949.33676-1-mngyadam@amazon.com/

It looks like most of the backports are from 2023 (v6.1..v6.6)
except for patch 4/6 which has been backported to 6.6.y already.

>
> To the group: Who's the appropriate person to handle these?
>
> Mahmoud: If the answer to the above is "???" or silence, would you be
> willing to take on stable testing and maintenance?
>
> Also FYI the normal practice (I think) is to cc linux-xfs, pick up some
> acks, and then resend with the acks and cc'd to stable.
>

Mahmoud,

I assume that you are running xfstests on LTS kernels regularly?
In that case, you should have an established baseline for failing/passing
tests on 6.1.y.
Did you run these backports against all tests to verify no regressions?
If you did - then please include this information (also which xfs configurations
were tested) in the posting of backport candidates to xfs list.

That is effectively the only thing that is required for doing reliable LTS
backport work.

Thanks,
Amir.

Re: [PATCH 6.1 0/6] backport xfs fix patches reported by xfs/179/270/557/606

[Mahmoud Adam]

Amir Goldstein <amir73il@gmail.com> writes:

> On Wed, Apr 3, 2024 at 9:18 PM Darrick J. Wong <djwong@kernel.org> wrote:
>> To the group: Who's the appropriate person to handle these?
>>
>> Mahmoud: If the answer to the above is "???" or silence, would you be
>> willing to take on stable testing and maintenance?

Probably there is an answer now :). But Yes, I'm okay with doing that,
Xfstests is already part for our nightly 6.1 testing.

>
> Mahmoud,
>
> I assume that you are running xfstests on LTS kernels regularly?
> In that case, you should have an established baseline for failing/passing
> tests on 6.1.y.
> Did you run these backports against all tests to verify no regressions?
> If you did - then please include this information (also which xfs configurations
> were tested) in the posting of backport candidates to xfs list.

Yes, I did run the full xfstests to confirm no regression. we do
regularly run the latest stable xfstests version with loopback
setup. and we run 'xfs/quick' group over x86_64 & arm64 to catch any
regression. I'll make sure to post to xfs list first next time :)

our setup looks similar to this:

sudo fallocate -l 5G $MOUNT_POINT/block-xfs.img
sudo mkfs.xfs -f -m reflink=1 $MOUNT_POINT/block-xfs.img
sudo losetup -f $MOUNT_POINT/block-xfs.img
sudo mkdir -p $MOUNT_POINT/test
sudo mount /dev/loop0 $MOUNT_POINT/test

sudo fallocate -l 5G $MOUNT_POINT/block-xfs-scratch.img
sudo losetup -f $MOUNT_POINT/block-xfs-scratch.img

local.config:
    export DISABLE_UDF_TEST=1
    export TEST_DEV=/dev/loop0
    export TEST_DIR=$MOUNT_POINT/test
    export SCRATCH_MNT=$MOUNT_POINT/scratch
    export SCRATCH_DEV=/dev/loop1

Thanks,
MNAdam

Re: [PATCH 6.1 0/6] backport xfs fix patches reported by xfs/179/270/557/606

[Greg KH]

On Thu, Apr 04, 2024 at 11:15:25AM +0200, Mahmoud Adam wrote:
> Amir Goldstein <amir73il@gmail.com> writes:
> 
> > On Wed, Apr 3, 2024 at 9:18 PM Darrick J. Wong <djwong@kernel.org> wrote:
> >> To the group: Who's the appropriate person to handle these?
> >>
> >> Mahmoud: If the answer to the above is "???" or silence, would you be
> >> willing to take on stable testing and maintenance?
> 
> Probably there is an answer now :). But Yes, I'm okay with doing that,
> Xfstests is already part for our nightly 6.1 testing.
> 
> >
> > Mahmoud,
> >
> > I assume that you are running xfstests on LTS kernels regularly?
> > In that case, you should have an established baseline for failing/passing
> > tests on 6.1.y.
> > Did you run these backports against all tests to verify no regressions?
> > If you did - then please include this information (also which xfs configurations
> > were tested) in the posting of backport candidates to xfs list.
> 
> Yes, I did run the full xfstests to confirm no regression. we do
> regularly run the latest stable xfstests version with loopback
> setup. and we run 'xfs/quick' group over x86_64 & arm64 to catch any
> regression. I'll make sure to post to xfs list first next time :)
> 
> our setup looks similar to this:
> 
> sudo fallocate -l 5G $MOUNT_POINT/block-xfs.img
> sudo mkfs.xfs -f -m reflink=1 $MOUNT_POINT/block-xfs.img
> sudo losetup -f $MOUNT_POINT/block-xfs.img
> sudo mkdir -p $MOUNT_POINT/test
> sudo mount /dev/loop0 $MOUNT_POINT/test
> 
> sudo fallocate -l 5G $MOUNT_POINT/block-xfs-scratch.img
> sudo losetup -f $MOUNT_POINT/block-xfs-scratch.img
> 
> local.config:
>     export DISABLE_UDF_TEST=1
>     export TEST_DEV=/dev/loop0
>     export TEST_DIR=$MOUNT_POINT/test
>     export SCRATCH_MNT=$MOUNT_POINT/scratch
>     export SCRATCH_DEV=/dev/loop1

So does this mean we should take these for stable inclusion, or are they
going to need some other tests/acks for us to be able to do this?

thanks,

greg k-h

Re: [PATCH 6.1 0/6] backport xfs fix patches reported by xfs/179/270/557/606

[Amir Goldstein]

On Fri, Apr 5, 2024 at 12:27 PM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Thu, Apr 04, 2024 at 11:15:25AM +0200, Mahmoud Adam wrote:
> > Amir Goldstein <amir73il@gmail.com> writes:
> >
> > > On Wed, Apr 3, 2024 at 9:18 PM Darrick J. Wong <djwong@kernel.org> wrote:
> > >> To the group: Who's the appropriate person to handle these?
> > >>
> > >> Mahmoud: If the answer to the above is "???" or silence, would you be
> > >> willing to take on stable testing and maintenance?
> >
> > Probably there is an answer now :). But Yes, I'm okay with doing that,
> > Xfstests is already part for our nightly 6.1 testing.

Let's wait for Leah to chime in and then decide.
Leah's test coverage is larger than the tests that Mahmoud ran.

Thanks,
Amir.

Re: [PATCH 6.1 0/6] backport xfs fix patches reported by xfs/179/270/557/606

[Amir Goldstein]

On Fri, Apr 5, 2024 at 2:12 PM Mahmoud Adam <mngyadam@amazon.com> wrote:
>
>
> Dropping stable mailing list to avoid spamming the thread

Adding Chandan and xfs list.

> Amir Goldstein <amir73il@gmail.com> writes:
>
> > On Fri, Apr 5, 2024 at 12:27 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> >>
> >> On Thu, Apr 04, 2024 at 11:15:25AM +0200, Mahmoud Adam wrote:
> >> > Amir Goldstein <amir73il@gmail.com> writes:
> >> >
> >> > > On Wed, Apr 3, 2024 at 9:18 PM Darrick J. Wong <djwong@kernel.org> wrote:
> >> > >> To the group: Who's the appropriate person to handle these?
> >> > >>
> >> > >> Mahmoud: If the answer to the above is "???" or silence, would you be
> >> > >> willing to take on stable testing and maintenance?
> >> >
> >> > Probably there is an answer now :). But Yes, I'm okay with doing that,
> >> > Xfstests is already part for our nightly 6.1 testing.
> >
> > Let's wait for Leah to chime in and then decide.
> > Leah's test coverage is larger than the tests that Mahmoud ran.
> >
>
> For curiosity, What kind of larger coverage used?

If you only run 'xfs/quick' that is a small part of the tests.
generic/quick are not a bit least important, but generally speaking
several rounds of -g auto is the standard for regression testing.

kdevops runs these 7 xfs configurations by default:
https://github.com/linux-kdevops/kdevops/blob/main/workflows/fstests/xfs/Kconfig#L763

but every tester can customize the configurations.
Leah is running gce-xfstests with some other set of configurations.

Thanks,
Amir.

Re: [PATCH 6.1 0/6] backport xfs fix patches reported by xfs/179/270/557/606

[Leah Rumancik]

Hi!

I'm happy to run some tests for this. I have a set currently in
progress. Can do this set next and probably have out in a week or two
if that timeline works for you.

- Leah


On Fri, Apr 5, 2024 at 7:40 AM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Fri, Apr 5, 2024 at 2:12 PM Mahmoud Adam <mngyadam@amazon.com> wrote:
> >
> >
> > Dropping stable mailing list to avoid spamming the thread
>
> Adding Chandan and xfs list.
>
> > Amir Goldstein <amir73il@gmail.com> writes:
> >
> > > On Fri, Apr 5, 2024 at 12:27 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > >>
> > >> On Thu, Apr 04, 2024 at 11:15:25AM +0200, Mahmoud Adam wrote:
> > >> > Amir Goldstein <amir73il@gmail.com> writes:
> > >> >
> > >> > > On Wed, Apr 3, 2024 at 9:18 PM Darrick J. Wong <djwong@kernel.org> wrote:
> > >> > >> To the group: Who's the appropriate person to handle these?
> > >> > >>
> > >> > >> Mahmoud: If the answer to the above is "???" or silence, would you be
> > >> > >> willing to take on stable testing and maintenance?
> > >> >
> > >> > Probably there is an answer now :). But Yes, I'm okay with doing that,
> > >> > Xfstests is already part for our nightly 6.1 testing.
> > >
> > > Let's wait for Leah to chime in and then decide.
> > > Leah's test coverage is larger than the tests that Mahmoud ran.
> > >
> >
> > For curiosity, What kind of larger coverage used?
>
> If you only run 'xfs/quick' that is a small part of the tests.
> generic/quick are not a bit least important, but generally speaking
> several rounds of -g auto is the standard for regression testing.
>
> kdevops runs these 7 xfs configurations by default:
> https://github.com/linux-kdevops/kdevops/blob/main/workflows/fstests/xfs/Kconfig#L763
>
> but every tester can customize the configurations.
> Leah is running gce-xfstests with some other set of configurations.
>
> Thanks,
> Amir.
>

Re: [PATCH 6.1 0/6] backport xfs fix patches reported by xfs/179/270/557/606

[Greg KH]

On Fri, Apr 05, 2024 at 12:55:41PM +0300, Amir Goldstein wrote:
> On Fri, Apr 5, 2024 at 12:27 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Thu, Apr 04, 2024 at 11:15:25AM +0200, Mahmoud Adam wrote:
> > > Amir Goldstein <amir73il@gmail.com> writes:
> > >
> > > > On Wed, Apr 3, 2024 at 9:18 PM Darrick J. Wong <djwong@kernel.org> wrote:
> > > >> To the group: Who's the appropriate person to handle these?
> > > >>
> > > >> Mahmoud: If the answer to the above is "???" or silence, would you be
> > > >> willing to take on stable testing and maintenance?
> > >
> > > Probably there is an answer now :). But Yes, I'm okay with doing that,
> > > Xfstests is already part for our nightly 6.1 testing.
> 
> Let's wait for Leah to chime in and then decide.
> Leah's test coverage is larger than the tests that Mahmoud ran.

Ok, I'll drop these from my review queue now, when they are "good
enough" can someone resend them please?

thanks,

greg k-h

Re: [PATCH 6.1 0/6] backport xfs fix patches reported by xfs/179/270/557/606

[Leah Rumancik]

Thanks, will do!

- Leah


On Thu, Apr 11, 2024 at 2:22 AM Greg KH <gregkh@linuxfoundation.org> wrote:
>
> On Fri, Apr 05, 2024 at 12:55:41PM +0300, Amir Goldstein wrote:
> > On Fri, Apr 5, 2024 at 12:27 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > >
> > > On Thu, Apr 04, 2024 at 11:15:25AM +0200, Mahmoud Adam wrote:
> > > > Amir Goldstein <amir73il@gmail.com> writes:
> > > >
> > > > > On Wed, Apr 3, 2024 at 9:18 PM Darrick J. Wong <djwong@kernel.org> wrote:
> > > > >> To the group: Who's the appropriate person to handle these?
> > > > >>
> > > > >> Mahmoud: If the answer to the above is "???" or silence, would you be
> > > > >> willing to take on stable testing and maintenance?
> > > >
> > > > Probably there is an answer now :). But Yes, I'm okay with doing that,
> > > > Xfstests is already part for our nightly 6.1 testing.
> >
> > Let's wait for Leah to chime in and then decide.
> > Leah's test coverage is larger than the tests that Mahmoud ran.
>
> Ok, I'll drop these from my review queue now, when they are "good
> enough" can someone resend them please?
>
> thanks,
>
> greg k-h

Re: [PATCH 6.1 0/6] backport xfs fix patches reported by xfs/179/270/557/606

[Leah Rumancik]

Just an update on this, I added these patches to my current set and am
now seeing issues with the logdev config not finishing. Will separate
out into two sets again and try to track down the issue.

- Leah

On Fri, Apr 5, 2024 at 11:53 AM Leah Rumancik <leah.rumancik@gmail.com> wrote:
>
> Hi!
>
> I'm happy to run some tests for this. I have a set currently in
> progress. Can do this set next and probably have out in a week or two
> if that timeline works for you.
>
> - Leah
>
>
> On Fri, Apr 5, 2024 at 7:40 AM Amir Goldstein <amir73il@gmail.com> wrote:
> >
> > On Fri, Apr 5, 2024 at 2:12 PM Mahmoud Adam <mngyadam@amazon.com> wrote:
> > >
> > >
> > > Dropping stable mailing list to avoid spamming the thread
> >
> > Adding Chandan and xfs list.
> >
> > > Amir Goldstein <amir73il@gmail.com> writes:
> > >
> > > > On Fri, Apr 5, 2024 at 12:27 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > > >>
> > > >> On Thu, Apr 04, 2024 at 11:15:25AM +0200, Mahmoud Adam wrote:
> > > >> > Amir Goldstein <amir73il@gmail.com> writes:
> > > >> >
> > > >> > > On Wed, Apr 3, 2024 at 9:18 PM Darrick J. Wong <djwong@kernel.org> wrote:
> > > >> > >> To the group: Who's the appropriate person to handle these?
> > > >> > >>
> > > >> > >> Mahmoud: If the answer to the above is "???" or silence, would you be
> > > >> > >> willing to take on stable testing and maintenance?
> > > >> >
> > > >> > Probably there is an answer now :). But Yes, I'm okay with doing that,
> > > >> > Xfstests is already part for our nightly 6.1 testing.
> > > >
> > > > Let's wait for Leah to chime in and then decide.
> > > > Leah's test coverage is larger than the tests that Mahmoud ran.
> > > >
> > >
> > > For curiosity, What kind of larger coverage used?
> >
> > If you only run 'xfs/quick' that is a small part of the tests.
> > generic/quick are not a bit least important, but generally speaking
> > several rounds of -g auto is the standard for regression testing.
> >
> > kdevops runs these 7 xfs configurations by default:
> > https://github.com/linux-kdevops/kdevops/blob/main/workflows/fstests/xfs/Kconfig#L763
> >
> > but every tester can customize the configurations.
> > Leah is running gce-xfstests with some other set of configurations.
> >
> > Thanks,
> > Amir.
> >