diff mbox series

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

Message ID 169687595684.3969352.13337782664797983922.stgit@frogsfrogsfrogs (mailing list archive)
State New, archived
Headers show
Series xfs: bug fixes for 6.6 | expand

Commit Message

Darrick J. Wong Oct. 9, 2023, 6:25 p.m. UTC 
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(-)

Comments

Christoph Hellwig Oct. 10, 2023, 6:56 a.m. UTC | #1 
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>
Darrick J. Wong Oct. 10, 2023, 5:34 p.m. UTC | #2 
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>
diff mbox series

Patch

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);
 }