Message ID | 169687595684.3969352.13337782664797983922.stgit@frogsfrogsfrogs (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | xfs: bug fixes for 6.6 | expand |
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>
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 --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); }