public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <djwong@kernel.org>
To: Dave Chinner <david@fromorbit.com>
Cc: linux-xfs@vger.kernel.org
Subject: Re: [PATCH 28/42] xfs: convert xfs_alloc_vextent_iterate_ags() to use perag walker
Date: Wed, 1 Feb 2023 15:13:13 -0800	[thread overview]
Message-ID: <Y9ryCao6vEySwgOR@magnolia> (raw)
In-Reply-To: <20230118224505.1964941-29-david@fromorbit.com>

On Thu, Jan 19, 2023 at 09:44:51AM +1100, Dave Chinner wrote:
> From: Dave Chinner <dchinner@redhat.com>
> 
> Now that the AG iteration code in the core allocation code has been
> cleaned up, we can easily convert it to use a for_each_perag..()
> variant to use active references and skip AGs that it can't get
> active references on.
> 
> Signed-off-by: Dave Chinner <dchinner@redhat.com>
> ---
>  fs/xfs/libxfs/xfs_ag.h    | 22 ++++++---
>  fs/xfs/libxfs/xfs_alloc.c | 98 ++++++++++++++++++---------------------
>  2 files changed, 60 insertions(+), 60 deletions(-)
> 
> diff --git a/fs/xfs/libxfs/xfs_ag.h b/fs/xfs/libxfs/xfs_ag.h
> index 8f43b91d4cf3..5e18536dfdce 100644
> --- a/fs/xfs/libxfs/xfs_ag.h
> +++ b/fs/xfs/libxfs/xfs_ag.h
> @@ -253,6 +253,7 @@ xfs_perag_next_wrap(
>  	struct xfs_perag	*pag,
>  	xfs_agnumber_t		*agno,
>  	xfs_agnumber_t		stop_agno,
> +	xfs_agnumber_t		restart_agno,
>  	xfs_agnumber_t		wrap_agno)
>  {
>  	struct xfs_mount	*mp = pag->pag_mount;
> @@ -260,10 +261,11 @@ xfs_perag_next_wrap(
>  	*agno = pag->pag_agno + 1;
>  	xfs_perag_rele(pag);
>  	while (*agno != stop_agno) {
> -		if (*agno >= wrap_agno)
> -			*agno = 0;
> -		if (*agno == stop_agno)
> -			break;
> +		if (*agno >= wrap_agno) {
> +			if (restart_agno >= stop_agno)
> +				break;
> +			*agno = restart_agno;
> +		}
>  
>  		pag = xfs_perag_grab(mp, *agno);
>  		if (pag)
> @@ -274,14 +276,20 @@ xfs_perag_next_wrap(
>  }
>  
>  /*
> - * Iterate all AGs from start_agno through wrap_agno, then 0 through
> + * Iterate all AGs from start_agno through wrap_agno, then restart_agno through
>   * (start_agno - 1).
>   */
> -#define for_each_perag_wrap_at(mp, start_agno, wrap_agno, agno, pag) \
> +#define for_each_perag_wrap_range(mp, start_agno, restart_agno, wrap_agno, agno, pag) \
>  	for ((agno) = (start_agno), (pag) = xfs_perag_grab((mp), (agno)); \
>  		(pag) != NULL; \
>  		(pag) = xfs_perag_next_wrap((pag), &(agno), (start_agno), \
> -				(wrap_agno)))
> +				(restart_agno), (wrap_agno)))
> +/*
> + * Iterate all AGs from start_agno through wrap_agno, then 0 through
> + * (start_agno - 1).
> + */
> +#define for_each_perag_wrap_at(mp, start_agno, wrap_agno, agno, pag) \
> +	for_each_perag_wrap_range((mp), (start_agno), 0, (wrap_agno), (agno), (pag))
>  
>  /*
>   * Iterate all AGs from start_agno through to the end of the filesystem, then 0
> diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> index 43a054002da3..39f3e76efcab 100644
> --- a/fs/xfs/libxfs/xfs_alloc.c
> +++ b/fs/xfs/libxfs/xfs_alloc.c
> @@ -3156,6 +3156,7 @@ xfs_alloc_vextent_prepare_ag(
>  	if (need_pag)
>  		args->pag = xfs_perag_get(args->mp, args->agno);
>  
> +	args->agbp = NULL;
>  	error = xfs_alloc_fix_freelist(args, 0);
>  	if (error) {
>  		trace_xfs_alloc_vextent_nofix(args);
> @@ -3255,8 +3256,8 @@ xfs_alloc_vextent_finish(
>  	XFS_STATS_ADD(mp, xs_allocb, args->len);
>  
>  out_drop_perag:
> -	if (drop_perag) {
> -		xfs_perag_put(args->pag);
> +	if (drop_perag && args->pag) {
> +		xfs_perag_rele(args->pag);
>  		args->pag = NULL;
>  	}
>  	return error;
> @@ -3304,6 +3305,10 @@ xfs_alloc_vextent_this_ag(
>   * we attempt to allocation in as there is no locality optimisation possible for
>   * those allocations.
>   *
> + * On return, args->pag may be left referenced if we finish before the "all
> + * failed" return point. The allocation finish still needs the perag, and
> + * so the caller will release it once they've finished the allocation.
> + *
>   * When we wrap the AG iteration at the end of the filesystem, we have to be
>   * careful not to wrap into AGs below ones we already have locked in the
>   * transaction if we are doing a blocking iteration. This will result in an
> @@ -3318,72 +3323,59 @@ xfs_alloc_vextent_iterate_ags(
>  	uint32_t		flags)
>  {
>  	struct xfs_mount	*mp = args->mp;
> +	xfs_agnumber_t		agno;
>  	int			error = 0;
>  
> -	ASSERT(start_agno >= minimum_agno);
> +restart:
> +	for_each_perag_wrap_range(mp, start_agno, minimum_agno,
> +			mp->m_sb.sb_agcount, agno, args->pag) {
> +		args->agno = agno;
> +		trace_printk("sag %u minag %u agno %u pag %u, agbno %u, agcnt %u",
> +			start_agno, minimum_agno, agno, args->pag->pag_agno,
> +			target_agbno, mp->m_sb.sb_agcount);

Please remove the debugging statement or (if it's useful) convert this
to a static tracepoint.

--D

>  
> -	/*
> -	 * Loop over allocation groups twice; first time with
> -	 * trylock set, second time without.
> -	 */
> -	args->agno = start_agno;
> -	for (;;) {
> -		args->pag = xfs_perag_get(mp, args->agno);
>  		error = xfs_alloc_vextent_prepare_ag(args);
>  		if (error)
>  			break;
> -
> -		if (args->agbp) {
> -			/*
> -			 * Allocation is supposed to succeed now, so break out
> -			 * of the loop regardless of whether we succeed or not.
> -			 */
> -			if (args->agno == start_agno && target_agbno) {
> -				args->agbno = target_agbno;
> -				error = xfs_alloc_ag_vextent_near(args);
> -			} else {
> -				args->agbno = 0;
> -				error = xfs_alloc_ag_vextent_size(args);
> -			}
> -			break;
> +		if (!args->agbp) {
> +			trace_xfs_alloc_vextent_loopfailed(args);
> +			continue;
>  		}
>  
> -		trace_xfs_alloc_vextent_loopfailed(args);
> -
>  		/*
> -		 * If we are try-locking, we can't deadlock on AGF locks so we
> -		 * can wrap all the way back to the first AG. Otherwise, wrap
> -		 * back to the start AG so we can't deadlock and let the end of
> -		 * scan handler decide what to do next.
> +		 * Allocation is supposed to succeed now, so break out of the
> +		 * loop regardless of whether we succeed or not.
>  		 */
> -		if (++(args->agno) == mp->m_sb.sb_agcount) {
> -			if (flags & XFS_ALLOC_FLAG_TRYLOCK)
> -				args->agno = 0;
> -			else
> -				args->agno = minimum_agno;
> -		}
> -
> -		/*
> -		 * Reached the starting a.g., must either be done
> -		 * or switch to non-trylock mode.
> -		 */
> -		if (args->agno == start_agno) {
> -			if (flags == 0) {
> -				args->agbno = NULLAGBLOCK;
> -				trace_xfs_alloc_vextent_allfailed(args);
> -				break;
> -			}
> +		if (args->agno == start_agno && target_agbno) {
>  			args->agbno = target_agbno;
> -			flags = 0;
> +			error = xfs_alloc_ag_vextent_near(args);
> +		} else {
> +			args->agbno = 0;
> +			error = xfs_alloc_ag_vextent_size(args);
>  		}
> -		xfs_perag_put(args->pag);
> +		break;
> +	}
> +	if (error) {
> +		xfs_perag_rele(args->pag);
>  		args->pag = NULL;
> +		return error;
>  	}
> +	if (args->agbp)
> +		return 0;
> +
>  	/*
> -	 * The perag is left referenced in args for the caller to clean
> -	 * up after they've finished the allocation.
> +	 * We didn't find an AG we can alloation from. If we were given
> +	 * constraining flags by the caller, drop them and retry the allocation
> +	 * without any constraints being set.
>  	 */
> -	return error;
> +	if (flags) {
> +		flags = 0;
> +		goto restart;
> +	}
> +
> +	ASSERT(args->pag == NULL);
> +	trace_xfs_alloc_vextent_allfailed(args);
> +	return 0;
>  }
>  
>  /*
> @@ -3524,7 +3516,7 @@ xfs_alloc_vextent_near_bno(
>  	}
>  
>  	if (needs_perag)
> -		args->pag = xfs_perag_get(mp, args->agno);
> +		args->pag = xfs_perag_grab(mp, args->agno);
>  
>  	error = xfs_alloc_vextent_prepare_ag(args);
>  	if (!error && args->agbp)
> -- 
> 2.39.0
> 

  reply	other threads:[~2023-02-01 23:15 UTC|newest]

Thread overview: 79+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-18 22:44 [PATCH 00/42] xfs: per-ag centric allocation alogrithms Dave Chinner
2023-01-18 22:44 ` [PATCH 01/42] xfs: fix low space alloc deadlock Dave Chinner
2023-01-19 16:39   ` Allison Henderson
2024-12-23  9:43   ` luminosity1999
2023-01-18 22:44 ` [PATCH 02/42] xfs: prefer free inodes at ENOSPC over chunk allocation Dave Chinner
2023-01-19 19:08   ` Allison Henderson
2023-01-18 22:44 ` [PATCH 03/42] xfs: block reservation too large for minleft allocation Dave Chinner
2023-01-19 20:38   ` Allison Henderson
2023-01-18 22:44 ` [PATCH 04/42] xfs: drop firstblock constraints from allocation setup Dave Chinner
2023-01-19 22:03   ` Allison Henderson
2023-01-18 22:44 ` [PATCH 05/42] xfs: t_firstblock is tracking AGs not blocks Dave Chinner
2023-01-19 22:12   ` Allison Henderson
2023-01-18 22:44 ` [PATCH 06/42] xfs: don't assert fail on transaction cancel with deferred ops Dave Chinner
2023-01-19 22:18   ` Allison Henderson
2023-01-18 22:44 ` [PATCH 07/42] xfs: active perag reference counting Dave Chinner
2023-01-21  5:16   ` Allison Henderson
2023-02-01 19:08   ` Darrick J. Wong
2023-02-06 22:56     ` Dave Chinner
2023-01-18 22:44 ` [PATCH 08/42] xfs: rework the perag trace points to be perag centric Dave Chinner
2023-01-21  5:16   ` Allison Henderson
2023-01-18 22:44 ` [PATCH 09/42] xfs: convert xfs_imap() to take a perag Dave Chinner
2023-02-01 19:10   ` Darrick J. Wong
2023-01-18 22:44 ` [PATCH 10/42] xfs: use active perag references for inode allocation Dave Chinner
2023-01-22  6:48   ` Allison Henderson
2023-01-18 22:44 ` [PATCH 11/42] xfs: inobt can use perags in many more places than it does Dave Chinner
2023-01-22  6:48   ` Allison Henderson
2023-01-18 22:44 ` [PATCH 12/42] xfs: convert xfs_ialloc_next_ag() to an atomic Dave Chinner
2023-01-22  7:03   ` Allison Henderson
2023-01-18 22:44 ` [PATCH 13/42] xfs: perags need atomic operational state Dave Chinner
2023-01-23  4:04   ` Allison Henderson
2023-01-18 22:44 ` [PATCH 14/42] xfs: introduce xfs_for_each_perag_wrap() Dave Chinner
2023-01-23  5:41   ` Allison Henderson
2023-02-06 23:14     ` Dave Chinner
2023-02-01 19:28   ` Darrick J. Wong
2023-01-18 22:44 ` [PATCH 15/42] xfs: rework xfs_alloc_vextent() Dave Chinner
2023-02-01 19:39   ` Darrick J. Wong
2023-01-18 22:44 ` [PATCH 16/42] xfs: factor xfs_alloc_vextent_this_ag() for _iterate_ags() Dave Chinner
2023-01-18 22:44 ` [PATCH 17/42] xfs: combine __xfs_alloc_vextent_this_ag and xfs_alloc_ag_vextent Dave Chinner
2023-02-01 22:25   ` Darrick J. Wong
2023-01-18 22:44 ` [PATCH 18/42] xfs: use xfs_alloc_vextent_this_ag() where appropriate Dave Chinner
2023-01-18 22:44 ` [PATCH 19/42] xfs: factor xfs_bmap_btalloc() Dave Chinner
2023-01-18 22:44 ` [PATCH 20/42] xfs: use xfs_alloc_vextent_first_ag() where appropriate Dave Chinner
2023-02-01 22:43   ` Darrick J. Wong
2023-02-06 23:16     ` Dave Chinner
2023-01-18 22:44 ` [PATCH 21/42] xfs: use xfs_alloc_vextent_start_bno() " Dave Chinner
2023-02-01 22:51   ` Darrick J. Wong
2023-01-18 22:44 ` [PATCH 22/42] xfs: introduce xfs_alloc_vextent_near_bno() Dave Chinner
2023-02-01 22:52   ` Darrick J. Wong
2023-01-18 22:44 ` [PATCH 23/42] xfs: introduce xfs_alloc_vextent_exact_bno() Dave Chinner
2023-02-01 23:00   ` Darrick J. Wong
2023-01-18 22:44 ` [PATCH 24/42] xfs: introduce xfs_alloc_vextent_prepare() Dave Chinner
2023-01-18 22:44 ` [PATCH 25/42] xfs: move allocation accounting to xfs_alloc_vextent_set_fsbno() Dave Chinner
2023-01-18 22:44 ` [PATCH 26/42] xfs: fold xfs_alloc_ag_vextent() into callers Dave Chinner
2023-01-18 22:44 ` [PATCH 27/42] xfs: move the minimum agno checks into xfs_alloc_vextent_check_args Dave Chinner
2023-01-18 22:44 ` [PATCH 28/42] xfs: convert xfs_alloc_vextent_iterate_ags() to use perag walker Dave Chinner
2023-02-01 23:13   ` Darrick J. Wong [this message]
2023-01-18 22:44 ` [PATCH 29/42] xfs: convert trim to use for_each_perag_range Dave Chinner
2023-02-01 23:15   ` Darrick J. Wong
2023-02-06 23:19     ` Dave Chinner
2023-01-18 22:44 ` [PATCH 30/42] xfs: factor out filestreams from xfs_bmap_btalloc_nullfb Dave Chinner
2023-01-18 22:44 ` [PATCH 31/42] xfs: get rid of notinit from xfs_bmap_longest_free_extent Dave Chinner
2023-01-18 22:44 ` [PATCH 32/42] xfs: use xfs_bmap_longest_free_extent() in filestreams Dave Chinner
2023-01-18 22:44 ` [PATCH 33/42] xfs: move xfs_bmap_btalloc_filestreams() to xfs_filestreams.c Dave Chinner
2023-01-18 22:44 ` [PATCH 34/42] xfs: merge filestream AG lookup into xfs_filestream_select_ag() Dave Chinner
2023-01-18 22:44 ` [PATCH 35/42] xfs: merge new filestream AG selection " Dave Chinner
2023-01-18 22:44 ` [PATCH 36/42] xfs: remove xfs_filestream_select_ag() longest extent check Dave Chinner
2023-01-18 22:45 ` [PATCH 37/42] xfs: factor out MRU hit case in xfs_filestream_select_ag Dave Chinner
2023-01-18 22:45 ` [PATCH 38/42] xfs: track an active perag reference in filestreams Dave Chinner
2023-01-18 22:45 ` [PATCH 39/42] xfs: use for_each_perag_wrap in xfs_filestream_pick_ag Dave Chinner
2023-01-18 22:45 ` [PATCH 40/42] xfs: pass perag to filestreams tracing Dave Chinner
2023-01-18 22:45 ` [PATCH 41/42] xfs: return a referenced perag from filestreams allocator Dave Chinner
2023-02-02  0:01   ` Darrick J. Wong
2023-02-06 23:22     ` Dave Chinner
2023-01-18 22:45 ` [PATCH 42/42] xfs: refactor the filestreams allocator pick functions Dave Chinner
2023-02-02  0:08   ` Darrick J. Wong
2023-02-06 23:26     ` Dave Chinner
2023-02-02  0:14 ` [PATCH 00/42] xfs: per-ag centric allocation alogrithms Darrick J. Wong
2023-02-06 23:13   ` Dave Chinner
  -- strict thread matches above, loose matches on Subject: below --
2023-02-09 22:17 [PATCH v3 " Dave Chinner
2023-02-09 22:18 ` [PATCH 28/42] xfs: convert xfs_alloc_vextent_iterate_ags() to use perag walker Dave Chinner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Y9ryCao6vEySwgOR@magnolia \
    --to=djwong@kernel.org \
    --cc=david@fromorbit.com \
    --cc=linux-xfs@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox