Re: [PATCH, RFC] xfs: take indirect blocks into accounting when selecting an AG

[Dave Chinner <david@fromorbit.com>]

On Fri, Dec 09, 2016 at 06:41:45PM +0100, Christoph Hellwig wrote:
> On Fri, Dec 09, 2016 at 09:32:13AM -0800, Darrick J. Wong wrote:
> > > +	if (xfs_alloc_is_userdata(args->datatype))
> > > +		indlen = __xfs_bmap_worst_indlen(args->mp, max(args->minlen, args->maxlen));
> > 
> > /me wonders, when is it the case that minlen > maxlen?
> 
> Good question.  I just added that when I noticed minlen alone doesn't
> work as we might need the bigger calculation based on maxlen.  I'll
> do a quick audit and move to maxlen only.
> 
> > 
> > I'm also wondering why we can't just increase args->minleft to require
> > that we leave enough space in whichever AG we pick to expand to bmbt?
> > AFAICT that's the purpose of the minleft field.
> 
> Not sure what the original intentions was, but as-is it seems pretty
> b0rked.
> 
> E.g. xfs_bmap_btalloc, xfs_bmapi_allocate or xfs_alloc_vextentjust set
> minleft to 0 when when we are low on space which make it a bit pointless.

I'm pretty sure these cases were added as a mechanism to prevent
ENOSPC occurrences during delalloc conversion when, in the general
case, the allocation would have succeeded. i.e. "in most cases the
BMBT is not going to change shape, so let's just ignore the blocks
that might be needed for that and hope we don't need them".

> Also in the bmap code where we set minleft we don't really know how
> much we'll need as we'll only decide on the actual final allocation
> size deep down in the allocator.  I'll do a little archeology session
> now to figure out how we got the current minleft semantics, as they seem
> really weird.

I'd just set it according to maxlen. I don't really care if we can't
allocate the last few blocks in an AG for certain types of
allocation - the failure fallbacks should drop back to an allocation
attempt of maxlen = minlen and we can set minleft accordingly
for those. If it still fails, then we really have ENOSPC...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com