[PATCHSET 0/2] xfs: bug fixes for 6.6

[PATCHSET 0/2] xfs: bug fixes for 6.6

[Darrick J. Wong]

Hi all,

Bug fixes for XFS for 6.6.

If you're going to start using this code, I strongly recommend pulling
from my git trees, which are linked below.

This has been running on the djcloud for months with no problems.  Enjoy!
Comments and questions are, as always, welcome.

--D

kernel git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfs-linux.git/log/?h=xfs-6.6-fixes
---
 fs/xfs/libxfs/xfs_ag.c   |    6 ++++++
 fs/xfs/xfs_extent_busy.c |    3 ++-
 2 files changed, 8 insertions(+), 1 deletion(-)

[PATCH 1/2] xfs: adjust the incore perag block_count when shrinking

[Darrick J. Wong]

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

If we reduce the number of blocks in an AG, we must update the incore
geometry values as well.

Fixes: 0800169e3e2c9 ("xfs: Pre-calculate per-AG agbno geometry")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 fs/xfs/libxfs/xfs_ag.c |    6 ++++++
 1 file changed, 6 insertions(+)


diff --git a/fs/xfs/libxfs/xfs_ag.c b/fs/xfs/libxfs/xfs_ag.c
index e9cc481b4ddff..f9f4d694640d0 100644
--- a/fs/xfs/libxfs/xfs_ag.c
+++ b/fs/xfs/libxfs/xfs_ag.c
@@ -1001,6 +1001,12 @@ xfs_ag_shrink_space(
 		error = -ENOSPC;
 		goto resv_init_out;
 	}
+
+	/* Update perag geometry */
+	pag->block_count -= delta;
+	__xfs_agino_range(pag->pag_mount, pag->block_count, &pag->agino_min,
+				&pag->agino_max);
+
 	xfs_ialloc_log_agi(*tpp, agibp, XFS_AGI_LENGTH);
 	xfs_alloc_log_agf(*tpp, agfbp, XFS_AGF_LENGTH);
 	return 0;

Re: [PATCH 1/2] xfs: adjust the incore perag block_count when shrinking

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 2/2] xfs: process free extents to busy list in FIFO order

[Darrick J. Wong]

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

When we're adding extents to the busy discard list, add them to the tail
of the list so that we get FIFO order.  For FITRIM commands, this means
that we send discard bios sorted in order from longest to shortest, like
we did before commit 89cfa899608fc.

For transactions that are freeing extents, this puts them in the
transaction's busy list in FIFO order as well, which shouldn't make any
noticeable difference.

Fixes: 89cfa899608fc ("xfs: reduce AGF hold times during fstrim operations")
Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 fs/xfs/xfs_extent_busy.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)


diff --git a/fs/xfs/xfs_extent_busy.c b/fs/xfs/xfs_extent_busy.c
index 746814815b1da..9ecfdcdc752f7 100644
--- a/fs/xfs/xfs_extent_busy.c
+++ b/fs/xfs/xfs_extent_busy.c
@@ -62,7 +62,8 @@ xfs_extent_busy_insert_list(
 	rb_link_node(&new->rb_node, parent, rbp);
 	rb_insert_color(&new->rb_node, &pag->pagb_tree);
 
-	list_add(&new->list, busy_list);
+	/* always process discard lists in fifo order */
+	list_add_tail(&new->list, busy_list);
 	spin_unlock(&pag->pagb_lock);
 }
 

Re: [PATCH 2/2] xfs: process free extents to busy list in FIFO order

[Christoph Hellwig]

On Mon, Oct 09, 2023 at 11:25:56AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> When we're adding extents to the busy discard list, add them to the tail
> of the list so that we get FIFO order.  For FITRIM commands, this means
> that we send discard bios sorted in order from longest to shortest, like
> we did before commit 89cfa899608fc.
> 
> For transactions that are freeing extents, this puts them in the
> transaction's busy list in FIFO order as well, which shouldn't make any
> noticeable difference.
> 
> Fixes: 89cfa899608fc ("xfs: reduce AGF hold times during fstrim operations")
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>

Does this actually fix an observed issue, or just restor the previous
behavior?

Eitherway the change make sense:

Reviewed-by: Christoph Hellwig <hch@lst.de>

Re: [PATCH 2/2] xfs: process free extents to busy list in FIFO order

[Darrick J. Wong]

On Mon, Oct 09, 2023 at 11:56:32PM -0700, Christoph Hellwig wrote:
> On Mon, Oct 09, 2023 at 11:25:56AM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > When we're adding extents to the busy discard list, add them to the tail
> > of the list so that we get FIFO order.  For FITRIM commands, this means
> > that we send discard bios sorted in order from longest to shortest, like
> > we did before commit 89cfa899608fc.
> > 
> > For transactions that are freeing extents, this puts them in the
> > transaction's busy list in FIFO order as well, which shouldn't make any
> > noticeable difference.
> > 
> > Fixes: 89cfa899608fc ("xfs: reduce AGF hold times during fstrim operations")
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> 
> Does this actually fix an observed issue, or just restor the previous
> behavior?

I don't /think/ there's a real issue here; it just looks funny that the
tracepoints are now ordered from higher to lower LBAs within arbitrarily
sized groups.

--D

> Eitherway the change make sense:
> 
> Reviewed-by: Christoph Hellwig <hch@lst.de>