On Wed, Feb 01, 2023 at 04:08:48PM -0800, Darrick J. Wong wrote:
> On Thu, Jan 19, 2023 at 09:45:05AM +1100, Dave Chinner wrote:
> > From: Dave Chinner <dchinner@xxxxxxxxxx>
> >
> > Now that the filestreams allocator is largely rewritten,
> > restructure the main entry point and pick function to seperate out
> > the different operations cleanly. The MRU lookup function should not
> > handle the start AG selection on MRU lookup failure, and nor should
> > the pick function handle building the association that is inserted
> > into the MRU.
> >
> > This leaves the filestreams allocator fairly clean and easy to
> > understand, returning to the caller with an active perag reference
> > and a target block to allocate at.
> >
> > Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
> > ---
> > fs/xfs/xfs_filestream.c | 247 +++++++++++++++++++++-------------------
> > fs/xfs/xfs_trace.h | 9 +-
> > 2 files changed, 132 insertions(+), 124 deletions(-)
> >
> > diff --git a/fs/xfs/xfs_filestream.c b/fs/xfs/xfs_filestream.c
> > index 523a3b8b5754..0a1d316ebdba 100644
> > --- a/fs/xfs/xfs_filestream.c
> > +++ b/fs/xfs/xfs_filestream.c
> > @@ -48,19 +48,19 @@ xfs_fstrm_free_func(
> > }
> >
> > /*
> > - * Scan the AGs starting at startag looking for an AG that isn't in use and has
> > - * at least minlen blocks free.
> > + * Scan the AGs starting at start_agno looking for an AG that isn't in use and
> > + * has at least minlen blocks free. If no AG is found to match the allocation
> > + * requirements, pick the AG with the most free space in it.
> > */
> > static int
> > xfs_filestream_pick_ag(
> > struct xfs_alloc_arg *args,
> > - struct xfs_inode *ip,
> > + xfs_ino_t pino,
> > xfs_agnumber_t start_agno,
> > int flags,
> > xfs_extlen_t *longest)
> > {
> > - struct xfs_mount *mp = ip->i_mount;
> > - struct xfs_fstrm_item *item;
> > + struct xfs_mount *mp = args->mp;
> > struct xfs_perag *pag;
> > struct xfs_perag *max_pag = NULL;
> > xfs_extlen_t minlen = *longest;
> > @@ -68,8 +68,6 @@ xfs_filestream_pick_ag(
> > xfs_agnumber_t agno;
> > int err, trylock;
>
> Who consumes trylock? Is this supposed to get passed through to
> xfs_bmap_longest_free_extent, or is the goal here merely to run the
> for_each_perag_wrap loop twice before going for the most free or any old
> perag?
It was originally used in this loop for directing the AGF locking,
but it looks like I removed all the cases where we we directly read
and lock AGFs in this loop. Hence it's now only used to run the loop
a second time. I'll change it to a boolean flag instead.
-Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
