Greg,
Chandan is preparing a series of backports from v5.11 to 5.4.y. These two backports were selected by Chandan for 5.4.y, but are currently missing from 5.10.y.
Specifically, patch #2 fixes a problem seen in the wild on UEK and the UEK kernels already carry this patch.
The patches have gone through the usual xfs test/review routine.
Thanks, Amir.
Brian Foster (1): xfs: don't reuse busy extents on extent trim
Darrick J. Wong (1): xfs: shut down the filesystem if we screw up quota reservation
fs/xfs/xfs_extent_busy.c | 14 -------------- fs/xfs/xfs_trans_dquot.c | 13 ++++++++++--- 2 files changed, 10 insertions(+), 17 deletions(-)
From: "Darrick J. Wong" djwong@kernel.org
commit 2a4bdfa8558ca2904dc17b83497dc82aa7fc05e9 upstream.
If we ever screw up the quota reservations enough to trip the assertions, something's wrong with the quota code. Shut down the filesystem when this happens, because this is corruption.
Signed-off-by: Darrick J. Wong djwong@kernel.org Reviewed-by: Christoph Hellwig hch@lst.de Reviewed-by: Brian Foster bfoster@redhat.com Signed-off-by: Amir Goldstein amir73il@gmail.com Acked-by: Darrick J. Wong djwong@kernel.org --- fs/xfs/xfs_trans_dquot.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/fs/xfs/xfs_trans_dquot.c b/fs/xfs/xfs_trans_dquot.c index 288ea38c43ad..5ca210e6626c 100644 --- a/fs/xfs/xfs_trans_dquot.c +++ b/fs/xfs/xfs_trans_dquot.c @@ -16,6 +16,7 @@ #include "xfs_quota.h" #include "xfs_qm.h" #include "xfs_trace.h" +#include "xfs_error.h"
STATIC void xfs_trans_alloc_dqinfo(xfs_trans_t *);
@@ -708,9 +709,11 @@ xfs_trans_dqresv( XFS_TRANS_DQ_RES_INOS, ninos); } - ASSERT(dqp->q_blk.reserved >= dqp->q_blk.count); - ASSERT(dqp->q_rtb.reserved >= dqp->q_rtb.count); - ASSERT(dqp->q_ino.reserved >= dqp->q_ino.count); + + if (XFS_IS_CORRUPT(mp, dqp->q_blk.reserved < dqp->q_blk.count) || + XFS_IS_CORRUPT(mp, dqp->q_rtb.reserved < dqp->q_rtb.count) || + XFS_IS_CORRUPT(mp, dqp->q_ino.reserved < dqp->q_ino.count)) + goto error_corrupt;
xfs_dqunlock(dqp); return 0; @@ -720,6 +723,10 @@ xfs_trans_dqresv( if (xfs_dquot_type(dqp) == XFS_DQTYPE_PROJ) return -ENOSPC; return -EDQUOT; +error_corrupt: + xfs_dqunlock(dqp); + xfs_force_shutdown(mp, SHUTDOWN_CORRUPT_INCORE); + return -EFSCORRUPTED; }
From: Brian Foster bfoster@redhat.com
commit 06058bc40534530e617e5623775c53bb24f032cb upstream.
Freed extents are marked busy from the point the freeing transaction commits until the associated CIL context is checkpointed to the log. This prevents reuse and overwrite of recently freed blocks before the changes are committed to disk, which can lead to corruption after a crash. The exception to this rule is that metadata allocation is allowed to reuse busy extents because metadata changes are also logged.
As of commit 97d3ac75e5e0 ("xfs: exact busy extent tracking"), XFS has allowed modification or complete invalidation of outstanding busy extents for metadata allocations. This implementation assumes that use of the associated extent is imminent, which is not always the case. For example, the trimmed extent might not satisfy the minimum length of the allocation request, or the allocation algorithm might be involved in a search for the optimal result based on locality.
generic/019 reproduces a corruption caused by this scenario. First, a metadata block (usually a bmbt or symlink block) is freed from an inode. A subsequent bmbt split on an unrelated inode attempts a near mode allocation request that invalidates the busy block during the search, but does not ultimately allocate it. Due to the busy state invalidation, the block is no longer considered busy to subsequent allocation. A direct I/O write request immediately allocates the block and writes to it. Finally, the filesystem crashes while in a state where the initial metadata block free had not committed to the on-disk log. After recovery, the original metadata block is in its original location as expected, but has been corrupted by the aforementioned dio.
This demonstrates that it is fundamentally unsafe to modify busy extent state for extents that are not guaranteed to be allocated. This applies to pretty much all of the code paths that currently trim busy extents for one reason or another. Therefore to address this problem, drop the reuse mechanism from the busy extent trim path. This code already knows how to return partial non-busy ranges of the targeted free extent and higher level code tracks the busy state of the allocation attempt. If a block allocation fails where one or more candidate extents is busy, we force the log and retry the allocation.
Signed-off-by: Brian Foster bfoster@redhat.com Reviewed-by: Darrick J. Wong djwong@kernel.org Signed-off-by: Darrick J. Wong djwong@kernel.org Reviewed-by: Chandan Babu R chandanrlinux@gmail.com Reviewed-by: Christoph Hellwig hch@lst.de Signed-off-by: Amir Goldstein amir73il@gmail.com Acked-by: Darrick J. Wong djwong@kernel.org --- fs/xfs/xfs_extent_busy.c | 14 -------------- 1 file changed, 14 deletions(-)
diff --git a/fs/xfs/xfs_extent_busy.c b/fs/xfs/xfs_extent_busy.c index 5c2695a42de1..a4075685d9eb 100644 --- a/fs/xfs/xfs_extent_busy.c +++ b/fs/xfs/xfs_extent_busy.c @@ -344,7 +344,6 @@ xfs_extent_busy_trim( ASSERT(*len > 0);
spin_lock(&args->pag->pagb_lock); -restart: fbno = *bno; flen = *len; rbp = args->pag->pagb_tree.rb_node; @@ -363,19 +362,6 @@ xfs_extent_busy_trim( continue; }
- /* - * If this is a metadata allocation, try to reuse the busy - * extent instead of trimming the allocation. - */ - if (!(args->datatype & XFS_ALLOC_USERDATA) && - !(busyp->flags & XFS_EXTENT_BUSY_DISCARDED)) { - if (!xfs_extent_busy_update_extent(args->mp, args->pag, - busyp, fbno, flen, - false)) - goto restart; - continue; - } - if (bbno <= fbno) { /* start overlap */
On Tue, Mar 28, 2023 at 10:35:10AM +0300, Amir Goldstein wrote:
Greg,
Chandan is preparing a series of backports from v5.11 to 5.4.y. These two backports were selected by Chandan for 5.4.y, but are currently missing from 5.10.y.
Specifically, patch #2 fixes a problem seen in the wild on UEK and the UEK kernels already carry this patch.
The patches have gone through the usual xfs test/review routine.
Now queued up, thanks.
greg k-h
linux-stable-mirror@lists.linaro.org
-
Amir Goldstein -
Greg Kroah-Hartman