Re: [PATCH 2/2] xfs: Limit total allocation request to maximum possible

[Brian Foster <bfoster@redhat.com>]

On Wed, Sep 18, 2019 at 10:24:53AM +0200, Carlos Maiolino wrote:
> The original allocation request may have a total value way beyond
> possible limits.
> 
> Trim it down to the maximum possible if needed
> 
> Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
> ---

Confused.. what was wrong with the original bma.total patch that it
needs to be replaced? I was assuming we'd replace the allocation retry
patch with the minlen alignment fixups and combine those with the
bma.total patch to fix the problem. Hm?

>  fs/xfs/libxfs/xfs_bmap.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 07aad70f3931..3aa0bf5cc7e3 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -3477,6 +3477,11 @@ xfs_bmap_btalloc(
>  			error = xfs_bmap_btalloc_filestreams(ap, &args, &blen);
>  		else
>  			error = xfs_bmap_btalloc_nullfb(ap, &args, &blen);
> +
> +		/* We can never have total larger than blen, so trim it now */
> +		if (args.total > blen)
> +			args.total = blen;
> +

I don't think this is safe. The reason the original patch only updated
certain callers is because those callers only used it for extra blocks
that are already incorported into bma.minleft by the bmap layer itself.
There are still other callers for which bma.total is specifically
intended to be larger than the map size.

(I think the whole total thing is still kind of a confusing mess in this
regard, but fixing that is a separate problem.)

Brian

>  		if (error)
>  			return error;
>  	} else if (ap->tp->t_flags & XFS_TRANS_LOWMODE) {
> -- 
> 2.20.1
>