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

[Christoph Hellwig <hch@lst.de>]

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.

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.

> /* Make sure we leave enough space in this AG for a bmbt expansion. */

Sure.

> > +xfs_filblks_t
> > +__xfs_bmap_worst_indlen(
> > +	xfs_mount_t	*mp,		/* mount structure */
> 
> struct xfs_mount?

Yeah, minimum changes for now.  If we end up going down this route I'd
probably just always pass the mount to xfs_bmap_worst_indlen, but
I wanted the minimal amount of change for now.