On Wed, Feb 01, 2023 at 03:15:15PM -0800, Darrick J. Wong wrote:
> On Thu, Jan 19, 2023 at 09:44:52AM +1100, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@xxxxxxxxxx>
> >
> > To convert it to using active perag references and hence make it
> > shrink safe.
> >
> > Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> > ---
> > fs/xfs/xfs_discard.c | 50 ++++++++++++++++++++------------------------
> > 1 file changed, 23 insertions(+), 27 deletions(-)
> >
> > diff --git a/fs/xfs/xfs_discard.c b/fs/xfs/xfs_discard.c
> > index bfc829c07f03..afc4c78b9eed 100644
> > --- a/fs/xfs/xfs_discard.c
> > +++ b/fs/xfs/xfs_discard.c
> > @@ -21,23 +21,20 @@
> >
> > STATIC int
> > xfs_trim_extents(
> > - struct xfs_mount *mp,
> > - xfs_agnumber_t agno,
> > + struct xfs_perag *pag,
> > xfs_daddr_t start,
> > xfs_daddr_t end,
> > xfs_daddr_t minlen,
> > uint64_t *blocks_trimmed)
> > {
> > + struct xfs_mount *mp = pag->pag_mount;
> > struct block_device *bdev = mp->m_ddev_targp->bt_bdev;
> > struct xfs_btree_cur *cur;
> > struct xfs_buf *agbp;
> > struct xfs_agf *agf;
> > - struct xfs_perag *pag;
> > int error;
> > int i;
> >
> > - pag = xfs_perag_get(mp, agno);
> > -
> > /*
> > * Force out the log. This means any transactions that might have freed
>
> This is a tangent, but one thing I've wondered is if it's really
> necessary to force the log for *every* AG that we want to trim? Even if
> we've just come from trimming the previous AG?
I suspect the thought behind this is that TRIM operations can be
really slow, so there can be a big build-up of new busy extents as a
large fragmented AG is trimmed.
I don't think it really matters at this point - if you are running a
multi-AG trim range, a few extra log forces is the least of your
performance worries. If someone reports it as a perf problem, let's
look at it then....
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
