On 23 Apr 2021 at 18:40, Brian Foster wrote:
> perag reservation is enabled at mount time on a per AG basis. The
> upcoming change to set aside allocbt blocks from block reservation
> requires a populated allocbt counter as soon as possible after mount
> to be fully effective against large perag reservations. Therefore as
> a preparation step, initialize the pagf on all mounts where at least
> one reservation is active. Note that this already occurs to some
> degree on most default format filesystems as reservation requirement
> calculations already depend on the AGF or AGI, depending on the
> reservation type.
>
The changes look good to me.
Reviewed-by: Chandan Babu R <chandanrlinux@xxxxxxxxx>
> Signed-off-by: Brian Foster <bfoster@xxxxxxxxxx>
> ---
> fs/xfs/libxfs/xfs_ag_resv.c | 34 +++++++++++++++++++++++-----------
> 1 file changed, 23 insertions(+), 11 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_ag_resv.c b/fs/xfs/libxfs/xfs_ag_resv.c
> index 6c5f8d10589c..e32a1833d523 100644
> --- a/fs/xfs/libxfs/xfs_ag_resv.c
> +++ b/fs/xfs/libxfs/xfs_ag_resv.c
> @@ -253,7 +253,8 @@ xfs_ag_resv_init(
> xfs_agnumber_t agno = pag->pag_agno;
> xfs_extlen_t ask;
> xfs_extlen_t used;
> - int error = 0;
> + int error = 0, error2;
> + bool has_resv = false;
>
> /* Create the metadata reservation. */
> if (pag->pag_meta_resv.ar_asked == 0) {
> @@ -291,6 +292,8 @@ xfs_ag_resv_init(
> if (error)
> goto out;
> }
> + if (ask)
> + has_resv = true;
> }
>
> /* Create the RMAPBT metadata reservation */
> @@ -304,19 +307,28 @@ xfs_ag_resv_init(
> error = __xfs_ag_resv_init(pag, XFS_AG_RESV_RMAPBT, ask, used);
> if (error)
> goto out;
> + if (ask)
> + has_resv = true;
> }
>
> -#ifdef DEBUG
> - /* need to read in the AGF for the ASSERT below to work */
> - error = xfs_alloc_pagf_init(pag->pag_mount, tp, pag->pag_agno, 0);
> - if (error)
> - return error;
> -
> - ASSERT(xfs_perag_resv(pag, XFS_AG_RESV_METADATA)->ar_reserved +
> - xfs_perag_resv(pag, XFS_AG_RESV_RMAPBT)->ar_reserved <=
> - pag->pagf_freeblks + pag->pagf_flcount);
> -#endif
> out:
> + /*
> + * Initialize the pagf if we have at least one active reservation on the
> + * AG. This may have occurred already via reservation calculation, but
> + * fall back to an explicit init to ensure the in-core allocbt usage
> + * counters are initialized as soon as possible. This is important
> + * because filesystems with large perag reservations are susceptible to
> + * free space reservation problems that the allocbt counter is used to
> + * address.
> + */
> + if (has_resv) {
> + error2 = xfs_alloc_pagf_init(mp, tp, pag->pag_agno, 0);
> + if (error2)
> + return error2;
> + ASSERT(xfs_perag_resv(pag, XFS_AG_RESV_METADATA)->ar_reserved +
> + xfs_perag_resv(pag, XFS_AG_RESV_RMAPBT)->ar_reserved <=
> + pag->pagf_freeblks + pag->pagf_flcount);
> + }
> return error;
> }
--
chandan
