On Tue, Dec 13, 2016 at 04:59:24PM +0100, Christoph Hellwig wrote:
> We can't just set minleft to 0 when we're low on space - that's exactly
> what we need minleft for: to protect space in the AG for btree block
> allocations when we are low on free space.
>
> Signed-off-by: Christoph Hellwig <hch@xxxxxx>
....
The xfs_alloc_vextent() loop changes look fine. The minleft hacks
always troubled me.
> ---
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 2760bc3..44773c9 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -3812,7 +3812,6 @@ xfs_bmap_btalloc(
> args.fsbno = 0;
> args.type = XFS_ALLOCTYPE_FIRST_AG;
> args.total = ap->minlen;
> - args.minleft = 0;
> if ((error = xfs_alloc_vextent(&args)))
> return error;
> ap->dfops->dop_low = true;
But this looks wrong. when combined with the next patch that sets:
args.minleft = xfs_bmap_worst_indlen(ap->ip, args.maxlen);
we've got a minleft value that might be for multigigabyte
allocation, but now we are only asking for a total allocation
of minlen, and that might be 1 block. IOWs, shouldn't this "last,
final attempt" code actually do something like this:
args.maxlen = ap->minlen;
args.total = ap->minlen;
args.minleft = xfs_bmap_worst_indlen(ap->ip, args.maxlen);
> +++ b/fs/xfs/libxfs/xfs_bmap_btree.c
> @@ -499,20 +499,6 @@ xfs_bmbt_alloc_block(
> goto try_another_ag;
> }
>
> - if (args.fsbno == NULLFSBLOCK && args.minleft) {
> - /*
> - * Could not find an AG with enough free space to satisfy
> - * a full btree split. Try again without minleft and if
> - * successful activate the lowspace algorithm.
> - */
> - args.fsbno = 0;
> - args.type = XFS_ALLOCTYPE_FIRST_AG;
> - args.minleft = 0;
> - error = xfs_alloc_vextent(&args);
> - if (error)
> - goto error0;
> - cur->bc_private.b.dfops->dop_low = true;
> - }
> if (args.fsbno == NULLFSBLOCK) {
> XFS_BTREE_TRACE_CURSOR(cur, XBT_EXIT);
> *stat = 0;
Yes, that's fair enough considering the common in the block that
set args.minleft, and that was a bug fix introduced long after this
fallback was put in place because the fallback could still fail...
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
--
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
