On Tue, Dec 13, 2016 at 04:59:26PM +0100, Christoph Hellwig wrote:
> We must decide in xfs_alloc_fix_freelist if we can perform an
> allocation from a given AG is possible or not based on the available
> space, and should not fail the allocation past that point on a
> healthy file system.
>
> But currently we have two additional places that second-guess
> xfs_alloc_fix_freelist: xfs_alloc_ag_vextent tries to adjust the
> maxlen parameter to remove the reservation before doing the
> allocation (but ignores the various minium freespace requirements),
> and xfs_alloc_fix_minleft tries to fix up the allocated length
> after we've found an extent, but ignores the reservations and also
> doesn't take the AGFL into account (and thus fails allocations
> for not matching minlen in some cases).
>
> Remove all these later fixups and just correct the maxlen argument
> inside xfs_alloc_fix_freelist once we have the AGF buffer locked.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
> ---
> fs/xfs/libxfs/xfs_alloc.c | 78 +++++++++--------------------------------------
> fs/xfs/libxfs/xfs_alloc.h | 2 +-
> 2 files changed, 16 insertions(+), 64 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> index 91a6c17..87328a8 100644
> --- a/fs/xfs/libxfs/xfs_alloc.c
> +++ b/fs/xfs/libxfs/xfs_alloc.c
...
> @@ -2073,10 +2015,19 @@ xfs_alloc_space_available(
>
> /* do we have enough free space remaining for the allocation? */
> available = (int)(pag->pagf_freeblks + pag->pagf_flcount -
> - reservation - min_free - args->total);
> - if (available < (int)args->minleft || available <= 0)
> + reservation - min_free - args->minleft);
> + if (available < (int)args->total)
> return false;
>
> + /*
> + * Clamp maxlen to the amount of free space available for the actual
> + * extent allocation.
> + */
> + if (available < (int)args->maxlen && !(flags & XFS_ALLOC_FLAG_CHECK)) {
> + args->maxlen = available;
> + ASSERT(args->maxlen > 0);
Nit: could we assert args->maxlen >= args->minlen here as well?
Otherwise looks good to me:
Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>
> + }
> +
> return true;
> }
>
> @@ -2122,7 +2073,8 @@ xfs_alloc_fix_freelist(
> }
>
> need = xfs_alloc_min_freelist(mp, pag);
> - if (!xfs_alloc_space_available(args, need, flags))
> + if (!xfs_alloc_space_available(args, need, flags |
> + XFS_ALLOC_FLAG_CHECK))
> goto out_agbp_relse;
>
> /*
> diff --git a/fs/xfs/libxfs/xfs_alloc.h b/fs/xfs/libxfs/xfs_alloc.h
> index 7c404a6..1d0f48a 100644
> --- a/fs/xfs/libxfs/xfs_alloc.h
> +++ b/fs/xfs/libxfs/xfs_alloc.h
> @@ -56,7 +56,7 @@ typedef unsigned int xfs_alloctype_t;
> #define XFS_ALLOC_FLAG_FREEING 0x00000002 /* indicate caller is freeing extents*/
> #define XFS_ALLOC_FLAG_NORMAP 0x00000004 /* don't modify the rmapbt */
> #define XFS_ALLOC_FLAG_NOSHRINK 0x00000008 /* don't shrink the freelist */
> -
> +#define XFS_ALLOC_FLAG_CHECK 0x00000010 /* test only, don't modify args */
>
> /*
> * Argument structure for xfs_alloc routines.
> --
> 2.1.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
