On Tue, Dec 13, 2016 at 04:59:27PM +0100, Christoph Hellwig wrote:
> ->total is a bit of an odd parameter passed down to the low-level
> allocator all the way from the high-level callers. It's supposed to
> contain the maximum number of blocks to be allocated for the whole
> transaction [1].
>
> But in xfs_iomap_write_allocate we only convert existing delayed
> allocations and thus only have a minimal block reservation for the
> current transaction, so xfs_alloc_space_available can't use it for
> the allocation decisions. Use the maximum of args->total and the
> calculated block requirement to make a decision. We probably should
> get rid of args->total eventually and instead apply ->minleft more
> broadly, but that will require some extensive changes all over.
>
> [1] which creates lots of confusion as most callers don't decrement it
> once doing a first allocation. But that's for a separate series.
> ---
> fs/xfs/libxfs/xfs_alloc.c | 7 ++++---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> index 87328a8..85039e5 100644
> --- a/fs/xfs/libxfs/xfs_alloc.c
> +++ b/fs/xfs/libxfs/xfs_alloc.c
> @@ -1998,7 +1998,7 @@ xfs_alloc_space_available(
> int flags)
> {
> struct xfs_perag *pag = args->pag;
> - xfs_extlen_t longest;
> + xfs_extlen_t alloc_len, longest;
> xfs_extlen_t reservation; /* blocks that are still reserved */
> int available;
>
> @@ -2008,15 +2008,16 @@ xfs_alloc_space_available(
> reservation = xfs_ag_resv_needed(pag, args->resv);
>
> /* do we have enough contiguous free space for the allocation? */
> + alloc_len = args->minlen + (args->alignment - 1)+ args->minalignslop;
Whitespace nit, missing space: ^
Also, is the addition of braces intentional? I believe it is possible
for args->alignment == 0.
Nits aside:
Reviewed-by: Brian Foster <bfoster@xxxxxxxxxx>
> longest = xfs_alloc_longest_free_extent(args->mp, pag, min_free,
> reservation);
> - if ((args->minlen + args->alignment + args->minalignslop - 1) > longest)
> + if (longest < alloc_len)
> return false;
>
> /* do we have enough free space remaining for the allocation? */
> available = (int)(pag->pagf_freeblks + pag->pagf_flcount -
> reservation - min_free - args->minleft);
> - if (available < (int)args->total)
> + if (available < (int)max(args->total, alloc_len))
> return false;
>
> /*
> --
> 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
