This is a note to let you know that I've just added the patch titled
xfs: reorder iunlink remove operation in xfs_ifree
to the 5.10-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
xfs-reorder-iunlink-remove-operation-in-xfs_ifree.patch
and it can be found in the queue-5.10 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.
>From foo@baz Sat Sep 24 11:38:56 AM CEST 2022
From: Amir Goldstein <amir73il@xxxxxxxxx>
Date: Thu, 22 Sep 2022 18:47:27 +0300
Subject: xfs: reorder iunlink remove operation in xfs_ifree
To: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
Cc: Sasha Levin <sashal@xxxxxxxxxx>, "Darrick J . Wong" <djwong@xxxxxxxxxx>, Leah Rumancik <leah.rumancik@xxxxxxxxx>, Chandan Babu R <chandan.babu@xxxxxxxxxx>, linux-xfs@xxxxxxxxxxxxxxx, stable@xxxxxxxxxxxxxxx, Dave Chinner <dchinner@xxxxxxxxxx>, Frank Hofmann <fhofmann@xxxxxxxxxxxxxx>, "Darrick J . Wong" <darrick.wong@xxxxxxxxxx>, Dave Chinner <david@xxxxxxxxxxxxx>
Message-ID: <20220922154728.97402-2-amir73il@xxxxxxxxx>
From: Dave Chinner <dchinner@xxxxxxxxxx>
commit 9a5280b312e2e7898b6397b2ca3cfd03f67d7be1 upstream.
[backport for 5.10.y]
The O_TMPFILE creation implementation creates a specific order of
operations for inode allocation/freeing and unlinked list
modification. Currently both are serialised by the AGI, so the order
doesn't strictly matter as long as the are both in the same
transaction.
However, if we want to move the unlinked list insertions largely out
from under the AGI lock, then we have to be concerned about the
order in which we do unlinked list modification operations.
O_TMPFILE creation tells us this order is inode allocation/free,
then unlinked list modification.
Change xfs_ifree() to use this same ordering on unlinked list
removal. This way we always guarantee that when we enter the
iunlinked list removal code from this path, we already have the AGI
locked and we don't have to worry about lock nesting AGI reads
inside unlink list locks because it's already locked and attached to
the transaction.
We can do this safely as the inode freeing and unlinked list removal
are done in the same transaction and hence are atomic operations
with respect to log recovery.
Reported-by: Frank Hofmann <fhofmann@xxxxxxxxxxxxxx>
Fixes: 298f7bec503f ("xfs: pin inode backing buffer to the inode log item")
Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
Reviewed-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
Signed-off-by: Dave Chinner <david@xxxxxxxxxxxxx>
Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
Acked-by: Darrick J. Wong <djwong@xxxxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
---
fs/xfs/xfs_inode.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
--- a/fs/xfs/xfs_inode.c
+++ b/fs/xfs/xfs_inode.c
@@ -2669,14 +2669,13 @@ xfs_ifree_cluster(
}
/*
- * This is called to return an inode to the inode free list.
- * The inode should already be truncated to 0 length and have
- * no pages associated with it. This routine also assumes that
- * the inode is already a part of the transaction.
+ * This is called to return an inode to the inode free list. The inode should
+ * already be truncated to 0 length and have no pages associated with it. This
+ * routine also assumes that the inode is already a part of the transaction.
*
- * The on-disk copy of the inode will have been added to the list
- * of unlinked inodes in the AGI. We need to remove the inode from
- * that list atomically with respect to freeing it here.
+ * The on-disk copy of the inode will have been added to the list of unlinked
+ * inodes in the AGI. We need to remove the inode from that list atomically with
+ * respect to freeing it here.
*/
int
xfs_ifree(
@@ -2694,13 +2693,16 @@ xfs_ifree(
ASSERT(ip->i_d.di_nblocks == 0);
/*
- * Pull the on-disk inode from the AGI unlinked list.
+ * Free the inode first so that we guarantee that the AGI lock is going
+ * to be taken before we remove the inode from the unlinked list. This
+ * makes the AGI lock -> unlinked list modification order the same as
+ * used in O_TMPFILE creation.
*/
- error = xfs_iunlink_remove(tp, ip);
+ error = xfs_difree(tp, ip->i_ino, &xic);
if (error)
return error;
- error = xfs_difree(tp, ip->i_ino, &xic);
+ error = xfs_iunlink_remove(tp, ip);
if (error)
return error;
Patches currently in stable-queue which might be from amir73il@xxxxxxxxx are
queue-5.10/xfs-reorder-iunlink-remove-operation-in-xfs_ifree.patch
queue-5.10/xfs-validate-inode-fork-size-against-fork-format.patch
queue-5.10/xfs-fix-up-non-directory-creation-in-sgid-directories.patch
